[GitHub] [incubator-daffodil] bsloane1650 edited a comment on issue #201: Fix bug in handling parse errors

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

bsloane1650 edited a comment on issue #201: Fix bug in handling parse errors
URL: https://github.com/apache/incubator-daffodil/pull/201#issuecomment-474395380
 
 
   @stevedlawrence 
   
   Unfortunately, the original error (which caused the poorly behaving Parse error) was in simply broken code so I don't know of any way to trigger it from a TDML. I can probably create a unit test that exersises that particular error path.
   
   Updating the type to be an Either type seems like the correct solution. However, as we are in the runtime system, I am wondering if the use of Maybe here was a deliberate choice for performance reasons. (I believe these objects get created routinely as part of backtracking).
   
   I am also a bit suspicious of the way the original asymetry between the parse and unparse case:
   
   ```
   def toParseError = new ParseError(schemaContext, dataContext, Maybe(this), maybeFormatString, args: _*)
   
   def toUnparseError = new UnparseError(schemaContext, dataContext, maybeCause, maybeFormatString, args: _*)
   ``` 
   
   Since you are not allowed to have both maybeCause and maybeFormatString, I didn't see how the parse case could ever be correct, so I just copied the unparse case. (In particular, a caller may never pass maybeFormatString to ProcessingError if it would ever be converted to a ParseError, and I suspect that calling _.toParseError.toParseError will behave silly in that the error will be nested an additional time for each call.)
   
   Looking at the code, it seems the info being lost by not including "this" as the cause is the modeName arguement passed to the outer ProcessingError ("Expression Evaluation" in this case). Unfortunately, ParseError makes a point of being a "Parse" error which, by definition, overwrites the value of modeName with "Parse".
   
   Maybe it makes sense for modeName to be some sort of list? so the generated error looks more like:
   
   ```
   Parse Error: Expression Evaluation Error: Cannot convert 'not an int!' from String type to Long (Cannot convert to type long: For input string: "not an int!").
   ```
   
   (The repetition of "Error" is independent, but probably a good idea from the perspective of string matching).

----------------------------------------------------------------
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


With regards,
Apache Git Services