killbill-memoizeit

Centralize exception handling for plugin calls thru PaymentPluginDispatcher.dispatchWithExceptionHandling(). At

5/2/2016 9:59:14 PM

Details

diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/control/OperationControlCallback.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/control/OperationControlCallback.java
index 22b0d54..9e09c24 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/control/OperationControlCallback.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/control/OperationControlCallback.java
@@ -18,8 +18,6 @@
 package org.killbill.billing.payment.core.sm.control;
 
 import java.util.List;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeoutException;
 
 import javax.annotation.Nullable;
 
@@ -47,12 +45,10 @@ import org.killbill.billing.payment.dispatcher.PluginDispatcher;
 import org.killbill.billing.payment.dispatcher.PluginDispatcher.PluginDispatcherReturnType;
 import org.killbill.billing.util.config.PaymentConfig;
 import org.killbill.commons.locker.GlobalLocker;
-import org.killbill.commons.locker.LockFailedException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Joiner;
-import com.google.common.base.MoreObjects;
 
 public abstract class OperationControlCallback extends OperationCallbackBase<Payment, PaymentApiException> implements OperationCallback {
 
@@ -149,33 +145,19 @@ public abstract class OperationControlCallback extends OperationCallbackBase<Pay
                     throw new OperationException(e, executePluginOnFailureCallsAndSetRetryDate(paymentControlContext));
                 } catch (final RuntimeException e) {
                     // Attempts to set the retry date in context if needed.
-                    executePluginOnFailureCallsAndSetRetryDate(paymentControlContext);
-                    throw new OperationException(e, OperationResult.EXCEPTION);
+                    throw new OperationException(e, executePluginOnFailureCallsAndSetRetryDate(paymentControlContext));
                 }
             }
         });
     }
 
     @Override
-    protected OperationException unwrapExceptionFromDispatchedTask(final Exception e) {
-        // If this is an ExecutionException we attempt to extract the cause first
-        final Throwable originalExceptionOrCausePossiblyOperationException = e instanceof ExecutionException ? MoreObjects.firstNonNull(e.getCause(), e) : e;
-
-        // Unwrap OperationException too (doOperationCallback wraps exceptions in OperationException)
-        final Throwable originalExceptionOrCause = originalExceptionOrCausePossiblyOperationException instanceof OperationException ? MoreObjects.firstNonNull(originalExceptionOrCausePossiblyOperationException.getCause(), originalExceptionOrCausePossiblyOperationException) : originalExceptionOrCausePossiblyOperationException;
-
-        if (originalExceptionOrCause instanceof OperationException) {
-            return (OperationException) originalExceptionOrCause;
-        } else if (originalExceptionOrCause instanceof LockFailedException) {
-            logger.warn("Failed to lock accountId='{}'", paymentStateContext.getAccount().getId());
-        } else if (originalExceptionOrCause instanceof TimeoutException) {
-            logger.warn("Call TIMEOUT for accountId='{}'", paymentStateContext.getAccount().getId());
-        } else if (originalExceptionOrCause instanceof InterruptedException) {
-            logger.warn("Call was interrupted for accountId='{}'", paymentStateContext.getAccount().getId());
-        } else {
-            logger.warn("Operation failed for accountId='{}'", paymentStateContext.getAccount().getId(), e);
+    protected OperationException unwrapExceptionFromDispatchedTask(final PaymentApiException e) {
+        if (e.getCause() instanceof OperationException) {
+            return (OperationException) e.getCause();
         }
-        return new OperationException(originalExceptionOrCause, getOperationResultOnException(paymentStateContext));
+        logger.warn("Operation failed for accountId='{}'", paymentStateContext.getAccount().getId(), e);
+        return new OperationException(e, getOperationResultOnException(paymentStateContext));
     }
 
     private OperationResult getOperationResultOnException(final PaymentStateContext paymentStateContext) {
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/OperationCallbackBase.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/OperationCallbackBase.java
index 1f52802..50c95a3 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/OperationCallbackBase.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/OperationCallbackBase.java
@@ -18,14 +18,14 @@
 package org.killbill.billing.payment.core.sm;
 
 import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeoutException;
 
 import org.killbill.automaton.OperationException;
 import org.killbill.automaton.OperationResult;
 import org.killbill.billing.account.api.Account;
+import org.killbill.billing.payment.api.PaymentApiException;
 import org.killbill.billing.payment.core.ProcessorBase.CallableWithAccountLock;
 import org.killbill.billing.payment.core.ProcessorBase.DispatcherCallback;
+import org.killbill.billing.payment.dispatcher.PaymentPluginDispatcher;
 import org.killbill.billing.payment.dispatcher.PluginDispatcher;
 import org.killbill.billing.payment.dispatcher.PluginDispatcher.PluginDispatcherReturnType;
 import org.killbill.billing.util.config.PaymentConfig;
@@ -68,17 +68,10 @@ public abstract class OperationCallbackBase<CallbackOperationResult, CallbackOpe
                                                                                                                                            paymentConfig,
                                                                                                                                            callback);
             logger.debug("Calling plugin(s) {}", pluginNames);
-            final OperationResult operationResult = paymentPluginDispatcher.dispatchWithTimeout(task);
+            final OperationResult operationResult = PaymentPluginDispatcher.dispatchWithExceptionHandling(account, pluginNames, task, paymentPluginDispatcher);
             logger.debug("Successful plugin(s) call of {} for account {} with result {}", pluginNames, account.getExternalKey(), operationResult);
             return operationResult;
-        } catch (final ExecutionException e) {
-            throw unwrapExceptionFromDispatchedTask(e);
-        } catch (final TimeoutException e) {
-            logger.warn("TimeoutException while executing the plugin(s) {}", pluginNames);
-            throw unwrapExceptionFromDispatchedTask(e);
-        } catch (final InterruptedException e) {
-            Thread.currentThread().interrupt();
-            logger.warn("InterruptedException while executing the following plugin(s): {}", pluginNames);
+        } catch (final PaymentApiException e) {
             throw unwrapExceptionFromDispatchedTask(e);
         }
     }
