[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #378: Added try/catch for potential unparse exceptions

[GitBox <gi...@apache.org>]

stevedlawrence commented on a change in pull request #378:
URL: https://github.com/apache/incubator-daffodil/pull/378#discussion_r420785919



##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/InfosetInputter.scala
##########
@@ -420,7 +420,14 @@ abstract class InfosetInputter
         }
       } else if (erd.outputValueCalcExpr.isEmpty) {
         val primType = elem.erd.optPrimType.get
-        val obj = primType.fromXMLString(txt)
+        val obj = try {
+          primType.fromXMLString(txt)

Review comment:
       Grepping for ``fromXMLString``, there's a handful of other places where we call this without wrapping it with try/catch. I'm wondering if we have this same issue in other places and need the try/catch?
   
   I'm also concerned that different primitive types use different logic in their fromXMLString implementation that might throw different exceptions that we might not be accounting for here. Thoughts on modifying the ``fromXMLString`` implementations to be more thorough about checking for exceptions, and then they rethrow a single exception that callers of ``fromXMLString`` should catch so they don't have to know about particulars of different prim type implementations? This is similar to [DAFFODIL-1681](https://issues.apache.org/jira/browse/DAFFODIL-1681).

##########
File path: daffodil-test/src/test/resources/org/apache/daffodil/section05/simple_types/SimpleTypes.tdml
##########
@@ -3981,7 +3982,27 @@
       </tdml:dfdlInfoset>
     </tdml:infoset>
   </tdml:parserTestCase>
+
+   <!--
+    Test name: binaryInt_unparseError
+    Schema: SimpleTypes-binary
+    Purpose: Unparse a binary int with a value that is too large to be an xs:int
+  -->
   
+  <tdml:unparserTestCase name="binaryInt_unparseError" root="int02" roundTrip="false"
+      model="SimpleTypes-binary" description="Section 5 Schema types - int - DFDL-5-013R">
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <ex:int02>7089904312036126320</ex:int02>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+    <tdml:document>7089904312036126320</tdml:document>

Review comment:
       There shouldn't be a document if there are errors.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org