[incubator-daffodil] branch master updated: Avoid supressing original parse errors

[sl...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

slawrence pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-daffodil.git


The following commit(s) were added to refs/heads/master by this push:
     new 9719bb4  Avoid supressing original parse errors
9719bb4 is described below

commit 9719bb400977e336e0a1a5a115a58a4af198bc6d
Author: Brandon Sloane <bs...@tresys.com>
AuthorDate: Mon Mar 18 11:48:05 2019 -0400

    Avoid supressing original parse errors
    
    After parsing an element, regardless of if an error was thrown,
    additional state validation is conducted. If said validation fails,
    following an error being thrown in the parse function, we now include
    the original error as a supressed error in the final one.
    
    DAFFODIL-2099
---
 .../org/apache/daffodil/processors/Runtime.scala   | 16 ++++++----
 .../daffodil/processors/parsers/PState.scala       | 35 +++++++++++++++-------
 2 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/Runtime.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/Runtime.scala
index f45d7fc..b1743a0 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/Runtime.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/Runtime.scala
@@ -216,7 +216,7 @@ class DataProcessor(val ssrd: SchemaSetRuntimeData)
   }
 
   private def doParse(p: Parser, state: PState) {
-    var wasThrow = true
+    var optThrown: Maybe[Throwable] = None
     try {
       try {
         this.startElement(state, p)
@@ -232,11 +232,15 @@ class DataProcessor(val ssrd: SchemaSetRuntimeData)
         // so that subsequent use of the state can generally work and have a context.
         //
         state.setMaybeProcessor(Maybe(p))
-
-        wasThrow = false
-      } finally {
-        state.verifyFinalState(wasThrow)
-      }
+      } catch {
+        //We will actually be handling all errors in the outer loop
+        //However, there is a chance that our finally block will itself throw.
+        //In such a case, it is useful to include the original error.
+        case e: Throwable => {
+          optThrown = Some(e)
+          throw e
+        }
+      } finally { state.verifyFinalState(optThrown) }
 
     } catch {
       // technically, runtime shouldn't throw. It's really too heavyweight a construct. And "failure"
diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
index 7c8746b..23b1ef2 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
@@ -60,6 +60,8 @@ import org.apache.daffodil.util.Pool
 import org.apache.daffodil.util.Poolable
 import org.apache.daffodil.infoset.DIComplexState
 import org.apache.daffodil.infoset.DISimpleState
+import org.apache.daffodil.exceptions.UnsuppressableException
+import org.apache.daffodil.exceptions.Abort
 
 object MPState {
 
@@ -336,18 +338,31 @@ 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.
+   * 
+   * verifyFinalState may be called from within another try..catch block for which certain excepetions
+   * are expected as control flow.
+   * To avoid accidently creating such an exception, we wrap our generated exception in UnsuppressableException
    */
-  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 => {
+        if (optThrown.isDefined) e.addSuppressed(optThrown.get)
+        val toThrow = new Abort(e)
+        toThrow.addSuppressed(e)
+        throw toThrow
+      }
     }
-    // These we check regardless of throw or not.
-    markPool.finalCheck
-    dataInputStream.inputSource.compact // discard any storage that can be freed.
-    dataInputStream.validateFinalStreamState
   }
 }