CAMEL-11229 Infinite recursion if exception hap... ...pens inside exception handler
A bit simpler unit test and reformatted source code. Project: http://git-wip-us.apache.org/repos/asf/camel/repo Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/02f41cdb Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/02f41cdb Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/02f41cdb Branch: refs/heads/camel-2.18.x Commit: 02f41cdbc9db51171e45f582fb733f6dba18fa18 Parents: 5491f48 Author: Zoran Regvart <zregv...@apache.org> Authored: Mon May 8 11:54:59 2017 +0200 Committer: Claus Ibsen <davscl...@apache.org> Committed: Thu May 11 18:41:43 2017 +0200 ---------------------------------------------------------------------- .../camel/processor/RedeliveryErrorHandler.java | 75 +++++++++++--------- .../onexception/OnExceptionRecursionTest.java | 48 ++----------- 2 files changed, 47 insertions(+), 76 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/camel/blob/02f41cdb/camel-core/src/main/java/org/apache/camel/processor/RedeliveryErrorHandler.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/processor/RedeliveryErrorHandler.java b/camel-core/src/main/java/org/apache/camel/processor/RedeliveryErrorHandler.java index 32a1bbe..7e978de 100644 --- a/camel-core/src/main/java/org/apache/camel/processor/RedeliveryErrorHandler.java +++ b/camel-core/src/main/java/org/apache/camel/processor/RedeliveryErrorHandler.java @@ -854,50 +854,55 @@ public abstract class RedeliveryErrorHandler extends ErrorHandlerSupport impleme Throwable origExceptionCaught = exchange.getProperty(Exchange.EXCEPTION_CAUGHT, Throwable.class); if (origExceptionCaught != null) { - log.error("Second Exception occured inside exception handler. Failing exchange", e); + log.error("Second Exception occurred inside exception handler. Failing exchange", e); exchange.setProperty(Exchange.UNIT_OF_WORK_EXHAUSTED, true); } else { // store the original caused exception in a property, so we can restore it later exchange.setProperty(Exchange.EXCEPTION_CAUGHT, e); - // find the error handler to use (if any) - OnExceptionDefinition exceptionPolicy = getExceptionPolicy(exchange, e); - if (exceptionPolicy != null) { - data.currentRedeliveryPolicy = exceptionPolicy.createRedeliveryPolicy(exchange.getContext(), data.currentRedeliveryPolicy); - data.handledPredicate = exceptionPolicy.getHandledPolicy(); - data.continuedPredicate = exceptionPolicy.getContinuedPolicy(); - data.retryWhilePredicate = exceptionPolicy.getRetryWhilePolicy(); - data.useOriginalInMessage = exceptionPolicy.getUseOriginalMessagePolicy() != null && exceptionPolicy.getUseOriginalMessagePolicy(); - - // route specific failure handler? - Processor processor = null; - UnitOfWork uow = exchange.getUnitOfWork(); - if (uow != null && uow.getRouteContext() != null) { - String routeId = uow.getRouteContext().getRoute().getId(); - processor = exceptionPolicy.getErrorHandler(routeId); - } else if (!exceptionPolicy.getErrorHandlers().isEmpty()) { - // note this should really not happen, but we have this code as a fail safe - // to be backwards compatible with the old behavior - log.warn("Cannot determine current route from Exchange with id: {}, will fallback and use first error handler.", exchange.getExchangeId()); - processor = exceptionPolicy.getErrorHandlers().iterator().next(); - } - if (processor != null) { - data.failureProcessor = processor; - } + // find the error handler to use (if any) + OnExceptionDefinition exceptionPolicy = getExceptionPolicy(exchange, e); + if (exceptionPolicy != null) { + data.currentRedeliveryPolicy = exceptionPolicy.createRedeliveryPolicy(exchange.getContext(), + data.currentRedeliveryPolicy); + data.handledPredicate = exceptionPolicy.getHandledPolicy(); + data.continuedPredicate = exceptionPolicy.getContinuedPolicy(); + data.retryWhilePredicate = exceptionPolicy.getRetryWhilePolicy(); + data.useOriginalInMessage = exceptionPolicy.getUseOriginalMessagePolicy() != null + && exceptionPolicy.getUseOriginalMessagePolicy(); + + // route specific failure handler? + Processor processor = null; + UnitOfWork uow = exchange.getUnitOfWork(); + if (uow != null && uow.getRouteContext() != null) { + String routeId = uow.getRouteContext().getRoute().getId(); + processor = exceptionPolicy.getErrorHandler(routeId); + } else if (!exceptionPolicy.getErrorHandlers().isEmpty()) { + // note this should really not happen, but we have this code + // as a fail safe + // to be backwards compatible with the old behavior + log.warn( + "Cannot determine current route from Exchange with id: {}, will fallback and use first error handler.", + exchange.getExchangeId()); + processor = exceptionPolicy.getErrorHandlers().iterator().next(); + } + if (processor != null) { + data.failureProcessor = processor; + } - // route specific on redelivery? - processor = exceptionPolicy.getOnRedelivery(); - if (processor != null) { - data.onRedeliveryProcessor = processor; - } - // route specific on exception occurred? - processor = exceptionPolicy.getOnExceptionOccurred(); - if (processor != null) { - data.onExceptionProcessor = processor; + // route specific on redelivery? + processor = exceptionPolicy.getOnRedelivery(); + if (processor != null) { + data.onRedeliveryProcessor = processor; + } + // route specific on exception occurred? + processor = exceptionPolicy.getOnExceptionOccurred(); + if (processor != null) { + data.onExceptionProcessor = processor; + } } } - } // only log if not failure handled or not an exhausted unit of work if (!ExchangeHelper.isFailureHandled(exchange) && !ExchangeHelper.isUnitOfWorkExhausted(exchange)) { http://git-wip-us.apache.org/repos/asf/camel/blob/02f41cdb/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionRecursionTest.java ---------------------------------------------------------------------- diff --git a/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionRecursionTest.java b/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionRecursionTest.java index 39c5c1b..a88e5f3 100644 --- a/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionRecursionTest.java +++ b/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionRecursionTest.java @@ -18,25 +18,17 @@ package org.apache.camel.processor.onexception; import org.apache.camel.CamelExecutionException; import org.apache.camel.ContextTestSupport; -import org.apache.camel.Exchange; -import org.apache.camel.Processor; import org.apache.camel.builder.RouteBuilder; -import org.junit.Assert; - - /** * Test that exceptions in an onException handler route do not go into recursion */ public class OnExceptionRecursionTest extends ContextTestSupport { - private int counter; - public void testRecursion() throws Exception { try { - template.sendBody("direct:start", "Hello World"); + template.sendBody("direct:test", "Hello World"); } catch (CamelExecutionException e) { - Throwable inner = e.getCause(); - Assert.assertEquals("Simulate exception in route", inner.getMessage()); + assertTrue("Simulate exception in route", e.getCause() instanceof IllegalStateException); } } @@ -45,41 +37,15 @@ public class OnExceptionRecursionTest extends ContextTestSupport { return new RouteBuilder() { @Override public void configure() throws Exception { - onException(MyException.class).process(new Processor() { - - @Override - public void process(Exchange exchange) throws Exception { - System.out.println(); - } - }).to("direct:exhandler"); - from("direct:exhandler").process(recursionProcessor()) - .throwException(new MyException("Simulate Exception in handler route")); - from("direct:start").throwException(new MyException("Simulate exception in route")); - } - - }; - - } + onException(Throwable.class).to("direct:handle"); - private Processor recursionProcessor() { - return new Processor() { - @Override - public void process(Exchange exchange) throws Exception { - counter++; - if (counter > 1) { - throw new IllegalStateException("Test failed as camel would go into recursion"); - } + from("direct:test").throwException(new IllegalStateException()).to("log:test"); + from("direct:handle").throwException(new NullPointerException()); } - }; - } - class MyException extends RuntimeException { - private static final long serialVersionUID = 1L; + }; - MyException(String msg) { - super(msg); - } - }; + } }
