[GitHub] [daffodil] stevedlawrence commented on a diff in pull request #1058: Better Error Reporting for Library API

["stevedlawrence (via GitHub)" <gi...@apache.org>]

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


##########
daffodil-lib/src/main/scala/org/apache/daffodil/lib/api/Diagnostic.scala:
##########
@@ -308,4 +308,12 @@ trait WithDiagnostics {
    * then one can proceed to run the compiled entity.
    */
   def isError: Boolean
+
+  /**
+   * Helper method to check that isError is false, if not it throws
+   * a usage error caused by illegal state caused by a compilation error
+   */
+  def checkNotError(): Unit = {
+    Assert.usage(!isError, new IllegalStateException(getDiagnostics.find(_.isError).get))

Review Comment:
   We've lost the message that says the API is being used incorrectly so a developer using the API wont know what to change. The user will just see something like:
   
   > Usage Error: InvalidStateException, cause: Schema Defition Error: ....
   
   The SDE found in the diagnostic is not the problem, it's more like a symptom. The underlying problem is incorrect API usage, so that needs to be included in the error message. The core issue of this bug is that it's not clear what to do if the check fails and the user gets an exception, and I don't think this does anything to make that more clear.



##########
daffodil-sapi/src/test/scala/org/apache/daffodil/example/TestScalaAPI.scala:
##########
@@ -887,7 +887,7 @@ class TestScalaAPI {
     try {
       res = dp.parse(input, outputter)
     } catch {
-      case e: Abort => {
+      case e: UsageException => {

Review Comment:
   This is an API change, please remember to add a Deprecation/Compatibility section to your commit message.



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