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:
[email protected]


With regards,
Apache Git Services

Reply via email to