WW-5414 Always call afterInvocation even in case of exception

apache · May 11, 2024 · 32f9b68 · 32f9b68
1 parent 649760d
commit 32f9b68
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 7 deletions.
diff --git a/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java b/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java
@@ -20,8 +20,11 @@
 
 import com.opensymphony.xwork2.ActionContext;
 import com.opensymphony.xwork2.ActionInvocation;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 import java.io.Serializable;
+import java.util.concurrent.ExecutionException;
 
 /**
  * Background thread to be executed by the ExecuteAndWaitInterceptor.
@@ -30,6 +33,8 @@ public class StrutsBackgroundProcess implements BackgroundProcess, Serializable
 
     private static final long serialVersionUID = 3884464776311686443L;
 
+    private static final Logger LOG = LogManager.getLogger(StrutsBackgroundProcess.class);
+
     private final String threadName;
     private final int threadPriority;
 
@@ -44,8 +49,8 @@ public class StrutsBackgroundProcess implements BackgroundProcess, Serializable
     /**
      * Constructs a background process
      *
-     * @param invocation The action invocation
-     * @param threadName The name of background thread
+     * @param invocation     The action invocation
+     * @param threadName     The name of background thread
      * @param threadPriority The priority of background thread
      */
     public StrutsBackgroundProcess(ActionInvocation invocation, String threadName, int threadPriority) {
@@ -61,11 +66,17 @@ public BackgroundProcess prepare() {
                 try {
                     beforeInvocation();
                     result = invocation.invokeActionOnly();
-                    afterInvocation();
                 } catch (Exception e) {
+                    LOG.warn("Exception during invokeActionOnly() execution", e);
                     exception = e;
                 } finally {
-                  done = true;
+                    try {
+                        afterInvocation();
+                    } catch (Exception ex) {
+                        exception = ex;
+                        LOG.warn("Exception during afterInvocation() execution", ex);
+                    }
+                    done = true;
                 }
             });
             processThread.setName(threadName);

diff --git a/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java b/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java
@@ -59,9 +59,9 @@ public void testSerializeDeserialize() throws Exception {
         invocation.setInvocationContext(ActionContext.getContext());
 
         StrutsBackgroundProcess bp = (StrutsBackgroundProcess) new StrutsBackgroundProcess(
-            invocation,
-            "BackgroundProcessTest.testSerializeDeserialize",
-            Thread.MIN_PRIORITY
+                invocation,
+                "BackgroundProcessTest.testSerializeDeserialize",
+                Thread.MIN_PRIORITY
         ).prepare();
         executor.execute(bp);
 
@@ -120,6 +120,33 @@ public void testMultipleProcesses() throws InterruptedException {
         assertEquals(100, mutableState.get());
     }
 
+    public void testErrorableProcesses1() throws InterruptedException {
+        MockActionInvocationWithActionInvoker invocation = new MockActionInvocationWithActionInvoker(() -> {
+            throw new IllegalStateException("boom");
+        });
+
+        BackgroundProcess bp = new ErrorableBackgroundProcess(invocation, null).prepare();
+        executor.execute(bp);
+
+        Thread.sleep(100);
+
+        assertTrue("afterInvocation not called in case of exception", ((ErrorableBackgroundProcess) bp).isDoneAfter());
+    }
+
+    public void testErrorableProcesses2() throws InterruptedException {
+        MockActionInvocationWithActionInvoker invocation = new MockActionInvocationWithActionInvoker(() -> {
+            throw new IllegalArgumentException("boom!");
+        });
+
+        IllegalStateException expected = new IllegalStateException("after!");
+        BackgroundProcess bp = new ErrorableBackgroundProcess(invocation, expected).prepare();
+        executor.execute(bp);
+
+        Thread.sleep(100);
+
+        assertEquals(expected, bp.getException());
+    }
+
     public void testUnpreparedProcess() throws ExecutionException, InterruptedException, TimeoutException {
         // given
         MockActionInvocationWithActionInvoker invocation = new MockActionInvocationWithActionInvoker(() -> "done");
@@ -177,3 +204,28 @@ protected void afterInvocation() throws Exception {
         lock.notify();
     }
 }
+
+class ErrorableBackgroundProcess extends StrutsBackgroundProcess {
+
+    private final Exception afterException;
+    private boolean doneAfter;
+
+    public ErrorableBackgroundProcess(ActionInvocation invocation, Exception afterException) {
+        super(invocation, "errorabale process", Thread.NORM_PRIORITY);
+        this.afterException = afterException;
+    }
+
+    @Override
+    protected void afterInvocation() throws Exception {
+        if (afterException != null) {
+            throw afterException;
+        } else {
+            super.afterInvocation();
+            doneAfter = true;
+        }
+    }
+
+    public boolean isDoneAfter() {
+        return doneAfter;
+    }
+}