Mule

Exception strategy invoked twice on TransactionTemplate

Details

  • User impact:
    Low
  • Similar Issues:
    MULE-4955 Refactor Exception Strategy invocation so we don't get exception strategies invoked twice
    MULE-2313 Exception Strategy logs error twice
    MULE-1404 DefaultExceptionStrategy is invoked twice
    MULE-5837 Error at startup when a sub-flow is invoked twice via flow-ref
    MULE-5086 An error message is logged when an exception strategy is invoked for a flow or configuration pattern
    MULE-4233 Quartz connector ignores exception strategy
    MULE-4339 Exception Handler is called twice for the same error
    MULE-5969 Refactor code to use TransactionTemplateFactory method and exception re-thrown strategy
    MULE-4927 Exception strategy invoked both for connector *and* service exception strategies when exception on sync outbound endpoint
    MULE-4821 Exception message sent twice to the outbound endpoint defined in the exception strategy

Description

The exceptions handled by the TransactionTemplate can be doubly handled if, despite having an ExceptionListener, they are not in a transaction.

Issue Links

Activity

Hide
Edu Pereda added a comment - 30/Jun/10 11:34 AM

This issue was detected on EE-1968.

Show
Edu Pereda added a comment - 30/Jun/10 11:34 AM This issue was detected on EE-1968.
Hide
Edu Pereda added a comment - 30/Jun/10 11:38 AM

This is a proposed fix that would avoid the double call to the exception handler.

It means that the TransactionTemplate will only handle the exception if it has an exceptionListener AND it has a transaction, otherwise, the exception is rethrown. If it is handled, then it won't be rethrown.

It was committed on http://fisheye.codehaus.org/changelog/mule/?cs=17753 for 2.2.x

Index: core/src/main/java/org/mule/transaction/TransactionTemplate.java
===================================================================
--- core/src/main/java/org/mule/transaction/TransactionTemplate.java	(revision 17734)
+++ core/src/main/java/org/mule/transaction/TransactionTemplate.java	(working copy)
@@ -10,8 +10,6 @@
 
 package org.mule.transaction;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.mule.api.MuleContext;
 import org.mule.api.transaction.ExternalTransactionAwareTransactionFactory;
 import org.mule.api.transaction.Transaction;
@@ -23,6 +21,9 @@
 
 import java.beans.ExceptionListener;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
 public class TransactionTemplate
 {
     private static final Log logger = LogFactory.getLog(TransactionTemplate.class);
@@ -130,7 +131,7 @@
         catch (Exception e)
         {
             tx = TransactionCoordination.getInstance().getTransaction();
-            if (exceptionListener != null)
+            if (isExceptionHandledAtThisLevel(tx))
             {
                 logger.info("Exception Caught in Transaction template.  Handing off to exception handler: "
                     + exceptionListener);
@@ -163,9 +164,10 @@
                 // the context delimited by XA's ALWAYS_BEGIN
                 return null;
             }
-            else if (exceptionListener != null && tx != null)
+            else if (isExceptionHandledAtThisLevel(tx))
             {
-                // if there's an exception listener, it has been handled already, don't loop
+                // if exception is handled at this level, it has been handled
+                // already, don't loop
                 return null;
             }
             else
@@ -189,6 +191,18 @@
         }
     }
 
+    /**
+     * The exception must be handled at this level if there is an
+     * {@link #exceptionListener} and there is a transaction.
+     * 
+     * @param tx
+     * @return
+     */
+    protected boolean isExceptionHandledAtThisLevel(Transaction tx)
+    {
+        return exceptionListener != null && tx != null;
+    }
+
     protected void resolveTransaction(Transaction tx) throws TransactionException
     {
         if (tx.isRollbackOnly())
Show
Edu Pereda added a comment - 30/Jun/10 11:38 AM This is a proposed fix that would avoid the double call to the exception handler. It means that the TransactionTemplate will only handle the exception if it has an exceptionListener AND it has a transaction, otherwise, the exception is rethrown. If it is handled, then it won't be rethrown. It was committed on http://fisheye.codehaus.org/changelog/mule/?cs=17753 for 2.2.x
Index: core/src/main/java/org/mule/transaction/TransactionTemplate.java
===================================================================
--- core/src/main/java/org/mule/transaction/TransactionTemplate.java	(revision 17734)
+++ core/src/main/java/org/mule/transaction/TransactionTemplate.java	(working copy)
@@ -10,8 +10,6 @@
 
 package org.mule.transaction;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.mule.api.MuleContext;
 import org.mule.api.transaction.ExternalTransactionAwareTransactionFactory;
 import org.mule.api.transaction.Transaction;
@@ -23,6 +21,9 @@
 
 import java.beans.ExceptionListener;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
 public class TransactionTemplate
 {
     private static final Log logger = LogFactory.getLog(TransactionTemplate.class);
@@ -130,7 +131,7 @@
         catch (Exception e)
         {
             tx = TransactionCoordination.getInstance().getTransaction();
-            if (exceptionListener != null)
+            if (isExceptionHandledAtThisLevel(tx))
             {
                 logger.info("Exception Caught in Transaction template.  Handing off to exception handler: "
                     + exceptionListener);
@@ -163,9 +164,10 @@
                 // the context delimited by XA's ALWAYS_BEGIN
                 return null;
             }
-            else if (exceptionListener != null && tx != null)
+            else if (isExceptionHandledAtThisLevel(tx))
             {
-                // if there's an exception listener, it has been handled already, don't loop
+                // if exception is handled at this level, it has been handled
+                // already, don't loop
                 return null;
             }
             else
@@ -189,6 +191,18 @@
         }
     }
 
+    /**
+     * The exception must be handled at this level if there is an
+     * {@link #exceptionListener} and there is a transaction.
+     * 
+     * @param tx
+     * @return
+     */
+    protected boolean isExceptionHandledAtThisLevel(Transaction tx)
+    {
+        return exceptionListener != null && tx != null;
+    }
+
     protected void resolveTransaction(Transaction tx) throws TransactionException
     {
         if (tx.isRollbackOnly())
Hide
Edu Pereda added a comment - 02/Jul/10 08:21 AM
Show
Edu Pereda added a comment - 02/Jul/10 08:21 AM Added this small change: http://fisheye.codehaus.org/changelog/mule/?cs=17812
Hide
Edu Pereda added a comment - 15/Jul/10 09:51 AM

This is fixed on 2.2.x (it will be out in 2.2.6).

I am not closing this until we merge it to 3.x.

Show
Edu Pereda added a comment - 15/Jul/10 09:51 AM This is fixed on 2.2.x (it will be out in 2.2.6). I am not closing this until we merge it to 3.x.
Hide
Edu Pereda added a comment - 15/Sep/10 03:02 PM
Show
Edu Pereda added a comment - 15/Sep/10 03:02 PM Merged to 3.x: http://fisheye.codehaus.org/changelog/mule/?cs=17830 Closing.

People

Vote (0)
Watch (0)

Dates

  • Created:
    30/Jun/10 11:33 AM
    Updated:
    15/Sep/10 03:02 PM
    Resolved:
    15/Sep/10 03:02 PM