This is an automated email from the ASF dual-hosted git repository.
davsclaus pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/main by this push:
new 3a481b4c0de4 CAMEL-25113: camel-core - onException configuration: fix
bugs found in a deep review (#27019)
3a481b4c0de4 is described below
commit 3a481b4c0de4d7df7e91aee68ccf88065b0d795c
Author: Claus Ibsen <[email protected]>
AuthorDate: Tue Sep 29 07:35:35 2026 +0200
CAMEL-25113: camel-core - onException configuration: fix bugs found in a
deep review (#27019)
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Signed-off-by: Claus Ibsen <[email protected]>
---
.../apache/camel/model/OnExceptionDefinition.java | 4 +-
.../org/apache/camel/reifier/CatchReifier.java | 3 +
.../reifier/errorhandler/ErrorHandlerReifier.java | 10 +-
.../OnExceptionReifierEdgeCasesTest.java | 149 +++++++++++++++++++++
.../ROOT/pages/camel-4x-upgrade-guide-4_23.adoc | 8 ++
.../modules/ROOT/pages/exception-clause.adoc | 35 +++--
.../modules/ROOT/pages/exception-redelivery.adoc | 6 +
7 files changed, 199 insertions(+), 16 deletions(-)
diff --git
a/core/camel-core-model/src/main/java/org/apache/camel/model/OnExceptionDefinition.java
b/core/camel-core-model/src/main/java/org/apache/camel/model/OnExceptionDefinition.java
index b43ae8accefa..65b780c381b6 100644
---
a/core/camel-core-model/src/main/java/org/apache/camel/model/OnExceptionDefinition.java
+++
b/core/camel-core-model/src/main/java/org/apache/camel/model/OnExceptionDefinition.java
@@ -222,8 +222,8 @@ public class OnExceptionDefinition extends
OutputDefinition<OnExceptionDefinitio
if (outputs == null || getOutputs().isEmpty()) {
// no outputs so there should be some sort of configuration
ObjectHelper.firstNotNull(handledPolicy, handled, continuedPolicy,
continued, retryWhilePolicy, retryWhile,
- redeliveryPolicyType, useOriginalMessage, useOriginalBody,
onRedeliveryRef,
- onRedelivery, onExceptionOccurred)
+ redeliveryPolicyType, redeliveryPolicyRef,
useOriginalMessage, useOriginalBody, onRedeliveryRef,
+ onRedelivery, onExceptionOccurredRef, onExceptionOccurred)
.orElseThrow(() -> new IllegalArgumentException(this + "
is not configured."));
}
}
diff --git
a/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/CatchReifier.java
b/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/CatchReifier.java
index ed9d59b33f14..65469b81f900 100644
---
a/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/CatchReifier.java
+++
b/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/CatchReifier.java
@@ -79,6 +79,9 @@ public class CatchReifier extends
ProcessorReifier<CatchDefinition> {
List<Class<? extends Throwable>> answer = new ArrayList<>(list.size());
for (String name : list) {
Class<Throwable> type =
camelContext.getClassResolver().resolveMandatoryClass(name, Throwable.class);
+ if (!Throwable.class.isAssignableFrom(type)) {
+ throw new IllegalArgumentException("The class: " + name + " in
doCatch is not an exception");
+ }
answer.add(type);
}
return answer;
diff --git
a/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/errorhandler/ErrorHandlerReifier.java
b/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/errorhandler/ErrorHandlerReifier.java
index fe8e307a267c..93bc9bdecdae 100644
---
a/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/errorhandler/ErrorHandlerReifier.java
+++
b/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/errorhandler/ErrorHandlerReifier.java
@@ -295,13 +295,15 @@ public abstract class ErrorHandlerReifier<T extends
ErrorHandlerFactory> extends
List<Class<? extends Throwable>> list;
if (ObjectHelper.isNotEmpty(exceptionType.getExceptions())) {
list = createExceptionClasses(exceptionType);
+ // create the predicate (once) when the route is created (such as
resolving placeholders of route templates)
+ Predicate when = exceptionType.getOnWhen() != null
+ ?
createPredicate(exceptionType.getOnWhen().getExpression()) : null;
for (Class<? extends Throwable> clazz : list) {
String routeId = null;
// only get the route id, if the exception type is route scoped
if (exceptionType.isRouteScoped()) {
routeId = route.getRouteId();
}
- Predicate when = exceptionType.getOnWhen() != null ?
exceptionType.getOnWhen().getExpression() : null;
ExceptionPolicyKey key = new ExceptionPolicyKey(routeId,
clazz, when);
ExceptionPolicy policy = createExceptionPolicy(exceptionType);
handlerSupport.addExceptionPolicy(key, policy);
@@ -324,7 +326,11 @@ public abstract class ErrorHandlerReifier<T extends
ErrorHandlerFactory> extends
}
protected Class<? extends Throwable> resolveExceptionClass(String name)
throws ClassNotFoundException {
- return camelContext.getClassResolver().resolveMandatoryClass(name,
Throwable.class);
+ Class<? extends Throwable> answer =
camelContext.getClassResolver().resolveMandatoryClass(name, Throwable.class);
+ if (!Throwable.class.isAssignableFrom(answer)) {
+ throw new IllegalArgumentException("The class: " + name + " in
onException is not an exception");
+ }
+ return answer;
}
/**
diff --git
a/core/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionReifierEdgeCasesTest.java
b/core/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionReifierEdgeCasesTest.java
new file mode 100644
index 000000000000..86578de6f4dd
--- /dev/null
+++
b/core/camel-core/src/test/java/org/apache/camel/processor/onexception/OnExceptionReifierEdgeCasesTest.java
@@ -0,0 +1,149 @@
+/*
+ * 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 java.io.IOException;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.camel.ContextTestSupport;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.builder.TemplatedRouteBuilder;
+import org.apache.camel.model.CatchDefinition;
+import org.apache.camel.model.OnExceptionDefinition;
+import org.apache.camel.model.TryDefinition;
+import org.apache.camel.processor.errorhandler.RedeliveryPolicy;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class OnExceptionReifierEdgeCasesTest extends ContextTestSupport {
+
+ @Override
+ public boolean isUseRouteBuilder() {
+ return false;
+ }
+
+ @Test
+ public void testOnWhenWithRouteTemplateParameter() throws Exception {
+ context.addRoutes(new RouteBuilder() {
+ @Override
+ public void configure() {
+ routeTemplate("myTemplate").templateParameter("code")
+ .from("direct:start")
+
.onException(IOException.class).onWhen(simple("${header.code} ==
'{{code}}'")).handled(true)
+ .to("mock:when").end()
+
.onException(IOException.class).handled(true).to("mock:fallback").end()
+ .throwException(new IOException("Forced"));
+ }
+ });
+ context.start();
+ TemplatedRouteBuilder.builder(context, "myTemplate").parameter("code",
"abc").add();
+
+ getMockEndpoint("mock:when").expectedMessageCount(1);
+ getMockEndpoint("mock:fallback").expectedMessageCount(0);
+
+ template.sendBodyAndHeader("direct:start", "Hello", "code", "abc");
+
+ assertMockEndpointsSatisfied();
+ }
+
+ @Test
+ public void testOnWhenInvalidSyntaxFailsOnStartup() {
+ assertThrows(Exception.class, () -> {
+ context.addRoutes(new RouteBuilder() {
+ @Override
+ public void configure() {
+ from("direct:start")
+
.onException(IOException.class).onWhen(simple("${header.foo
==")).handled(true).end()
+ .to("mock:result");
+ }
+ });
+ context.start();
+ });
+ }
+
+ @Test
+ public void testRedeliveryPolicyRefOnly() throws Exception {
+ RedeliveryPolicy policy = new RedeliveryPolicy();
+ policy.setMaximumRedeliveries(2);
+ policy.setRedeliveryDelay(0);
+ context.getRegistry().bind("myPolicy", policy);
+
+ AtomicInteger attempts = new AtomicInteger();
+ context.addRoutes(new RouteBuilder() {
+ @Override
+ public void configure() {
+ onException(IOException.class).redeliveryPolicyRef("myPolicy");
+
+ from("direct:start").process(e -> {
+ attempts.incrementAndGet();
+ throw new IOException("Forced");
+ });
+ }
+ });
+ context.start();
+
+ assertThrows(Exception.class, () -> template.sendBody("direct:start",
"Hello"));
+ assertEquals(3, attempts.get());
+ }
+
+ @Test
+ public void testNotAnExceptionClass() {
+ assertThrows(Exception.class, () -> {
+ context.addRoutes(new RouteBuilder() {
+ @Override
+ public void configure() {
+ OnExceptionDefinition def =
onException(IOException.class).handled(true);
+ def.getExceptions().clear();
+ def.getExceptions().add("java.lang.String");
+
+ from("direct:start").to("mock:result");
+ }
+ });
+ context.start();
+ });
+ }
+
+ @Test
+ public void testDoCatchNotAnExceptionClass() {
+ Exception e = assertThrows(Exception.class, () -> {
+ context.addRoutes(new RouteBuilder() {
+ @Override
+ public void configure() {
+ TryDefinition doTry =
from("direct:start").doTry().to("mock:result");
+ doTry.doCatch(IOException.class);
+ CatchDefinition doCatch = (CatchDefinition)
doTry.getOutputs().get(doTry.getOutputs().size() - 1);
+ // as in XML or YAML, where the classes are resolved from
their names
+ doCatch.setExceptionClasses(null);
+ doCatch.getExceptions().clear();
+ doCatch.getExceptions().add("java.lang.String");
+ }
+ });
+ context.start();
+ });
+ assertTrue(getRootCause(e).getMessage().contains("is not an
exception"), e.getMessage());
+ }
+
+ private static Throwable getRootCause(Throwable t) {
+ while (t.getCause() != null) {
+ t = t.getCause();
+ }
+ return t;
+ }
+}
diff --git
a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
index b31a927923b0..5cd5607798de 100644
--- a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
+++ b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
@@ -3462,6 +3462,14 @@ overflowing to a negative number.
The Simple function `${normalizeWhitespace()}` now turns every run of
whitespace (including tabs and new lines) into
a single space. Previously only runs of spaces were collapsed and a tab or new
line was kept as-is.
+=== camel-core - onException and doCatch configuration is validated when the
route is created
+
+The `onWhen` predicate of an `onException` is now created when the route is
created (as with `doCatch` and
+`onCompletion`), so an invalid `onWhen` expression now fails at startup,
instead of when an exception occurs.
+
+A class in `onException` or `doCatch` that is not an exception (such as
`<exception>java.lang.String</exception>` in XML
+or YAML) now fails at startup. Previously it was accepted, and never matched
any exception.
+
=== camel-debezium - a failed embedded engine is now reported
The Debezium consumers now register a `CompletionCallback` on the embedded
engine. When the engine stops
diff --git a/docs/user-manual/modules/ROOT/pages/exception-clause.adoc
b/docs/user-manual/modules/ROOT/pages/exception-clause.adoc
index 31ad69e29351..f02adaca74b9 100644
--- a/docs/user-manual/modules/ROOT/pages/exception-clause.adoc
+++ b/docs/user-manual/modules/ROOT/pages/exception-clause.adoc
@@ -174,16 +174,27 @@ Camel uses *`DefaultExceptionPolicyStrategy`* to
determine a strategy
how an exception being thrown should be handled by which *`onException`*
clause. The strategy is:
-* the order in which the *`onException`* is configured takes precedence.
-Camel will test from first...last defined.
-* Camel will start from the bottom (nested caused by) and recursive up
-in the exception hierarchy to find the first matching *`onException`*
-clause.
+* Camel tests the thrown exception, and each of its causes (the nested
+caused by exceptions), against the exceptions of all the *`onException`*
+clauses.
* *`instanceof`* test is used for testing the given exception with the
-*`onException`* clause defined exception list. An exact *`instanceof`*
-match will always be used, otherwise the *`onException`* clause that has
-an exception that is the closets super of the thrown exception is
-selected (recurring up the exception hierarchy).
+*`onException`* clause defined exception list. An exact match is
+preferred, otherwise the *`onException`* clause that has an exception
+that is the closest super class of the thrown exception is selected
+(recurring up the exception hierarchy).
+* The *closest* match wins, across the thrown exception and all its causes.
+For example, an exact match on the thrown (wrapper) exception wins over a
+super class match on a nested cause. When two clauses are equally close,
+the match on the most nested cause (bottom) wins.
+* When an *`onException`* in a route and an *`onException`* in the
+`RouteBuilder` (context scoped) are equally close, then the one in the
+route wins.
+* An *`onException`* with an `onWhen` predicate only matches when the
+predicate is `true`.
+* If the same exception is configured in more than one *`onException`*
+(in the same scope, without `onWhen`), then the *last* one configured is
+used. The *`onException`* from route configurations are added after the
+ones in the `RouteBuilder`, and are therefore used instead.
This is best illustrated with an exception:
@@ -197,9 +208,9 @@ onException(OrderFailedException.class)
.maximumRedeliveries(2);
----
-In the sample above we have defined two exceptions in
-which *`IOException`* is first, so Camel will pickup this exception if
-there is a match. *`IOException`* that is more general is selected then.
+In the sample above we have defined two exceptions, and Camel selects the
+*`onException`* that is the closest match of the thrown exception (or one of
+its causes).
So if an exception is thrown with this hierarchy:
diff --git a/docs/user-manual/modules/ROOT/pages/exception-redelivery.adoc
b/docs/user-manual/modules/ROOT/pages/exception-redelivery.adoc
index 703e9eacb1b0..cdd9ffff1a31 100644
--- a/docs/user-manual/modules/ROOT/pages/exception-redelivery.adoc
+++ b/docs/user-manual/modules/ROOT/pages/exception-redelivery.adoc
@@ -18,6 +18,12 @@ information about redeliver and which redeliver options
exists.
By default, any xref:exception-clause.adoc[Exception Clause] will *not*
redeliver! (as it sets the `maximumRedeliveries` option to 0).
+NOTE: When any redelivery option is configured on the *`onException`* (such as
+`redeliveryDelay`, `logStackTrace` or `retryAttemptedLogLevel`), then its
+redelivery policy is based on the redelivery policy of the error handler, and
+the `maximumRedeliveries` of the error handler is used (unless
+`maximumRedeliveries` is also configured on the *`onException`*).
+
Sometimes you want to configure the redelivery policy on a per exception
type basis. By default in the top examples, if an
*`org.apache.camel.ValidationException`* occurs then the message will
