[GitHub] [incubator-daffodil] bsloane1650 commented on a change in pull request #200: Avoid supressing original parse errors

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

bsloane1650 commented on a change in pull request #200: Avoid supressing original parse errors
URL: https://github.com/apache/incubator-daffodil/pull/200#discussion_r266592784
 
 

 ##########
 File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
 ##########
 @@ -336,18 +336,26 @@ final class PState private (
    * pools - all items returned, etc.
    *
    * If for some reason parsing ends with a throw (not supposed to, but just if)
-   * then all bets are off, so this must be called ONLY on a normal return from parse call.
-   * That is, the parse can succeed or fail with diagnostics, but it must have returned normally.
+   * then all bets are off, so most checks are disabled.
+   * Some checks are still done. If those fail, we include the original error (if present) as the cause.
    */
-  def verifyFinalState(wasThrow: Boolean): Unit = {
-    if (!wasThrow) {
-      Assert.invariant(this.discriminatorStack.length == 1)
-      mpstate.verifyFinalState()
+  def verifyFinalState(optThrown: Maybe[Throwable]): Unit = {
+    try{
+      if (optThrown.isEmpty) {
+        Assert.invariant(this.discriminatorStack.length == 1)
+        mpstate.verifyFinalState()
+      }
+      // These we check regardless of throw or not.
+      markPool.finalCheck
+      dataInputStream.inputSource.compact // discard any storage that can be freed.
+      dataInputStream.validateFinalStreamState
+    }catch{
+      case e:Throwable => {
+        println(s"Suppressed error: ${optThrown}")
 
 Review comment:
   Looking at this again, I do not see the need for any special print/log here. Since it is now included as a suppressed error of what is thrown, it should get displayed along with whatever displays that.
   
   What I do see is that the error being thrown here will potentially be interperated as a control-flow exception in Runtime.scala, even if we are supressing a non control-flow exception. Currently this won't happen, and I can't imagine why someone would even attempt throwing, say, an SDE from here, but it is non-obvious that such would be a problem. To avoid this issue, I am going to be wrapping all the errors in a Abort (which subclasses unsupressableException).

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