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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