@@ -91,5 +84,5 @@ public abstract class OperationCallbackBase<CallbackOperationResult, CallbackOpe
     //
     protected abstract CallbackOperationResult doCallSpecificOperationCallback() throws CallbackOperationException;
 
-    protected abstract OperationException unwrapExceptionFromDispatchedTask(final Exception e);
+    protected abstract OperationException unwrapExceptionFromDispatchedTask(final PaymentApiException e);
 }
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentOperation.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentOperation.java
index e269932..ba20e1a 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentOperation.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentOperation.java
@@ -19,8 +19,6 @@ package org.killbill.billing.payment.core.sm.payments;
 
 import java.math.BigDecimal;
 import java.util.Iterator;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeoutException;
 
 import org.killbill.automaton.Operation.OperationCallback;
 import org.killbill.automaton.OperationException;
@@ -44,11 +42,9 @@ import org.killbill.billing.payment.plugin.api.PaymentTransactionInfoPlugin;
 import org.killbill.billing.payment.provider.DefaultNoOpPaymentInfoPlugin;
 import org.killbill.billing.util.config.PaymentConfig;
 import org.killbill.commons.locker.GlobalLocker;
-import org.killbill.commons.locker.LockFailedException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.base.MoreObjects;
 import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
@@ -86,35 +82,16 @@ public abstract class PaymentOperation extends OperationCallbackBase<PaymentTran
         } else {
             try {
                 return doSimpleOperationCallback();
-            } catch (final Exception e) {
-                // We need to unwrap OperationException (see doSimpleOperationCallback below)
-                throw unwrapExceptionFromDispatchedTask(e);
-            }
-        }
+	        } catch (final OperationException e) {
+    	        throw convertToUnknownTransactionStatusAndErroredPaymentState(e);
+        	}
+		}
     }
 
     @Override
