CAMEL-11229: Add fail-safe in fatal error handler to detect circular looping
Project: http://git-wip-us.apache.org/repos/asf/camel/repo Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/a08cb431 Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/a08cb431 Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/a08cb431 Branch: refs/heads/camel-2.18.x Commit: a08cb431141e808f5bab78deac68966e2e770d47 Parents: 02f41cd Author: Claus Ibsen <davscl...@apache.org> Authored: Wed May 10 13:10:57 2017 +0200 Committer: Claus Ibsen <davscl...@apache.org> Committed: Thu May 11 18:41:49 2017 +0200 ---------------------------------------------------------------------- .../main/java/org/apache/camel/Exchange.java | 1 + .../processor/FatalFallbackErrorHandler.java | 119 ++++++-- .../camel/processor/RedeliveryErrorHandler.java | 100 +++---- ...adLetterChannelRestartFromBeginningTest.java | 3 +- .../OnExceptionGlobalScopedRecursionTest.java | 270 +++++++++++++++++++ .../onexception/OnExceptionRecursionTest.java | 81 +++++- .../OnExceptionRouteScopedRecursionTest.java | 262 ++++++++++++++++++ 7 files changed, 750 insertions(+), 86 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/camel/blob/a08cb431/camel-core/src/main/java/org/apache/camel/Exchange.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/Exchange.java b/camel-core/src/main/java/org/apache/camel/Exchange.java index 0a00d18..b1636bc 100644 --- a/camel-core/src/main/java/org/apache/camel/Exchange.java +++ b/camel-core/src/main/java/org/apache/camel/Exchange.java @@ -119,6 +119,7 @@ public interface Exchange { String FAILURE_HANDLED = "CamelFailureHandled"; String FAILURE_ENDPOINT = "CamelFailureEndpoint"; String FAILURE_ROUTE_ID = "CamelFailureRouteId"; + String FATAL_FALLBACK_ERROR_HANDLER = "CamelFatalFallbackErrorHandler"; String FILE_CONTENT_TYPE = "CamelFileContentType"; String FILE_LOCAL_WORK_PATH = "CamelFileLocalWorkPath"; String FILE_NAME = "CamelFileName"; http://git-wip-us.apache.org/repos/asf/camel/blob/a08cb431/camel-core/src/main/java/org/apache/camel/processor/FatalFallbackErrorHandler.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/processor/FatalFallbackErrorHandler.java b/camel-core/src/main/java/org/apache/camel/processor/FatalFallbackErrorHandler.java index 75c02c5..e04c15b 100644 --- a/camel-core/src/main/java/org/apache/camel/processor/FatalFallbackErrorHandler.java +++ b/camel-core/src/main/java/org/apache/camel/processor/FatalFallbackErrorHandler.java @@ -16,6 +16,8 @@ */ package org.apache.camel.processor; +import java.util.Stack; + import org.apache.camel.AsyncCallback; import org.apache.camel.Exchange; import org.apache.camel.Processor; @@ -25,6 +27,10 @@ import org.slf4j.LoggerFactory; /** * An {@link org.apache.camel.processor.ErrorHandler} used as a safe fallback when * processing by other error handlers such as the {@link org.apache.camel.model.OnExceptionDefinition}. + * <p/> + * This error handler is used as a fail-safe to ensure that error handling does not run in endless recursive looping + * which potentially can happen if a new exception is thrown while error handling a previous exception which then + * cause new error handling to process and this then keep on failing with new exceptions in an endless loop. * * @version */ @@ -44,44 +50,99 @@ public class FatalFallbackErrorHandler extends DelegateAsyncProcessor implements } @Override + @SuppressWarnings("unchecked") public boolean process(final Exchange exchange, final AsyncCallback callback) { + // prevent endless looping if we end up coming back to ourself + Stack<Processor> fatals = exchange.getProperty(Exchange.FATAL_FALLBACK_ERROR_HANDLER, null, Stack.class); + if (fatals == null) { + fatals = new Stack<>(); + exchange.setProperty(Exchange.FATAL_FALLBACK_ERROR_HANDLER, fatals); + } + if (fatals.search(this) > -1) { + LOG.warn("Circular error-handler detected - breaking out"); + // mark this exchange as already been error handler handled (just by having this property) + // the false value mean the caught exception will be kept on the exchange, causing the + // exception to be propagated back to the caller, and to break out routing + exchange.setProperty(Exchange.ERRORHANDLER_HANDLED, false); + callback.done(true); + return true; + } + + // okay we run under this fatal error handler now + final Processor fatal = this; + fatals.push(fatal); + // support the asynchronous routing engine boolean sync = processor.process(exchange, new AsyncCallback() { public void done(boolean doneSync) { - if (exchange.getException() != null) { - // an exception occurred during processing onException + try { + if (exchange.getException() != null) { + // an exception occurred during processing onException + + // log detailed error message with as much detail as possible + Throwable previous = exchange.getProperty(Exchange.EXCEPTION_CAUGHT, Throwable.class); - // log detailed error message with as much detail as possible - Throwable previous = exchange.getProperty(Exchange.EXCEPTION_CAUGHT, Throwable.class); - String msg = "Exception occurred while trying to handle previously thrown exception on exchangeId: " + // check if previous and this exception are set as the same exception + // which happens when using global scoped onException and you call a direct route that causes the 2nd exception + // then we need to find the original previous exception as the suppressed exception + if (previous != null && previous == exchange.getException()) { + previous = null; + // maybe previous was suppressed? + if (exchange.getException().getSuppressed().length > 0) { + previous = exchange.getException().getSuppressed()[0]; + } + } + + String msg = "Exception occurred while trying to handle previously thrown exception on exchangeId: " + exchange.getExchangeId() + " using: [" + processor + "]."; - if (previous != null) { - msg += " The previous and the new exception will be logged in the following."; - log(msg); - log("\\--> Previous exception on exchangeId: " + exchange.getExchangeId(), previous); - log("\\--> New exception on exchangeId: " + exchange.getExchangeId(), exchange.getException()); - } else { - log(msg); - log("\\--> New exception on exchangeId: " + exchange.getExchangeId(), exchange.getException()); - } + if (previous != null) { + msg += " The previous and the new exception will be logged in the following."; + log(msg); + log("\\--> Previous exception on exchangeId: " + exchange.getExchangeId(), previous); + log("\\--> New exception on exchangeId: " + exchange.getExchangeId(), exchange.getException()); + } else { + log(msg); + log("\\--> New exception on exchangeId: " + exchange.getExchangeId(), exchange.getException()); + } + + // add previous as suppressed to exception if not already there + if (previous != null) { + Throwable[] suppressed = exchange.getException().getSuppressed(); + boolean found = false; + for (Throwable t : suppressed) { + if (t == previous) { + found = true; + } + } + if (!found) { + exchange.getException().addSuppressed(previous); + } + } - // we can propagated that exception to the caught property on the exchange - // which will shadow any previously caught exception and cause this new exception - // to be visible in the error handler - exchange.setProperty(Exchange.EXCEPTION_CAUGHT, exchange.getException()); - - if (deadLetterChannel) { - // special for dead letter channel as we want to let it determine what to do, depending how - // it has been configured - exchange.removeProperty(Exchange.ERRORHANDLER_HANDLED); - } else { - // mark this exchange as already been error handler handled (just by having this property) - // the false value mean the caught exception will be kept on the exchange, causing the - // exception to be propagated back to the caller, and to break out routing - exchange.setProperty(Exchange.ERRORHANDLER_HANDLED, false); + // we can propagated that exception to the caught property on the exchange + // which will shadow any previously caught exception and cause this new exception + // to be visible in the error handler + exchange.setProperty(Exchange.EXCEPTION_CAUGHT, exchange.getException()); + + if (deadLetterChannel) { + // special for dead letter channel as we want to let it determine what to do, depending how + // it has been configured + exchange.removeProperty(Exchange.ERRORHANDLER_HANDLED); + } else { + // mark this exchange as already been error handler handled (just by having this property) + // the false value mean the caught exception will be kept on the exchange, causing the + // exception to be propagated back to the caller, and to break out routing + exchange.setProperty(Exchange.ERRORHANDLER_HANDLED, false); + } + } + } finally { + // no longer running under this fatal fallback error handler + Stack<Processor> fatals = exchange.getProperty(Exchange.FATAL_FALLBACK_ERROR_HANDLER, null, Stack.class); + if (fatals != null) { + fatals.remove(fatal); } + callback.done(doneSync); } - callback.done(doneSync); } }); http://git-wip-us.apache.org/repos/asf/camel/blob/a08cb431/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 7e978de..c0dd16e 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 @@ -851,56 +851,62 @@ public abstract class RedeliveryErrorHandler extends ErrorHandlerSupport impleme protected void handleException(Exchange exchange, RedeliveryData data, boolean isDeadLetterChannel) { Exception e = exchange.getException(); + // e is never null + + Throwable previous = exchange.getProperty(Exchange.EXCEPTION_CAUGHT, Throwable.class); + if (previous != null && previous != e) { + // a 2nd exception was thrown while handling a previous exception + // so we need to add the previous as suppressed by the new exception + // see also FatalFallbackErrorHandler + Throwable[] suppressed = e.getSuppressed(); + boolean found = false; + for (Throwable t : suppressed) { + if (t == previous) { + found = true; + } + } + if (!found) { + e.addSuppressed(previous); + } + } - Throwable origExceptionCaught = exchange.getProperty(Exchange.EXCEPTION_CAUGHT, Throwable.class); - if (origExceptionCaught != null) { - 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); - // 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 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 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; } } http://git-wip-us.apache.org/repos/asf/camel/blob/a08cb431/camel-core/src/test/java/org/apache/camel/processor/DeadLetterChannelRestartFromBeginningTest.java ---------------------------------------------------------------------- diff --git a/camel-core/src/test/java/org/apache/camel/processor/DeadLetterChannelRestartFromBeginningTest.java b/camel-core/src/test/java/org/apache/camel/processor/DeadLetterChannelRestartFromBeginningTest.java index e1aab51..5d6caa7 100644 --- a/camel-core/src/test/java/org/apache/camel/processor/DeadLetterChannelRestartFromBeginningTest.java +++ b/camel-core/src/test/java/org/apache/camel/processor/DeadLetterChannelRestartFromBeginningTest.java @@ -43,7 +43,8 @@ public class DeadLetterChannelRestartFromBeginningTest extends ContextTestSuppor // use fire and forget template.sendBody("seda:start", "Camel"); - setAssertPeriod(200); + setAssertPeriod(500); + assertMockEndpointsSatisfied(); } http://git-wip-us.apache.org/repos/asf/camel/blob/a08cb431/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionGlobalScopedRecursionTest.java ---------------------------------------------------------------------- diff --git a/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionGlobalScopedRecursionTest.java b/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionGlobalScopedRecursionTest.java new file mode 100644 index 0000000..417d4fa --- /dev/null +++ b/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionGlobalScopedRecursionTest.java @@ -0,0 +1,270 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.processor.onexception; + +import org.apache.camel.CamelExecutionException; +import org.apache.camel.ContextTestSupport; +import org.apache.camel.builder.RouteBuilder; + +/** + * Test that exceptions in an onException handler route do not go into recursion + */ +public class OnExceptionGlobalScopedRecursionTest extends ContextTestSupport { + + @Override + public boolean isUseRouteBuilder() { + return false; + } + + public void testRecursion() throws Exception { + getMockEndpoint("mock:a").expectedMessageCount(1); + getMockEndpoint("mock:b").expectedMessageCount(0); + getMockEndpoint("mock:c").expectedMessageCount(1); + + context.addRoutes(new RouteBuilder() { + @Override + public void configure() throws Exception { + onException(Throwable.class) + .to("mock:c") + .log("onException") + .throwException(new NullPointerException("A NPE error here")) + .end(); + + from("direct:test") + .to("mock:a") + .log("test") + .throwException(new IllegalStateException("Bad state")).to("log:test") + .to("mock:b"); + } + }); + context.start(); + + try { + template.sendBody("direct:test", "Hello World"); + fail("Should have thrown exception"); + } catch (CamelExecutionException e) { + NullPointerException npe = assertIsInstanceOf(NullPointerException.class, e.getCause()); + assertEquals("A NPE error here", npe.getMessage()); + + IllegalStateException ise = assertIsInstanceOf(IllegalStateException.class, npe.getSuppressed()[0]); + assertEquals("Bad state", ise.getMessage()); + } + } + + public void testRecursionHandled() throws Exception { + getMockEndpoint("mock:a").expectedMessageCount(1); + getMockEndpoint("mock:b").expectedMessageCount(0); + getMockEndpoint("mock:c").expectedMessageCount(1); + + context.addRoutes(new RouteBuilder() { + @Override + public void configure() throws Exception { + onException(Throwable.class) + .handled(true) + .to("mock:c") + .log("onException") + .throwException(new NullPointerException("A NPE error here")) + .end(); + + from("direct:test") + .to("mock:a") + .log("test") + .throwException(new IllegalStateException("Bad state")).to("log:test") + .to("mock:b"); + } + }); + context.start(); + + try { + template.sendBody("direct:test", "Hello World"); + fail("Should have thrown exception"); + } catch (CamelExecutionException e) { + NullPointerException npe = assertIsInstanceOf(NullPointerException.class, e.getCause()); + assertEquals("A NPE error here", npe.getMessage()); + + IllegalStateException ise = assertIsInstanceOf(IllegalStateException.class, npe.getSuppressed()[0]); + assertEquals("Bad state", ise.getMessage()); + } + } + + public void testRecursionDirectNoErrorHandler() throws Exception { + getMockEndpoint("mock:a").expectedMessageCount(1); + getMockEndpoint("mock:b").expectedMessageCount(0); + getMockEndpoint("mock:c").expectedMessageCount(1); + getMockEndpoint("mock:d").expectedMessageCount(1); + + context.addRoutes(new RouteBuilder() { + @Override + public void configure() throws Exception { + onException(Throwable.class) + .to("mock:c") + .log("onException") + .to("direct:error") + .end(); + + from("direct:test") + .to("mock:a") + .log("test") + .throwException(new IllegalStateException("Bad state")).to("log:test") + .to("mock:b"); + + // need to turn off error handler when linked with direct, in case you want the same as inlined + from("direct:error").errorHandler(noErrorHandler()) + .to("mock:d") + .log("error") + .throwException(new NullPointerException("A NPE error here")); + } + }); + context.start(); + + try { + template.sendBody("direct:test", "Hello World"); + fail("Should have thrown exception"); + } catch (CamelExecutionException e) { + NullPointerException npe = assertIsInstanceOf(NullPointerException.class, e.getCause()); + assertEquals("A NPE error here", npe.getMessage()); + + IllegalStateException ise = assertIsInstanceOf(IllegalStateException.class, npe.getSuppressed()[0]); + assertEquals("Bad state", ise.getMessage()); + } + } + + public void testRecursionHandledDirectNoErrorHandler() throws Exception { + getMockEndpoint("mock:a").expectedMessageCount(1); + getMockEndpoint("mock:b").expectedMessageCount(0); + getMockEndpoint("mock:c").expectedMessageCount(1); + getMockEndpoint("mock:d").expectedMessageCount(1); + + context.addRoutes(new RouteBuilder() { + @Override + public void configure() throws Exception { + onException(Throwable.class) + .handled(true) + .to("mock:c") + .log("onException") + .to("direct:error") + .end(); + + from("direct:test") + .to("mock:a") + .log("test") + .throwException(new IllegalStateException("Bad state")).to("log:test") + .to("mock:b"); + + // need to turn off error handler when linked with direct, in case you want the same as inlined + from("direct:error").errorHandler(noErrorHandler()) + .to("mock:d") + .log("error") + .throwException(new NullPointerException("A NPE error here")); + } + }); + context.start(); + + try { + template.sendBody("direct:test", "Hello World"); + fail("Should have thrown exception"); + } catch (CamelExecutionException e) { + NullPointerException npe = assertIsInstanceOf(NullPointerException.class, e.getCause()); + assertEquals("A NPE error here", npe.getMessage()); + + IllegalStateException ise = assertIsInstanceOf(IllegalStateException.class, npe.getSuppressed()[0]); + assertEquals("Bad state", ise.getMessage()); + } + } + + public void testRecursionDirect() throws Exception { + getMockEndpoint("mock:a").expectedMessageCount(1); + getMockEndpoint("mock:b").expectedMessageCount(0); + getMockEndpoint("mock:c").expectedMessageCount(1); + getMockEndpoint("mock:d").expectedMessageCount(1); + + context.addRoutes(new RouteBuilder() { + @Override + public void configure() throws Exception { + onException(Throwable.class) + .to("mock:c") + .log("onException") + .to("direct:error") + .end(); + + from("direct:test") + .to("mock:a") + .log("test") + .throwException(new IllegalStateException("Bad state")).to("log:test") + .to("mock:b"); + + from("direct:error") + .to("mock:d") + .log("error") + .throwException(new NullPointerException("A NPE error here")); + } + }); + context.start(); + + try { + template.sendBody("direct:test", "Hello World"); + fail("Should have thrown exception"); + } catch (CamelExecutionException e) { + NullPointerException npe = assertIsInstanceOf(NullPointerException.class, e.getCause()); + assertEquals("A NPE error here", npe.getMessage()); + // we can only see the NPE from the direct route + } + } + + public void testRecursionHandledDirect() throws Exception { + getMockEndpoint("mock:a").expectedMessageCount(1); + getMockEndpoint("mock:b").expectedMessageCount(0); + getMockEndpoint("mock:c").expectedMessageCount(1); + getMockEndpoint("mock:d").expectedMessageCount(1); + + context.addRoutes(new RouteBuilder() { + @Override + public void configure() throws Exception { + onException(Throwable.class) + .handled(true) + .to("mock:c") + .log("onException") + .to("direct:error") + .end(); + + from("direct:test") + .to("mock:a") + .log("test") + .throwException(new IllegalStateException("Bad state")).to("log:test") + .to("mock:b"); + + from("direct:error") + .to("mock:d") + .log("error") + .throwException(new NullPointerException("A NPE error here")); + } + }); + context.start(); + + // TODO: figure out why it does not throw exception (handle = true, new exception -> handle true?) + // TODO: and why route scoped seems to work + try { + template.sendBody("direct:test", "Hello World"); + fail("Should have thrown exception"); + } catch (CamelExecutionException e) { + NullPointerException npe = assertIsInstanceOf(NullPointerException.class, e.getCause()); + assertEquals("A NPE error here", npe.getMessage()); + // we can only see the NPE from the direct route + } + } + +} http://git-wip-us.apache.org/repos/asf/camel/blob/a08cb431/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 a88e5f3..bd12b0d 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 @@ -24,28 +24,91 @@ import org.apache.camel.builder.RouteBuilder; * Test that exceptions in an onException handler route do not go into recursion */ public class OnExceptionRecursionTest extends ContextTestSupport { - public void testRecursion() throws Exception { + + @Override + public boolean isUseRouteBuilder() { + return false; + } + + public void testRecursionDirect() throws Exception { + context.addRoutes(new RouteBuilder() { + @Override + public void configure() throws Exception { + onException(Throwable.class) + .to("mock:c") + .to("direct:handle"); + + from("direct:test") + .to("mock:a") + .throwException(new IllegalStateException("Bad state")) + .to("mock:b"); + + from("direct:handle") + .to("mock:d") + .log("Handling exception") + .throwException(new NullPointerException("A NPE error here")); + } + }); + context.start(); + + getMockEndpoint("mock:a").expectedMessageCount(1); + getMockEndpoint("mock:b").expectedMessageCount(0); + getMockEndpoint("mock:c").expectedMessageCount(1); + getMockEndpoint("mock:d").expectedMessageCount(1); + try { template.sendBody("direct:test", "Hello World"); + fail("Should have thrown exception"); } catch (CamelExecutionException e) { - assertTrue("Simulate exception in route", e.getCause() instanceof IllegalStateException); + NullPointerException npe = assertIsInstanceOf(NullPointerException.class, e.getCause()); + assertEquals("A NPE error here", npe.getMessage()); + + IllegalStateException ise = assertIsInstanceOf(IllegalStateException.class, npe.getSuppressed()[0]); + assertEquals("Bad state", ise.getMessage()); } + + // TODO: should only trigger error handling in direct route one time + // assertMockEndpointsSatisfied(); } - @Override - protected RouteBuilder createRouteBuilder() throws Exception { - return new RouteBuilder() { + public void testRecursionDirectNoErrorHandler() throws Exception { + context.addRoutes(new RouteBuilder() { @Override public void configure() throws Exception { - onException(Throwable.class).to("direct:handle"); + onException(Throwable.class) + .to("mock:c") + .to("direct:handle"); - from("direct:test").throwException(new IllegalStateException()).to("log:test"); + from("direct:test") + .to("mock:a") + .throwException(new IllegalStateException("Bad state")) + .to("mock:b"); - from("direct:handle").throwException(new NullPointerException()); + from("direct:handle").errorHandler(noErrorHandler()) + .to("mock:d") + .log("Handling exception") + .throwException(new NullPointerException("A NPE error here")); } + }); + context.start(); + + getMockEndpoint("mock:a").expectedMessageCount(1); + getMockEndpoint("mock:b").expectedMessageCount(0); + getMockEndpoint("mock:c").expectedMessageCount(1); + getMockEndpoint("mock:d").expectedMessageCount(1); - }; + try { + template.sendBody("direct:test", "Hello World"); + fail("Should have thrown exception"); + } catch (CamelExecutionException e) { + NullPointerException npe = assertIsInstanceOf(NullPointerException.class, e.getCause()); + assertEquals("A NPE error here", npe.getMessage()); + + IllegalStateException ise = assertIsInstanceOf(IllegalStateException.class, npe.getSuppressed()[0]); + assertEquals("Bad state", ise.getMessage()); + } + assertMockEndpointsSatisfied(); } } http://git-wip-us.apache.org/repos/asf/camel/blob/a08cb431/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionRouteScopedRecursionTest.java ---------------------------------------------------------------------- diff --git a/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionRouteScopedRecursionTest.java b/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionRouteScopedRecursionTest.java new file mode 100644 index 0000000..883a2f5 --- /dev/null +++ b/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionRouteScopedRecursionTest.java @@ -0,0 +1,262 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.processor.onexception; + +import org.apache.camel.CamelExecutionException; +import org.apache.camel.ContextTestSupport; +import org.apache.camel.builder.RouteBuilder; + +/** + * Test that exceptions in an onException handler route do not go into recursion + */ +public class OnExceptionRouteScopedRecursionTest extends ContextTestSupport { + + @Override + public boolean isUseRouteBuilder() { + return false; + } + + public void testRecursion() throws Exception { + getMockEndpoint("mock:a").expectedMessageCount(1); + getMockEndpoint("mock:b").expectedMessageCount(0); + getMockEndpoint("mock:c").expectedMessageCount(1); + getMockEndpoint("mock:d").expectedMessageCount(1); + + context.addRoutes(new RouteBuilder() { + @Override + public void configure() throws Exception { + from("direct:test") + .onException(Throwable.class) + .to("mock:c") + .log("onException") + .throwException(new NullPointerException("A NPE error here")) + .end() + .to("mock:a") + .log("test") + .throwException(new IllegalStateException("Bad state")).to("log:test") + .to("mock:b"); + } + }); + context.start(); + + try { + template.sendBody("direct:test", "Hello World"); + fail("Should have thrown exception"); + } catch (CamelExecutionException e) { + NullPointerException npe = assertIsInstanceOf(NullPointerException.class, e.getCause()); + assertEquals("A NPE error here", npe.getMessage()); + + IllegalStateException ise = assertIsInstanceOf(IllegalStateException.class, npe.getSuppressed()[0]); + assertEquals("Bad state", ise.getMessage()); + } + } + + public void testRecursionHandled() throws Exception { + getMockEndpoint("mock:a").expectedMessageCount(1); + getMockEndpoint("mock:b").expectedMessageCount(0); + getMockEndpoint("mock:c").expectedMessageCount(1); + getMockEndpoint("mock:d").expectedMessageCount(1); + + context.addRoutes(new RouteBuilder() { + @Override + public void configure() throws Exception { + from("direct:test") + .onException(Throwable.class) + .handled(true) + .log("onException") + .throwException(new NullPointerException("A NPE error here")) + .end() + .to("mock:a") + .log("test") + .throwException(new IllegalStateException("Bad state")).to("log:test") + .to("mock:b"); + } + }); + context.start(); + + try { + template.sendBody("direct:test", "Hello World"); + fail("Should have thrown exception"); + } catch (CamelExecutionException e) { + NullPointerException npe = assertIsInstanceOf(NullPointerException.class, e.getCause()); + assertEquals("A NPE error here", npe.getMessage()); + + IllegalStateException ise = assertIsInstanceOf(IllegalStateException.class, npe.getSuppressed()[0]); + assertEquals("Bad state", ise.getMessage()); + } + } + + public void testRecursionDirectNoErrorHandler() throws Exception { + getMockEndpoint("mock:a").expectedMessageCount(1); + getMockEndpoint("mock:b").expectedMessageCount(0); + getMockEndpoint("mock:c").expectedMessageCount(1); + getMockEndpoint("mock:d").expectedMessageCount(1); + + context.addRoutes(new RouteBuilder() { + @Override + public void configure() throws Exception { + from("direct:test") + .onException(Throwable.class) + .to("mock:c") + .log("onException") + .to("direct:error") + .end() + .to("mock:a") + .log("test") + .throwException(new IllegalStateException("Bad state")).to("log:test") + .to("mock:b"); + + // need to turn off error handler when linked with direct, in case you want the same as inlined + from("direct:error").errorHandler(noErrorHandler()) + .to("mock:d") + .log("error") + .throwException(new NullPointerException("A NPE error here")); + } + }); + context.start(); + + try { + template.sendBody("direct:test", "Hello World"); + fail("Should have thrown exception"); + } catch (CamelExecutionException e) { + NullPointerException npe = assertIsInstanceOf(NullPointerException.class, e.getCause()); + assertEquals("A NPE error here", npe.getMessage()); + + IllegalStateException ise = assertIsInstanceOf(IllegalStateException.class, npe.getSuppressed()[0]); + assertEquals("Bad state", ise.getMessage()); + } + } + + public void testRecursionHandledDirectNoErrorHandler() throws Exception { + getMockEndpoint("mock:a").expectedMessageCount(1); + getMockEndpoint("mock:b").expectedMessageCount(0); + getMockEndpoint("mock:c").expectedMessageCount(1); + getMockEndpoint("mock:d").expectedMessageCount(1); + + context.addRoutes(new RouteBuilder() { + @Override + public void configure() throws Exception { + from("direct:test") + .onException(Throwable.class) + .handled(true) + .to("mock:c") + .to("direct:error") + .end() + .to("mock:a") + .log("test") + .throwException(new IllegalStateException("Bad state")).to("log:test") + .to("mock:b"); + + // need to turn off error handler when linked with direct, in case you want the same as inlined + from("direct:error").errorHandler(noErrorHandler()) + .to("mock:d") + .log("error") + .throwException(new NullPointerException("A NPE error here")); + } + }); + context.start(); + + try { + template.sendBody("direct:test", "Hello World"); + fail("Should have thrown exception"); + } catch (CamelExecutionException e) { + NullPointerException npe = assertIsInstanceOf(NullPointerException.class, e.getCause()); + assertEquals("A NPE error here", npe.getMessage()); + + IllegalStateException ise = assertIsInstanceOf(IllegalStateException.class, npe.getSuppressed()[0]); + assertEquals("Bad state", ise.getMessage()); + } + } + + public void testRecursionDirect() throws Exception { + getMockEndpoint("mock:a").expectedMessageCount(1); + getMockEndpoint("mock:b").expectedMessageCount(0); + getMockEndpoint("mock:c").expectedMessageCount(1); + getMockEndpoint("mock:d").expectedMessageCount(1); + + context.addRoutes(new RouteBuilder() { + @Override + public void configure() throws Exception { + from("direct:test") + .onException(Throwable.class) + .to("mock:c") + .log("onException") + .to("direct:error") + .end() + .to("mock:a") + .log("test") + .throwException(new IllegalStateException("Bad state")).to("log:test") + .to("mock:b"); + + from("direct:error") + .to("mock:d") + .log("error") + .throwException(new NullPointerException("A NPE error here")); + } + }); + context.start(); + + try { + template.sendBody("direct:test", "Hello World"); + fail("Should have thrown exception"); + } catch (CamelExecutionException e) { + NullPointerException npe = assertIsInstanceOf(NullPointerException.class, e.getCause()); + assertEquals("A NPE error here", npe.getMessage()); + // we can only see the NPE from the direct route + } + } + + public void testRecursionHandledDirect() throws Exception { + getMockEndpoint("mock:a").expectedMessageCount(1); + getMockEndpoint("mock:b").expectedMessageCount(0); + getMockEndpoint("mock:c").expectedMessageCount(1); + getMockEndpoint("mock:d").expectedMessageCount(1); + + context.addRoutes(new RouteBuilder() { + @Override + public void configure() throws Exception { + from("direct:test") + .onException(Throwable.class) + .handled(true) + .to("mock:c") + .log("onException") + .to("direct:error") + .end() + .to("mock:a") + .log("test") + .throwException(new IllegalStateException("Bad state")).to("log:test") + .to("mock:b"); + + from("direct:error") + .to("mock:d") + .log("error") + .throwException(new NullPointerException("A NPE error here")); + } + }); + context.start(); + + try { + template.sendBody("direct:test", "Hello World"); + fail("Should have thrown exception"); + } catch (CamelExecutionException e) { + NullPointerException npe = assertIsInstanceOf(NullPointerException.class, e.getCause()); + assertEquals("A NPE error here", npe.getMessage()); + // we can only see the NPE from the direct route + } + } + +}
