From 08dedb822cb1f8df2e1ba263bc50eac31bff0e37 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 31 Aug 2010 18:31:11 +0000 Subject: [PATCH] SWF-1381 A couple of fixes for recursive exception handling --- .../engine/impl/FlowExecutionImpl.java | 22 ++++++- .../engine/impl/FlowExecutionImplTests.java | 63 +++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/spring-webflow/src/main/java/org/springframework/webflow/engine/impl/FlowExecutionImpl.java b/spring-webflow/src/main/java/org/springframework/webflow/engine/impl/FlowExecutionImpl.java index a9e07d7a..7e1bbf31 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/engine/impl/FlowExecutionImpl.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/engine/impl/FlowExecutionImpl.java @@ -583,7 +583,26 @@ public class FlowExecutionImpl implements FlowExecution, Externalizable { */ private void handleException(FlowExecutionException exception, RequestControlContext context) throws FlowExecutionException { + handleException(exception, context, 1); + } + + /** + * This is an overloaded method for {@link #handleException(FlowExecutionException, RequestControlContext)} adding a + * handleExceptionCount argument that can be used to prevent infinite recursion. + * + * @param exception the exception that occurred + * @param context the request control context the exception occurred in + * @param handleExceptionCount the number of recursive attempts made at exception handling + */ + private void handleException(FlowExecutionException exception, RequestControlContext context, + int handleExceptionCount) { listeners.fireExceptionThrown(context, exception); + if (handleExceptionCount > 5) { + if (logger.isDebugEnabled()) { + logger.debug("Exception not handled after 5 tries, aborting exception handling of [" + exception + "]"); + } + throw exception; + } if (logger.isDebugEnabled()) { if (exception.getCause() != null) { logger.debug("Attempting to handle [" + exception + "] with root cause [" + getRootCause(exception) @@ -602,7 +621,8 @@ public class FlowExecutionImpl implements FlowExecution, Externalizable { } } catch (FlowExecutionException newException) { // exception handling itself resulted in a new FlowExecutionException, try to handle it - handleException(newException, context); + handleException(newException, context, handleExceptionCount + 1); + handled = true; } if (!handled) { if (logger.isDebugEnabled()) { diff --git a/spring-webflow/src/test/java/org/springframework/webflow/engine/impl/FlowExecutionImplTests.java b/spring-webflow/src/test/java/org/springframework/webflow/engine/impl/FlowExecutionImplTests.java index 15b47a2e..c570f555 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/engine/impl/FlowExecutionImplTests.java +++ b/spring-webflow/src/test/java/org/springframework/webflow/engine/impl/FlowExecutionImplTests.java @@ -254,6 +254,41 @@ public class FlowExecutionImplTests extends TestCase { assertTrue(exceptionHandler.getHandled()); } + public void testExceptionHandledByNestedExceptionHandler() { + Flow flow = new Flow("flow"); + ExceptionThrowingExceptionHandler exceptionHandler = new ExceptionThrowingExceptionHandler(true); + flow.getExceptionHandlerSet().add(exceptionHandler); + new State(flow, "state") { + protected void doEnter(RequestControlContext context) throws FlowExecutionException { + throw new FlowExecutionException("flow", "state", "Oops"); + } + }; + FlowExecutionImpl execution = new FlowExecutionImpl(flow); + MockExternalContext context = new MockExternalContext(); + assertFalse(execution.hasStarted()); + execution.start(null, context); + assertEquals(2, exceptionHandler.getHandleCount()); + } + + public void testExceptionHandledAvoidEndlessRecursion() { + Flow flow = new Flow("flow"); + ExceptionThrowingExceptionHandler exceptionHandler = new ExceptionThrowingExceptionHandler(false); + flow.getExceptionHandlerSet().add(exceptionHandler); + new State(flow, "state") { + protected void doEnter(RequestControlContext context) throws FlowExecutionException { + throw new FlowExecutionException("flow", "state", "Oops"); + } + }; + FlowExecutionImpl execution = new FlowExecutionImpl(flow); + MockExternalContext context = new MockExternalContext(); + try { + execution.start(null, context); + fail("Should have aborted exception handling after 5 tries"); + } catch (FlowExecutionException e) { + assertEquals(5, exceptionHandler.handleCount); + } + } + public void testStartCannotCallTwice() { Flow flow = new Flow("flow"); new EndState(flow, "end"); @@ -435,4 +470,32 @@ public class FlowExecutionImplTests extends TestCase { } + private static class ExceptionThrowingExceptionHandler implements FlowExecutionExceptionHandler { + + private boolean throwOnlyOnce = true; + private int handleCount; + + public ExceptionThrowingExceptionHandler(boolean throwOnlyOnce) { + this.throwOnlyOnce = throwOnlyOnce; + } + + public int getHandleCount() { + return handleCount; + } + + public boolean canHandle(FlowExecutionException exception) { + return true; + } + + public void handle(FlowExecutionException exception, RequestControlContext context) { + this.handleCount++; + if (throwOnlyOnce && "nested exception".equals(exception.getMessage())) { + // No more exceptions + } else { + throw new FlowExecutionException(exception.getFlowId(), exception.getStateId(), "nested exception"); + } + } + + } + } \ No newline at end of file