Tapestry 5
  1. Tapestry 5
  2. TAP5-508

Exception reports can over-zealously squash exceptions

    Details

      Description

      Unchecked exceptions are reported differently on Exception reporting page.

      This should be changed, because many applications have convention to use an unchecked exception hierachy.

      1. TAP509.patch
        0.8 kB
        manuel aldana
      2. uncheckedExceptionTypeNotPrinted.png
        10 kB
        manuel aldana
      3. uncheckExceptionSwallowedIfChainingUsed.png
        40 kB
        manuel aldana

        Activity

        Hide
        manuel aldana added a comment -

        example for inconsistent reporting between checked and unchecked (type is not printend).

        Show
        manuel aldana added a comment - example for inconsistent reporting between checked and unchecked (type is not printend).
        Hide
        manuel aldana added a comment -

        patch, resolution: wrapping unchecked RuntimeException to another RuntimeException. This way the reporting is consistent between checked and unchecked.

        Show
        manuel aldana added a comment - patch, resolution: wrapping unchecked RuntimeException to another RuntimeException. This way the reporting is consistent between checked and unchecked.
        Hide
        manuel aldana added a comment -

        see patch attachement comment

        Show
        manuel aldana added a comment - see patch attachement comment
        Hide
        Filip S. Adamsen added a comment -

        The class name (or exception type, if you will) is part of the message created when you chain exceptions - it's not something Tapestry does, just regular Java.

        I don't see what the problem is, anyway - the type of the exception is displayed right below the line you included in your screenshot - and I certainly don't want Tapestry to wrap all RuntimeExceptions in yet another RuntimeException because of it.

        Show
        Filip S. Adamsen added a comment - The class name (or exception type, if you will) is part of the message created when you chain exceptions - it's not something Tapestry does, just regular Java. I don't see what the problem is, anyway - the type of the exception is displayed right below the line you included in your screenshot - and I certainly don't want Tapestry to wrap all RuntimeExceptions in yet another RuntimeException because of it.
        Hide
        manuel aldana added a comment -

        Yes, I still think it is inconsistent behaviour (error page should be transparent, whether user threw a checked or an unchecked exception himself).

        Even worse, when using exception-chaining it happens that unchecked application exception is not shown at all and is being swallowed (see screenshot uncheckExceptionSwallowedIfChainingUsed.png).

        I tend to use unchecked exception in my application and use exception chaining a lot. I remember that I was quite confused that my application exception did not show up on the error page.

        Reason seems to be that first top-level exception is filtered in error page. The easiest fix was to use this last RuntimeException wrap.

        Is this fix really an issue? What speaks against it?

        Show
        manuel aldana added a comment - Yes, I still think it is inconsistent behaviour (error page should be transparent, whether user threw a checked or an unchecked exception himself). Even worse, when using exception-chaining it happens that unchecked application exception is not shown at all and is being swallowed (see screenshot uncheckExceptionSwallowedIfChainingUsed.png). I tend to use unchecked exception in my application and use exception chaining a lot. I remember that I was quite confused that my application exception did not show up on the error page. Reason seems to be that first top-level exception is filtered in error page. The easiest fix was to use this last RuntimeException wrap. Is this fix really an issue? What speaks against it?
        Hide
        Filip S. Adamsen added a comment -

        Okay, I see now. I just tried chaining 3 runtime exception and as you said, only the innermost one was shown. That's odd, and certainly not what I would expect.

        I still don't like wrapping the whole thing in another exception, though. I believe fixing the root cause (no pun intended) is the way to go instead.

        Show
        Filip S. Adamsen added a comment - Okay, I see now. I just tried chaining 3 runtime exception and as you said, only the innermost one was shown. That's odd, and certainly not what I would expect. I still don't like wrapping the whole thing in another exception, though. I believe fixing the root cause (no pun intended) is the way to go instead.
        Hide
        Howard M. Lewis Ship added a comment -

        Filtering out the duplicate exceptions is intentional, otherwise the exception report page gets overwhelmingly cluttered with redundant data.

        Show
        Howard M. Lewis Ship added a comment - Filtering out the duplicate exceptions is intentional, otherwise the exception report page gets overwhelmingly cluttered with redundant data.
        Hide
        Howard M. Lewis Ship added a comment -

        Perhaps I could change presentation of "duplicate" exceptions to work like the new approach to stack frames: initially hidden (but still present), revealed via a checkbox.

        Show
        Howard M. Lewis Ship added a comment - Perhaps I could change presentation of "duplicate" exceptions to work like the new approach to stack frames: initially hidden (but still present), revealed via a checkbox.
        Hide
        Howard M. Lewis Ship added a comment -

        Tapestry has rules for which exceptions it squashes; in 5.2 and earlier, the rules are exceptions are squashed if they "add no value".

        • An exception whose message is a substring of its containing exception adds no value
        • An exception whose property values are present in the containing exception adds no value

        So you're seeing some of your checked exceptions squashed because the only difference is the exception class name.

        I'm temporarily adding a check: when the exception class name changes, the frame DOES add value (is not squashed)

        Show
        Howard M. Lewis Ship added a comment - Tapestry has rules for which exceptions it squashes; in 5.2 and earlier, the rules are exceptions are squashed if they "add no value". An exception whose message is a substring of its containing exception adds no value An exception whose property values are present in the containing exception adds no value So you're seeing some of your checked exceptions squashed because the only difference is the exception class name. I'm temporarily adding a check: when the exception class name changes, the frame DOES add value (is not squashed)
        Hide
        Hudson added a comment -

        Integrated in tapestry-trunk-freestyle #580 (See https://builds.apache.org/job/tapestry-trunk-freestyle/580/)
        TAP5-508: Exception reports can over-zealously squash exceptions

        hlship : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1185433
        Files :

        • /tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/ExceptionAnalyzerImpl.java
        Show
        Hudson added a comment - Integrated in tapestry-trunk-freestyle #580 (See https://builds.apache.org/job/tapestry-trunk-freestyle/580/ ) TAP5-508 : Exception reports can over-zealously squash exceptions hlship : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1185433 Files : /tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/ExceptionAnalyzerImpl.java

          People

          • Assignee:
            Howard M. Lewis Ship
            Reporter:
            manuel aldana
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development