From 37514f1ec7ce117a15a278e5c471bbd0b00afe7b Mon Sep 17 00:00:00 2001 From: Guichard Desrosiers Date: Tue, 5 May 2026 21:05:06 -0400 Subject: [PATCH] Fix duplicated TDML exception output and simplify exception handling TDMLException previously embedded cause message text in the outer exception message while also passing the same cause into the Java Exception cause chain. This resulted in duplicate output, where the same error appeared both in the exception message and again in the "Caused by:" section. -Simplified TDMLException to support two construction paths only: one for explicit message strings, and one for wrapping a Throwable. Removed the causes field from the TDMLException trait and use cause from TDMLExceptionImpl constructor parameter. -Also removed the constructor that accepted multiple causes. Callers that previously passed multiple exceptions now aggregate their messages into a single newline-delimited message before constructing the string-based exception. -Additionally updated TDMLRunner to catch XMLDifferenceException directly when calling XMLUtils.compareAndReport, since that method specifically throws XMLDifferenceException. DAFFODIL-3078 --- .../apache/daffodil/tdml/TDMLException.scala | 26 +++++--------- .../org/apache/daffodil/tdml/TDMLRunner.scala | 35 +++++++++++-------- 2 files changed, 28 insertions(+), 33 deletions(-) diff --git a/daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLException.scala b/daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLException.scala index f17289448d..1d46f1f51f 100644 --- a/daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLException.scala +++ b/daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLException.scala @@ -34,8 +34,6 @@ object TDMLException { new TDMLExceptionImpl(msg, implementation) def apply(cause: Throwable, implementation: Option[String]) = new TDMLExceptionImpl(cause, implementation) - def apply(causes: Iterable[Throwable], implementation: Option[String]) = - new TDMLExceptionImpl(causes, implementation) } /** @@ -46,7 +44,6 @@ object TDMLException { */ trait TDMLException { self: Exception => def msg: String - def causes: Iterable[Throwable] def implementation: Option[String] /** @@ -63,27 +60,20 @@ trait TDMLException { self: Exception => class TDMLExceptionImpl( override val msg: String, - override val causes: Iterable[Throwable], + cause: Throwable, override val implementation: Option[String] ) extends Exception( TDMLException.msgWithImpl(msg, implementation), - if (causes.nonEmpty) causes.head else null + cause ) with TDMLException { - + // message-only constructor def this(msg: String, implementation: Option[String]) = - this(msg, Nil, implementation) + this(msg, null, implementation) + // cause-based constructor def this(cause: Throwable, implementation: Option[String]) = - this(Misc.getNameFromClass(cause) + ": " + cause.getMessage(), List(cause), implementation) - - def this(causes: Iterable[Throwable], implementation: Option[String]) = this( - causes - .map { cause => Misc.getNameFromClass(cause) + ": " + cause.getMessage() } - .mkString("\n"), - causes, - implementation - ) + this(Misc.getNameFromClass(cause), cause, implementation) } /** @@ -105,7 +95,7 @@ class TDMLDiagnostic(diag: String, implementation: Option[String]) * with the implementation. Useful since this isn't necessarily a failure and * may want to be treated differently in some cases. * - * Carries causes because a failure to detect compatibility can be due to + * Carries a cause because a failure to detect compatibility can be due to * failures to reflectively create a junit org.junit.AssumptionViolatedException, and * if that is the case, we may need the exception to figure out the reason why * the reflective access failed. @@ -116,6 +106,6 @@ class TDMLTestNotCompatibleException( cause: Option[Throwable] = None ) extends TDMLExceptionImpl( "Test '%s' not compatible with implementation.".format(testName), - cause.toSeq, + cause.orNull, implementation ) diff --git a/daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala b/daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala index 0549c61ac1..24ea5857bf 100644 --- a/daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala +++ b/daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala @@ -75,6 +75,7 @@ import org.apache.daffodil.lib.util.SchemaUtils import org.apache.daffodil.lib.util.ThreadSafePool import org.apache.daffodil.lib.xml.DaffodilXMLLoader import org.apache.daffodil.lib.xml.XMLUtils +import org.apache.daffodil.lib.xml.XMLUtils.XMLDifferenceException import org.apache.daffodil.tdml.DiagnosticType.DiagnosticType import org.apache.daffodil.tdml.processor.AbstractTDMLDFDLProcessorFactory import org.apache.daffodil.tdml.processor.TDML @@ -140,6 +141,13 @@ private[tdml] object DFDLTestSuite { } +private[tdml] def aggregateExceptionMessages( + exceptions: Iterable[Throwable] +): String = + exceptions + .map(_.getMessage) + .mkString("\n") + /** * TDML test suite runner * @@ -375,7 +383,7 @@ class DFDLTestSuite private[tdml] ( } def reportLoadingErrors(): Nothing = { - throw TDMLException(loadingExceptions, None) + throw TDMLException(aggregateExceptionMessages(loadingExceptions), None) } var checkAllTopLevel: Boolean = compileAllTopLevel @@ -819,7 +827,8 @@ abstract class TestCase(testCaseXML: NodeSeq, val parent: DFDLTestSuite) { Some(XMLUtils.dafextURI) ) if (node eq null) - throw TDMLException(parent.loadingExceptions, None) + throw TDMLException(aggregateExceptionMessages(parent.loadingExceptions), None) + val definedConfig = DefinedConfig(node, parent) Some(definedConfig) } @@ -1212,9 +1221,7 @@ case class ParserTestCase(ptc: NodeSeq, parentArg: DFDLTestSuite) if (actual.isProcessingError) { // Means there was an error, not just warnings. - if (diagObjs.length == 1) throw TDMLException(diagObjs.head, implString) - val diags = actual.getDiagnostics.asScala.map(_.toString()).mkString("\n") - throw TDMLException(diags, implString) + throw TDMLException(aggregateExceptionMessages(diagObjs), implString) } else { // If we think we've succeeded, verify there are no errors // captured in the diagnostics. Otherwise there's probably @@ -1283,8 +1290,7 @@ case class ParserTestCase(ptc: NodeSeq, parentArg: DFDLTestSuite) val unparseResult = processor.unparse(parseResult, outStream) if (unparseResult.isProcessingError) { val diagObjs = unparseResult.getDiagnostics.asScala - if (diagObjs.length == 1) throw TDMLException(diagObjs.head, implString) - throw TDMLException(diagObjs, implString) + throw TDMLException(aggregateExceptionMessages(diagObjs), implString) } unparseResult } @@ -1545,7 +1551,8 @@ case class UnparserTestCase(ptc: NodeSeq, parentArg: DFDLTestSuite) (optExpectedData, optErrors) match { case (Some(expectedData), None) => { compileResult match { - case Left(diags) => throw TDMLException(diags.asScala, implString) + case Left(diags) => + throw TDMLException(aggregateExceptionMessages(diags.asScala), implString) case Right((diags, proc)) => { processor = proc runUnparserExpectSuccess( @@ -1614,7 +1621,7 @@ case class UnparserTestCase(ptc: NodeSeq, parentArg: DFDLTestSuite) case t: Throwable => toss(t, implString) } if (actual.isProcessingError) - throw TDMLException(actual.getDiagnostics.asScala, implString) + throw TDMLException(aggregateExceptionMessages(actual.getDiagnostics.asScala), implString) // // Test that we are getting the number of full bytes needed. @@ -1662,9 +1669,7 @@ case class UnparserTestCase(ptc: NodeSeq, parentArg: DFDLTestSuite) if (parseActual.isProcessingError) { // Means there was an error, not just warnings. - val diagObjs = parseActual.getDiagnostics - if (diagObjs.size == 1) throw diagObjs.get(0) - val diags = parseActual.getDiagnostics.asScala.map(_.toString()).mkString("\n") + val diags = aggregateExceptionMessages(parseActual.getDiagnostics.asScala) throw TDMLException(diags, implString) } val loc: api.DataLocation = parseActual.currentLocation @@ -1784,8 +1789,8 @@ object VerifyTestCase { try { XMLUtils.compareAndReport(expected, actual) } catch { - case e: Exception => - throw TDMLException(e, implString) + case e: XMLDifferenceException => + throw TDMLException(e.getMessage, implString) } } @@ -2819,7 +2824,7 @@ case class DFDLInfoset(di: Node, parent: Infoset) { val hasMoreExceptions = before.size < nAfter if (hasMoreExceptions) { val newExceptions = (testSuite.loadingExceptions.diff(before)) - testCase.toss(TDMLException(newExceptions, None), None) + testCase.toss(TDMLException(aggregateExceptionMessages(newExceptions), None), None) } elem.asInstanceOf[Elem] }