-    protected OperationException unwrapExceptionFromDispatchedTask(final Exception e) {
-        // If this is an ExecutionException we attempt to extract the cause first
-        final Throwable originalExceptionOrCausePossiblyOperationException = e instanceof ExecutionException ? MoreObjects.firstNonNull(e.getCause(), e) : e;
-
-        // Unwrap OperationException too (doOperationCallback wraps exceptions in OperationException)
-        final Throwable originalExceptionOrCause = originalExceptionOrCausePossiblyOperationException instanceof OperationException ? MoreObjects.firstNonNull(originalExceptionOrCausePossiblyOperationException.getCause(), originalExceptionOrCausePossiblyOperationException) : originalExceptionOrCausePossiblyOperationException;
-
-        //
-        // Any case of exception (checked or runtime) should lead to a TransactionStatus.UNKNOWN (and a XXX_ERRORED payment state).
-        // In order to reach that state we create PaymentTransactionInfoPlugin with an PaymentPluginStatus.UNDEFINED status (and an OperationResult.EXCEPTION).
-        //
-        if (originalExceptionOrCause instanceof LockFailedException) {
-            logger.warn("Failed to lock accountExternalKey='{}'", paymentStateContext.getAccount().getExternalKey());
-        } else if (originalExceptionOrCause instanceof TimeoutException) {
-            logger.warn("Plugin call TIMEOUT for accountExternalKey='{}'", paymentStateContext.getAccount().getExternalKey());
-        } else if (originalExceptionOrCause instanceof InterruptedException) {
-            logger.warn("Plugin call was interrupted for accountExternalKey='{}'", paymentStateContext.getAccount().getExternalKey());
-        } else {
-            logger.warn("Payment plugin call threw an exception for accountExternalKey='{}'", paymentStateContext.getAccount().getExternalKey(), originalExceptionOrCause);
-        }
-        return convertToUnknownTransactionStatusAndErroredPaymentState(originalExceptionOrCause);
+    protected OperationException unwrapExceptionFromDispatchedTask(final PaymentApiException e) {
+        logger.warn("Payment plugin call threw an exception for accountExternalKey='{}'", paymentStateContext.getAccount().getExternalKey(), e);
+        return convertToUnknownTransactionStatusAndErroredPaymentState(e);
     }
 
     //
@@ -123,7 +100,7 @@ public abstract class PaymentOperation extends OperationCallbackBase<PaymentTran
     // - Construct a PaymentTransactionInfoPlugin whose PaymentPluginStatus = UNDEFINED to end up with a paymentTransactionStatus = UNKNOWN and have a chance to
     //   be fixed by Janitor.
     //
-    private OperationException convertToUnknownTransactionStatusAndErroredPaymentState(final Throwable e) {
+    private OperationException convertToUnknownTransactionStatusAndErroredPaymentState(final Exception e) {
 
         final PaymentTransactionInfoPlugin paymentInfoPlugin = new DefaultNoOpPaymentInfoPlugin(paymentStateContext.getPaymentId(),
                                                                                                 paymentStateContext.getTransactionId(),
@@ -136,6 +113,9 @@ public abstract class PaymentOperation extends OperationCallbackBase<PaymentTran
                                                                                                 null,
                                                                                                 null);
         paymentStateContext.setPaymentTransactionInfoPlugin(paymentInfoPlugin);
+        if (e instanceof OperationException) {
+            return (OperationException) e;
+        }
         return new OperationException(e, OperationResult.EXCEPTION);
     }
 
diff --git a/payment/src/main/java/org/killbill/billing/payment/dispatcher/PaymentPluginDispatcher.java b/payment/src/main/java/org/killbill/billing/payment/dispatcher/PaymentPluginDispatcher.java
index fec3c2e..7a4e1e2 100644
--- a/payment/src/main/java/org/killbill/billing/payment/dispatcher/PaymentPluginDispatcher.java
+++ b/payment/src/main/java/org/killbill/billing/payment/dispatcher/PaymentPluginDispatcher.java
@@ -64,7 +64,8 @@ public class PaymentPluginDispatcher {
                 log.warn(format);
                 throw new PaymentApiException(ErrorCode.PAYMENT_INTERNAL_ERROR, format);
             } else {
-                throw new PaymentApiException(e, ErrorCode.PAYMENT_INTERNAL_ERROR, MoreObjects.firstNonNull(e.getMessage(), ""));
+                // Unwraps the ExecutionException (e.getCause()), since it's a dispatch implementation detail
+                throw new PaymentApiException(e.getCause(), ErrorCode.PAYMENT_INTERNAL_ERROR, MoreObjects.firstNonNull(e.getMessage(), ""));
             }
         }
     }
diff --git a/payment/src/test/java/org/killbill/billing/payment/core/sm/MockRetryAuthorizeOperationCallback.java b/payment/src/test/java/org/killbill/billing/payment/core/sm/MockRetryAuthorizeOperationCallback.java
index 2a3a677..07f878b 100644
--- a/payment/src/test/java/org/killbill/billing/payment/core/sm/MockRetryAuthorizeOperationCallback.java
+++ b/payment/src/test/java/org/killbill/billing/payment/core/sm/MockRetryAuthorizeOperationCallback.java
@@ -65,6 +65,8 @@ public class MockRetryAuthorizeOperationCallback extends AuthorizeControlOperati
         if (exception != null) {
             if (exception instanceof PaymentApiException) {
                 throw (PaymentApiException) exception;
+            } else if (exception instanceof RuntimeException) {
+                throw (RuntimeException) exception;
             } else {
                 throw new RuntimeException(exception);
             }
diff --git a/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPluginOperation.java b/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPluginOperation.java
index 41155e8..a1a514f 100644
--- a/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPluginOperation.java
+++ b/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPluginOperation.java
@@ -57,7 +57,7 @@ public class TestPluginOperation extends PaymentTestSuiteNoDB {
 
     private static final String PLUGIN_NAME_PLACEHOLDER = "pluginName";
 
-    private static final int TIMEOUT = 5;
+    private static final int TIMEOUT = 10;
 
     private final GlobalLocker locker = new MemoryGlobalLocker();
     private final Account account = Mockito.mock(Account.class);
@@ -97,7 +97,8 @@ public class TestPluginOperation extends PaymentTestSuiteNoDB {
             Assert.fail();
         } catch (final OperationException e) {
             Assert.assertEquals(e.getOperationResult(), OperationResult.EXCEPTION);
-            Assert.assertTrue(e.getCause() instanceof NullPointerException);
+            Assert.assertTrue(e.getCause() instanceof PaymentApiException);
+            Assert.assertTrue(e.getCause().getCause() instanceof NullPointerException);
         }
     }