You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "stevedlawrence (via GitHub)" <gi...@apache.org> on 2023/03/09 20:47:26 UTC

[GitHub] [daffodil] stevedlawrence commented on a diff in pull request #985: Throw exceptions on fatal SAX parsing errors

stevedlawrence commented on code in PR #985:
URL: https://github.com/apache/daffodil/pull/985#discussion_r1131575400


##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/DFDLSchemaFile.scala:
##########
@@ -92,12 +92,12 @@ class DFDLSchemaFileLoadErrorHandler(schemaFileLocation: SchemaFileLocation)
   }
 
   /**
-   * Called on a fatal exception. The parser/validator throws the exception after
-   * this call returns.
+   * Called on a fatal exception and throws the exception
    */
-  def fatalError(exception: SAXParseException) = error(
-    exception,
-  ) // same as non-fatal exception.
+  def fatalError(exception: SAXParseException) = {
+    loaderErrors_ :+= exception
+    throw exception

Review Comment:
   Reading the javadoc for fatalError, it seems like we shouldn't need to throw an exception here, and that the parser should throw the exception after calling fatalError if it is unable to continue.
   
   If this is needed to workaround a bug in Scala-xml that's fine, but it should be documented why it's needed. Maybe scala-xml expects fatalError implementations to throw the exceptions?



##########
daffodil-test/src/test/scala/org/apache/daffodil/section00/general/TestDisallowDocType.scala:
##########
@@ -87,7 +86,7 @@ class TestDisallowDocType {
   }
 
   @Test def test_infosetFileMustNotHaveDocType(): Unit = {
-    val e = intercept[TDMLException] {
+    val e = intercept[SAXParseException] {

Review Comment:
   Seems we shouldn't let SAXParseExceptions bubble  to the very top? I think the way the DaffodilConstructorLoader is intended to work is it doesn't throw exceptions but instead gathers up all errors in the error handler diagnostics to we can report them later?



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

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

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