castor
  1. castor
  2. CASTOR-1642

Exception during marshalling not unreported

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 1.0.3, 1.0.4
    • Fix Version/s: 1.0.5
    • Component/s: XML
    • Labels:
      None
    • Testcase included:
      yes
    • Number of attachments :
      4

      Description

      If an Exception occurs in the getValue of a field during marshalling, it is hidden in a catch with no logging and exception. For example, if an exception if thrown with the value for field "name" is gotten with "getValue", it is wrapped in an InvocationTargetException (from the invocation using reflection) and then in a CastorInvalidStateException (see FieldHandlerImpl.java:454 and 458).

      When the CastorInvalidStateException gets back to the Marshaller, it is squashed completely on line 1716:

      try

      { obj = elemDescriptor.getHandler().getValue(object); }

      catch(IllegalStateException ise)

      { continue; }

      In one case, my getXXX method did a DB query, which failed but there was nothing reported. In another case, there was a class loading problem, and again, nothing was reported.

      I think at least a error log of some sort is required. I would prefer to see a MarshallingException thrown, wrapping the original Exception.

      1. CastorExceptionTest.java
        1 kB
        Paul Philion
      2. CastorExceptionTest.xml
        0.4 kB
        Paul Philion
      3. marshal-exception-patch.txt
        1 kB
        Paul Philion
      4. marshaller-logging-patch.txt
        8 kB
        Paul Philion

        Issue Links

          Activity

          Hide
          Edward Kuns added a comment -

          Better than a custom log4j appender would just be a new option to either log or log-and-throw exceptions encountered during marshaling.

          As a first step in that direction, we could add getter/setter for something like "swallowExceptions" (some better name for the same trait) which by default would be true (for backwards compatiblity). And with this first step, someone instantiating a marshaler/unmarshaler could then use the setter to make the un/marshalers log-and-throw exceptions.

          As a second step in that direction, we could add a parameter to castor.properties (perhaps) and/or castorbuilder.properties and/or some other method of doing this globally so that this setting wouldn't have to be made programmatically on each marshaler and unmarshaler.

          Best would probably be to make subtasks of this task for these steps. Thoughts?

          Show
          Edward Kuns added a comment - Better than a custom log4j appender would just be a new option to either log or log-and-throw exceptions encountered during marshaling. As a first step in that direction, we could add getter/setter for something like "swallowExceptions" (some better name for the same trait) which by default would be true (for backwards compatiblity). And with this first step, someone instantiating a marshaler/unmarshaler could then use the setter to make the un/marshalers log-and-throw exceptions. As a second step in that direction, we could add a parameter to castor.properties (perhaps) and/or castorbuilder.properties and/or some other method of doing this globally so that this setting wouldn't have to be made programmatically on each marshaler and unmarshaler. Best would probably be to make subtasks of this task for these steps. Thoughts?
          Hide
          Niall Smart added a comment -

          The custom log4j appender suggestion is a temporary hack until a better bug fix is in place (in any case it assumes commons-logging is using log4j).

          In the longer term I would prefer to see the listener pattern used instead of just a simple flag: it allows the caller more flexibility (e.g., to decide on a case by case basis whether to propagate the exception), it also allows trivial backwards compatibility by registering a default "log and do nothing" listener.

          Show
          Niall Smart added a comment - The custom log4j appender suggestion is a temporary hack until a better bug fix is in place (in any case it assumes commons-logging is using log4j). In the longer term I would prefer to see the listener pattern used instead of just a simple flag: it allows the caller more flexibility (e.g., to decide on a case by case basis whether to propagate the exception), it also allows trivial backwards compatibility by registering a default "log and do nothing" listener.
          Hide
          Werner Guttmann added a comment -

          Niall, rest assured that you requested feature will make it into Castor (possibly maybe even with 1.0.6). I like the idea of creating a separate follow-up issue where we would discuss an approach to be taken, and subsequently sub-tasks for the implementation pieces.

          Show
          Werner Guttmann added a comment - Niall, rest assured that you requested feature will make it into Castor (possibly maybe even with 1.0.6). I like the idea of creating a separate follow-up issue where we would discuss an approach to be taken, and subsequently sub-tasks for the implementation pieces.
          Hide
          Edward Kuns added a comment -

          I just created CASTOR-1691 as suggested in http://jira.codehaus.org/browse/CASTOR-1642#action_80028 – it is linked to this issue.

          Show
          Edward Kuns added a comment - I just created CASTOR-1691 as suggested in http://jira.codehaus.org/browse/CASTOR-1642#action_80028 – it is linked to this issue.
          Hide
          Werner Guttmann added a comment -

          Thanks, Edward. Exactly what we need ....

          Show
          Werner Guttmann added a comment - Thanks, Edward. Exactly what we need ... .

            People

            • Assignee:
              Werner Guttmann
              Reporter:
              Paul Philion
            • Votes:
              1 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 1 hour, 15 minutes
                1h 15m
                Remaining:
                Remaining Estimate - 1 hour, 15 minutes
                1h 15m
                Logged:
                Time Spent - Not Specified
                Not Specified