Uploaded image for project: 'Solder'
  1. Solder
  2. SOLDER-315

Seam catch eats all exceptions silently

    Details

    • Estimated Difficulty:
      Medium

      Description

      I observed that seam catch eats all exceptions silently. Specifically it make my development stop for a few hours becouse we didn't know there was model update exception. Here is what I investigated:

      1. Every exception in CatchExceptionFilter appear to be handled (ie. catchEvent.isHandled() returned always true)
      2. It was becouse it is marked handled in ExceptionHandlerDispatch.executeHandlers switch statement. I.e. here:

      ...
      switch (breadthFirstEvent.getFlow()) {
      case HANDLED:
      eventException.setHandled(true);
      return;
      case MARK_HANDLED:
      eventException.setHandled(true); <<<<<<<<<<<<<<<<<<
      ....

      3. breadthFirstEvent.getFlow() is MARK_HANDLED becouse it's the default value.

      The result is that it's enough there is a matching exception handler to mark the caught exception as handled and stop any exception display. And it's highly probable that you have seam-transaction in your library, so you have org.jboss.seam.transaction.SimpleTransactionExceptionHandler.markTransactionRollback handler active. That handler is intended to mark all the exceptional transactions to roll back. However it is irrelevant for this handler what kind of exception it handles. I wouldn't say that it actually handles exceptions. It just wants to know of all the exception that may cause transaction to be invalid and to rollback the transaction. But it definitely should be no mean to stop further processing of exception and it was probably not intended to hide exceptions.

      I think there should be (at least) two kind of handlers. Those that just listen to exceptions and do some actions but they are not supposed to be a final destination for exceptions. These could be transaction related handlers, logging handlers, etc. And there should also be handlers that are able to deal with specific exceptions ultimately.

      Right now there is a notion that exception is handled if there is a handler for the exception. If so - SimpleTransactionExceptionHandler should not be a handler.

      I think it would be better if MARK_HANDLED was renamed to UNHANDLED and wouldn't cause eventException.setHandled(true). I would then use some annotation to add to those handler that are ultimate and definite, or set handled property programatically. The whole difference between HANDLED and MARK_HANDLED states is artificial. It feels like there should be a difference when some handler says that it really handled the situation or did not.

        Gliffy Diagrams

          Attachments

            Activity

              People

              • Assignee:
                Unassigned
                Reporter:
                mrmimo Mike Mosiewicz
              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: