Tapestry 5
  1. Tapestry 5
  2. TAP5-1963

Original exception lost in CommitAfterWorker upon abort

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 5.3.3
    • Fix Version/s: 5.3.4, 5.4
    • Component/s: tapestry-hibernate
    • Labels:
      None

      Description

      advise() in CommitAfterWorker surrounds the method invocation and session commit in a try/catch and runs manager.abort() if a RuntimeException is caught and then rethrows the RuntimeException. The problem is, depending on the original problem, it is not unlikely that manager.abort() itself could throw an exception (Transaction Not Started, Transaction Already Started, etc.). When that happens, the original RuntimeException is lost and all we get is the useless exception generated by the call to manager.abort().

      I think we should just throw away any exceptions generated by manager.abort() so that we always retain the original RuntimeException. Something like this:

      try

      { invocation.proceed(); // Success or checked exception: manager.commit(); }

      catch (RuntimeException ex)
      {
      try

      { manager.abort(); }

      catch (Exception e)

      { // throw away; we want the real "ex" exception to get rethrown }

      throw ex;
      }

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Closed Closed
        1d 6h 59m 1 Howard M. Lewis Ship 29/Jun/12 23:36
        Hide
        Hudson added a comment -

        Integrated in tapestry-trunk-freestyle #977 (See https://builds.apache.org/job/tapestry-trunk-freestyle/977/)
        TAP5-1963: Original exception lost in CommitAfterWorker upon abort (Revision 83e6d2edb1742977e72d084e9d750027c2f2c68d)

        Result = FAILURE
        hlship :
        Files :

        • tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/CommitAfterWorker.java
        • tapestry-jpa/src/main/java/org/apache/tapestry5/internal/jpa/CommitAfterMethodAdvice.java
        Show
        Hudson added a comment - Integrated in tapestry-trunk-freestyle #977 (See https://builds.apache.org/job/tapestry-trunk-freestyle/977/ ) TAP5-1963 : Original exception lost in CommitAfterWorker upon abort (Revision 83e6d2edb1742977e72d084e9d750027c2f2c68d) Result = FAILURE hlship : Files : tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/CommitAfterWorker.java tapestry-jpa/src/main/java/org/apache/tapestry5/internal/jpa/CommitAfterMethodAdvice.java
        Howard M. Lewis Ship made changes -
        Status Open [ 1 ] Closed [ 6 ]
        Assignee Howard M. Lewis Ship [ hlship ]
        Fix Version/s 5.3.4 [ 12320774 ]
        Fix Version/s 5.4 [ 12316401 ]
        Resolution Fixed [ 1 ]
        Ben Dotte made changes -
        Field Original Value New Value
        Description advise() in CommitAfterWorker surrounds the method invocation and session commit in a try/catch and runs manager.abort() if a RuntimeException is caught and then rethrows the RuntimeException. The problem is, depending on the original problem, it is not unlikely that manager.abort() itself could throw an exception (Transaction Not Started, Transaction Already Started, etc.). When that happens, the original RuntimeException is lost and all we get is the useless exception generated by the call to manager.abort().

        I think we should just throw away any exceptions generated by manager.abort() so that we always retain the original RuntimeException. Something like this:

        {code}
        try
        {
        invocation.proceed();
        // Success or checked exception:
        manager.commit();
        }
        catch (RuntimeException ex)
        {
        try
        {
        manager.abort();
        }
        catch (Exception e)
        {
        // throw away; we want the real "ex" exception to get rethrown
        }
        throw ex;
        }
        {code}
        advise() in CommitAfterWorker surrounds the method invocation and session commit in a try/catch and runs manager.abort() if a RuntimeException is caught and then rethrows the RuntimeException. The problem is, depending on the original problem, it is not unlikely that manager.abort() itself could throw an exception (Transaction Not Started, Transaction Already Started, etc.). When that happens, the original RuntimeException is lost and all we get is the useless exception generated by the call to manager.abort().

        I think we should just throw away any exceptions generated by manager.abort() so that we always retain the original RuntimeException. Something like this:

        try
        {
        invocation.proceed();
        // Success or checked exception:
        manager.commit();
        }
        catch (RuntimeException ex)
        {
        try
        {
        manager.abort();
        }
        catch (Exception e)
        {
        // throw away; we want the real "ex" exception to get rethrown
        }
        throw ex;
        }
        Ben Dotte created issue -

          People

          • Assignee:
            Howard M. Lewis Ship
            Reporter:
            Ben Dotte
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development