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 9ca2736acd4d CAMEL-25114: camel-core - doTry, doCatch and doFinally: 
fix bugs found in a deep review (#27020)
9ca2736acd4d is described below

commit 9ca2736acd4dc7e7d8c7cd923a4e15b2dfe0788a
Author: Claus Ibsen <[email protected]>
AuthorDate: Mon Sep 28 22:45:01 2026 +0200

    CAMEL-25114: camel-core - doTry, doCatch and doFinally: fix bugs found in a 
deep review (#27020)
    
    Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
    Signed-off-by: Claus Ibsen <[email protected]>
---
 .../org/apache/camel/catalog/docs/doTry-eip.adoc   |  32 ++-
 .../TransactedDoTryRecipientListTest.java          |   6 +-
 .../processor/SpringTryCatchMisconfiguredTest.java |   9 +-
 .../main/docs/modules/eips/pages/doTry-eip.adoc    |  32 ++-
 .../java/org/apache/camel/model/TryDefinition.java |  12 +-
 .../org/apache/camel/processor/CatchProcessor.java |  22 +-
 .../apache/camel/processor/FinallyProcessor.java   |  17 +-
 .../org/apache/camel/processor/TryProcessor.java   |  32 ++-
 .../org/apache/camel/reifier/CatchReifier.java     |   1 +
 .../org/apache/camel/reifier/FinallyReifier.java   |   1 +
 .../org/apache/camel/reifier/ProcessorReifier.java |  12 +
 .../java/org/apache/camel/reifier/TryReifier.java  |   5 +-
 .../apache/camel/processor/DoTryEdgeCasesTest.java | 253 +++++++++++++++++++++
 .../ROOT/pages/camel-4x-upgrade-guide-4_23.adoc    |  18 ++
 14 files changed, 430 insertions(+), 22 deletions(-)

diff --git 
a/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/doTry-eip.adoc
 
b/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/doTry-eip.adoc
index e93c0d2c715f..304606c2f286 100644
--- 
a/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/doTry-eip.adoc
+++ 
b/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/doTry-eip.adoc
@@ -204,6 +204,35 @@ If there is no `doFinally`, then the `end()` should be at 
the end of the last `d
 
 TIP: Instead of `end()` you can use `endDoTry()` to end and _return back_ to 
the try ... catch scope.
 
+A `doTry` must have one or more `doCatch` or `doFinally` blocks, otherwise the 
route fails to start.
+
+=== How doCatch matches an exception
+
+The `doCatch` blocks are tried in the order they are defined, and the first 
`doCatch` that matches is used
+(the same as in Java). For each `doCatch`, the exception and its causes are 
matched (starting from the root cause and going upwards to
+the thrown exception), and the `onWhen` predicate (if any) must also be true.
+
+This is different from `onException` (which selects the closest matching 
exception type across all its clauses).
+For example, if a `doCatch(IOException.class)` is defined before a 
`doCatch(RuntimeCamelException.class)`, then a
+`RuntimeCamelException` caused by an `IOException` is caught by the 
`doCatch(IOException.class)` block.
+
+The `Exchange.EXCEPTION_CAUGHT` exchange property contains the thrown 
exception (and not the cause that matched).
+
+=== Exceptions that are not caught
+
+An exception that is not caught by any `doCatch` (or an exception that is 
thrown in a `doCatch` or `doFinally` block)
+is not handled by the route error handler: `onException`, redelivery and the 
dead letter channel are not in use
+for the steps of a `doTry ... doCatch ... doFinally` block. The exception is 
thrown from the `doTry`, and the exchange fails.
+
+If an exception is thrown in the `doFinally` block while an exception from the 
`doTry` (or `doCatch`) block
+is still not handled, then the original exception is kept and the exception 
from the `doFinally` block is added
+as a suppressed exception.
+
+=== Stopping inside doTry
+
+Using `stop()` inside the `doTry` or `doCatch` block stops routing the 
message, and the `doFinally` block
+is not executed.
+
 === Nested doTry ... doCatch
 
 When nesting `doTry ... doCatch` from an outer `doTry ... doCatch` EIP,
@@ -226,7 +255,8 @@ from("direct:test")
     .end();
 ----
 
-Both `docatch 1` and `docatch 2` are in the inner `doTry`, and the outer 
`doTry` has no catch blocks.
+Both `docatch 1` and `docatch 2` are in the inner `doTry`, and the outer 
`doTry` has no catch blocks
+(and the route fails to start, as a `doTry` must have one or more `doCatch` or 
`doFinally` blocks).
 Use `endDoTry()` to end the blocks correctly:
 
 [source,java]
diff --git 
a/components/camel-spring-parent/camel-spring-xml/src/test/java/org/apache/camel/spring/interceptor/TransactedDoTryRecipientListTest.java
 
b/components/camel-spring-parent/camel-spring-xml/src/test/java/org/apache/camel/spring/interceptor/TransactedDoTryRecipientListTest.java
index 3b98718d1128..f498ed1c7a5b 100644
--- 
a/components/camel-spring-parent/camel-spring-xml/src/test/java/org/apache/camel/spring/interceptor/TransactedDoTryRecipientListTest.java
+++ 
b/components/camel-spring-parent/camel-spring-xml/src/test/java/org/apache/camel/spring/interceptor/TransactedDoTryRecipientListTest.java
@@ -41,7 +41,11 @@ public class TransactedDoTryRecipientListTest extends 
TransactionClientDataSourc
                         .transacted()
                         .doTry()
                             .recipientList(constant("direct:a"))
-                        .endDoTry();
+                        .endDoTry()
+                        // a doTry must have a doCatch or doFinally
+                        .doFinally()
+                            .to("mock:finally")
+                        .end();
 
                 from("direct:a")
                         .delay(1)
diff --git 
a/components/camel-spring-parent/camel-spring-xml/src/test/java/org/apache/camel/spring/processor/SpringTryCatchMisconfiguredTest.java
 
b/components/camel-spring-parent/camel-spring-xml/src/test/java/org/apache/camel/spring/processor/SpringTryCatchMisconfiguredTest.java
index aaca610ae303..94e7291d936d 100644
--- 
a/components/camel-spring-parent/camel-spring-xml/src/test/java/org/apache/camel/spring/processor/SpringTryCatchMisconfiguredTest.java
+++ 
b/components/camel-spring-parent/camel-spring-xml/src/test/java/org/apache/camel/spring/processor/SpringTryCatchMisconfiguredTest.java
@@ -22,8 +22,8 @@ import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import static 
org.apache.camel.spring.processor.SpringTestHelper.createSpringCamelContext;
-import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 class SpringTryCatchMisconfiguredTest extends ContextTestSupport {
 
@@ -40,8 +40,8 @@ class SpringTryCatchMisconfiguredTest extends 
ContextTestSupport {
         });
         FailedToCreateRouteException ftce = 
assertIsInstanceOf(FailedToCreateRouteException.class, e1);
         IllegalArgumentException iae = 
assertIsInstanceOf(IllegalArgumentException.class, ftce.getCause());
-        assertEquals(
-                "This doCatch should have a doTry as its parent on 
DoCatch[java.io.IOException -> [to[mock:fail]]]",
+        // the doCatch is not inside the doTry, so the doTry has no doCatch or 
doFinally
+        assertTrue(iae.getMessage().startsWith("doTry must have one or more 
doCatch or doFinally blocks"),
                 iae.getMessage());
 
         Exception e2 = assertThrows(Exception.class, () -> {
@@ -49,7 +49,8 @@ class SpringTryCatchMisconfiguredTest extends 
ContextTestSupport {
         });
         FailedToCreateRouteException ftcre = 
assertIsInstanceOf(FailedToCreateRouteException.class, e2);
         IllegalArgumentException iae2 = 
assertIsInstanceOf(IllegalArgumentException.class, ftcre.getCause());
-        assertEquals("This doFinally should have a doTry as its parent on 
DoFinally[[to[mock:finally]]]", iae2.getMessage());
+        assertTrue(iae2.getMessage().startsWith("doTry must have one or more 
doCatch or doFinally blocks"),
+                iae2.getMessage());
     }
 
 }
diff --git 
a/core/camel-core-engine/src/main/docs/modules/eips/pages/doTry-eip.adoc 
b/core/camel-core-engine/src/main/docs/modules/eips/pages/doTry-eip.adoc
index e93c0d2c715f..304606c2f286 100644
--- a/core/camel-core-engine/src/main/docs/modules/eips/pages/doTry-eip.adoc
+++ b/core/camel-core-engine/src/main/docs/modules/eips/pages/doTry-eip.adoc
@@ -204,6 +204,35 @@ If there is no `doFinally`, then the `end()` should be at 
the end of the last `d
 
 TIP: Instead of `end()` you can use `endDoTry()` to end and _return back_ to 
the try ... catch scope.
 
+A `doTry` must have one or more `doCatch` or `doFinally` blocks, otherwise the 
route fails to start.
+
+=== How doCatch matches an exception
+
+The `doCatch` blocks are tried in the order they are defined, and the first 
`doCatch` that matches is used
+(the same as in Java). For each `doCatch`, the exception and its causes are 
matched (starting from the root cause and going upwards to
+the thrown exception), and the `onWhen` predicate (if any) must also be true.
+
+This is different from `onException` (which selects the closest matching 
exception type across all its clauses).
+For example, if a `doCatch(IOException.class)` is defined before a 
`doCatch(RuntimeCamelException.class)`, then a
+`RuntimeCamelException` caused by an `IOException` is caught by the 
`doCatch(IOException.class)` block.
+
+The `Exchange.EXCEPTION_CAUGHT` exchange property contains the thrown 
exception (and not the cause that matched).
+
+=== Exceptions that are not caught
+
+An exception that is not caught by any `doCatch` (or an exception that is 
thrown in a `doCatch` or `doFinally` block)
+is not handled by the route error handler: `onException`, redelivery and the 
dead letter channel are not in use
+for the steps of a `doTry ... doCatch ... doFinally` block. The exception is 
thrown from the `doTry`, and the exchange fails.
+
+If an exception is thrown in the `doFinally` block while an exception from the 
`doTry` (or `doCatch`) block
+is still not handled, then the original exception is kept and the exception 
from the `doFinally` block is added
+as a suppressed exception.
+
+=== Stopping inside doTry
+
+Using `stop()` inside the `doTry` or `doCatch` block stops routing the 
message, and the `doFinally` block
+is not executed.
+
 === Nested doTry ... doCatch
 
 When nesting `doTry ... doCatch` from an outer `doTry ... doCatch` EIP,
@@ -226,7 +255,8 @@ from("direct:test")
     .end();
 ----
 
-Both `docatch 1` and `docatch 2` are in the inner `doTry`, and the outer 
`doTry` has no catch blocks.
+Both `docatch 1` and `docatch 2` are in the inner `doTry`, and the outer 
`doTry` has no catch blocks
+(and the route fails to start, as a `doTry` must have one or more `doCatch` or 
`doFinally` blocks).
 Use `endDoTry()` to end the blocks correctly:
 
 [source,java]
diff --git 
a/core/camel-core-model/src/main/java/org/apache/camel/model/TryDefinition.java 
b/core/camel-core-model/src/main/java/org/apache/camel/model/TryDefinition.java
index 5f95f3d66ad9..80e2933c6ac2 100644
--- 
a/core/camel-core-model/src/main/java/org/apache/camel/model/TryDefinition.java
+++ 
b/core/camel-core-model/src/main/java/org/apache/camel/model/TryDefinition.java
@@ -18,7 +18,6 @@ package org.apache.camel.model;
 
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.List;
 
 import jakarta.xml.bind.annotation.XmlAccessType;
@@ -142,10 +141,13 @@ public class TryDefinition extends 
OutputDefinition<TryDefinition> {
         // we must use a delegate so we can use the fluent builder based on
         // TryDefinition
         // to configure all with try .. catch .. finally
-        // set the onWhen predicate on all the catch definitions
-        Collection<CatchDefinition> col = 
ProcessorDefinitionHelper.filterTypeInOutputs(getOutputs(), 
CatchDefinition.class);
-        for (CatchDefinition doCatch : col) {
-            doCatch.setOnWhen(new OnWhenDefinition(predicate));
+        // set the onWhen predicate on the last doCatch (the one being 
configured) of this doTry
+        List<ProcessorDefinition<?>> list = getOutputs();
+        for (int i = list.size() - 1; i >= 0; i--) {
+            if (list.get(i) instanceof CatchDefinition doCatch) {
+                doCatch.setOnWhen(new OnWhenDefinition(predicate));
+                break;
+            }
         }
         return this;
     }
diff --git 
a/core/camel-core-processor/src/main/java/org/apache/camel/processor/CatchProcessor.java
 
b/core/camel-core-processor/src/main/java/org/apache/camel/processor/CatchProcessor.java
index 30bf2a219b1a..69a0e536b31f 100644
--- 
a/core/camel-core-processor/src/main/java/org/apache/camel/processor/CatchProcessor.java
+++ 
b/core/camel-core-processor/src/main/java/org/apache/camel/processor/CatchProcessor.java
@@ -131,9 +131,25 @@ public class CatchProcessor extends 
BaseDelegateProcessorSupport
     @Override
     public boolean process(final Exchange exchange, final AsyncCallback 
callback) {
         final Exception e = exchange.getException();
-        Throwable caught = catches(exchange, e);
-        // If a previous catch clause handled the exception or if this clause 
does not match, exit
-        if (exchange.getProperty(ExchangePropertyKey.EXCEPTION_HANDLED) != 
null || caught == null) {
+        // If a previous catch clause handled the exception, exit (before 
matching so this clause is not counted)
+        if (exchange.getProperty(ExchangePropertyKey.EXCEPTION_HANDLED) != 
null) {
+            callback.done(true);
+            return true;
+        }
+        Throwable caught;
+        try {
+            caught = catches(exchange, e);
+        } catch (Exception onWhenException) {
+            // the onWhen predicate failed, so the exchange fails with this 
exception (and the original as suppressed)
+            if (e != null && e != onWhenException) {
+                onWhenException.addSuppressed(e);
+            }
+            exchange.setException(onWhenException);
+            callback.done(true);
+            return true;
+        }
+        // If this clause does not match, exit
+        if (caught == null) {
             callback.done(true);
             return true;
         }
diff --git 
a/core/camel-core-processor/src/main/java/org/apache/camel/processor/FinallyProcessor.java
 
b/core/camel-core-processor/src/main/java/org/apache/camel/processor/FinallyProcessor.java
index 60ace585b1c4..4cb237b9db6d 100644
--- 
a/core/camel-core-processor/src/main/java/org/apache/camel/processor/FinallyProcessor.java
+++ 
b/core/camel-core-processor/src/main/java/org/apache/camel/processor/FinallyProcessor.java
@@ -125,11 +125,20 @@ public class FinallyProcessor extends 
BaseDelegateProcessorSupport
         public void done(boolean doneSync) {
             try {
                 if (exception == null) {
-                    
exchange.removeProperty(ExchangePropertyKey.FAILURE_ENDPOINT);
-                    
exchange.removeProperty(ExchangePropertyKey.FAILURE_ROUTE_ID);
-                    
exchange.removeProperty(ExchangePropertyKey.FAILURE_NODE_ID);
-                    
exchange.removeProperty(ExchangePropertyKey.FAILURE_LOCATION);
+                    // only clear the failure details when the doFinally did 
not fail
+                    // (the doTry restores the failure details of an earlier 
failure)
+                    if (exchange.getException() == null) {
+                        
exchange.removeProperty(ExchangePropertyKey.FAILURE_ENDPOINT);
+                        
exchange.removeProperty(ExchangePropertyKey.FAILURE_ROUTE_ID);
+                        
exchange.removeProperty(ExchangePropertyKey.FAILURE_NODE_ID);
+                        
exchange.removeProperty(ExchangePropertyKey.FAILURE_LOCATION);
+                    }
                 } else {
+                    // the original exception is kept, and an exception from 
the doFinally is added as suppressed
+                    Exception finallyException = exchange.getException();
+                    if (finallyException != null && finallyException != 
exception) {
+                        exception.addSuppressed(finallyException);
+                    }
                     // set exception back on exchange
                     exchange.setException(exception);
                     exchange.setProperty(ExchangePropertyKey.EXCEPTION_CAUGHT, 
exception);
diff --git 
a/core/camel-core-processor/src/main/java/org/apache/camel/processor/TryProcessor.java
 
b/core/camel-core-processor/src/main/java/org/apache/camel/processor/TryProcessor.java
index af2c5ad10026..1b95de5857d5 100644
--- 
a/core/camel-core-processor/src/main/java/org/apache/camel/processor/TryProcessor.java
+++ 
b/core/camel-core-processor/src/main/java/org/apache/camel/processor/TryProcessor.java
@@ -92,6 +92,13 @@ public class TryProcessor extends BaseProcessorSupport
         final AsyncCallback callback;
         final Iterator<Processor> processors;
         final Object lastHandled;
+        // the previous try route block (such as when this doTry is nested in 
another doTry)
+        final Object lastTryRouteBlock;
+        // the failure details of an earlier failure (before this doTry), 
which the doFinally must not remove
+        final Object lastFailureEndpoint;
+        final Object lastFailureRouteId;
+        final Object lastFailureNodeId;
+        final Object lastFailureLocation;
         boolean failureOriginCaptured;
 
         public TryState(Exchange exchange, AsyncCallback callback) {
@@ -99,6 +106,11 @@ public class TryProcessor extends BaseProcessorSupport
             this.callback = callback;
             this.processors = next().iterator();
             this.lastHandled = 
exchange.getProperty(ExchangePropertyKey.EXCEPTION_HANDLED);
+            this.lastTryRouteBlock = 
exchange.getProperty(ExchangePropertyKey.TRY_ROUTE_BLOCK);
+            this.lastFailureEndpoint = 
exchange.getProperty(ExchangePropertyKey.FAILURE_ENDPOINT);
+            this.lastFailureRouteId = 
exchange.getProperty(ExchangePropertyKey.FAILURE_ROUTE_ID);
+            this.lastFailureNodeId = 
exchange.getProperty(ExchangePropertyKey.FAILURE_NODE_ID);
+            this.lastFailureLocation = 
exchange.getProperty(ExchangePropertyKey.FAILURE_LOCATION);
             exchange.removeProperty(ExchangePropertyKey.EXCEPTION_HANDLED);
         }
 
@@ -125,8 +137,20 @@ public class TryProcessor extends BaseProcessorSupport
                 async.process(exchange, doneSync -> 
reactiveExecutor.schedule(this));
             } else {
                 ExchangeHelper.prepareOutToIn(exchange);
-                exchange.removeProperty(ExchangePropertyKey.TRY_ROUTE_BLOCK);
+                // restore the previous try route block, so an outer doTry is 
still in its try block
+                if (lastTryRouteBlock != null) {
+                    exchange.setProperty(ExchangePropertyKey.TRY_ROUTE_BLOCK, 
lastTryRouteBlock);
+                } else {
+                    
exchange.removeProperty(ExchangePropertyKey.TRY_ROUTE_BLOCK);
+                }
                 exchange.setProperty(ExchangePropertyKey.EXCEPTION_HANDLED, 
lastHandled);
+                if (exchange.getException() == null) {
+                    // restore the failure details of an earlier failure 
(which the doFinally removed)
+                    restoreProperty(ExchangePropertyKey.FAILURE_ENDPOINT, 
lastFailureEndpoint);
+                    restoreProperty(ExchangePropertyKey.FAILURE_ROUTE_ID, 
lastFailureRouteId);
+                    restoreProperty(ExchangePropertyKey.FAILURE_NODE_ID, 
lastFailureNodeId);
+                    restoreProperty(ExchangePropertyKey.FAILURE_LOCATION, 
lastFailureLocation);
+                }
                 if (LOG.isTraceEnabled()) {
                     LOG.trace("Processing complete for exchangeId: {} >>> {}", 
exchange.getExchangeId(), exchange);
                 }
@@ -134,6 +158,12 @@ public class TryProcessor extends BaseProcessorSupport
             }
         }
 
+        private void restoreProperty(ExchangePropertyKey key, Object value) {
+            if (value != null) {
+                exchange.setProperty(key, value);
+            }
+        }
+
         @Override
         public String toString() {
             return "TryState";
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 d7ce7ba20a0d..ed9d59b33f14 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
@@ -66,6 +66,7 @@ public class CatchReifier extends 
ProcessorReifier<CatchDefinition> {
         String id = getId(definition);
         processor.setId(id);
         processor.setRouteId(route.getRouteId());
+        injectStepId(processor);
         return wrapProcessor(processor);
     }
 
diff --git 
a/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/FinallyReifier.java
 
b/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/FinallyReifier.java
index fcdc3eaff9a7..5ff0f186342f 100644
--- 
a/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/FinallyReifier.java
+++ 
b/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/FinallyReifier.java
@@ -43,6 +43,7 @@ public class FinallyReifier extends 
ProcessorReifier<FinallyDefinition> {
         String id = getId(definition);
         processor.setId(id);
         processor.setRouteId(route.getRouteId());
+        injectStepId(processor);
         return wrapProcessor(processor);
     }
 
diff --git 
a/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/ProcessorReifier.java
 
b/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/ProcessorReifier.java
index 594045c7389b..71e8fbb5c47a 100644
--- 
a/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/ProcessorReifier.java
+++ 
b/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/ProcessorReifier.java
@@ -1009,4 +1009,16 @@ public abstract class ProcessorReifier<T extends 
ProcessorDefinition<?>> extends
         return disabled;
     }
 
+    /**
+     * Sets the step id (of the step the definition is inside) on the processor
+     */
+    protected void injectStepId(Object processor) {
+        if (processor instanceof StepIdAware stepIdAware) {
+            StepDefinition step = 
ProcessorDefinitionHelper.findFirstParentOfType(StepDefinition.class, 
definition, true);
+            if (step != null) {
+                stepIdAware.setStepId(
+                        
step.idOrCreate(camelContext.getCamelContextExtension().getContextPlugin(NodeIdFactory.class)));
+            }
+        }
+    }
 }
diff --git 
a/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/TryReifier.java
 
b/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/TryReifier.java
index 48c9061f6cde..a78264bd06a8 100644
--- 
a/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/TryReifier.java
+++ 
b/core/camel-core-reifier/src/main/java/org/apache/camel/reifier/TryReifier.java
@@ -48,8 +48,9 @@ public class TryReifier extends 
ProcessorReifier<TryDefinition> {
         }
 
         // user must have configured at least one catch or finally
-        if (definition.getFinallyClause() == null && 
definition.getCatchClauses() == null) {
-            throw new IllegalArgumentException("doTry must have one or more 
catch or finally blocks on " + this);
+        if (definition.getFinallyClause() == null
+                && (definition.getCatchClauses() == null || 
definition.getCatchClauses().isEmpty())) {
+            throw new IllegalArgumentException("doTry must have one or more 
doCatch or doFinally blocks on " + definition);
         }
 
         // must have finally processor as it set some state after completing 
the entire doTry block
diff --git 
a/core/camel-core/src/test/java/org/apache/camel/processor/DoTryEdgeCasesTest.java
 
b/core/camel-core/src/test/java/org/apache/camel/processor/DoTryEdgeCasesTest.java
new file mode 100644
index 000000000000..f5fb53c7ba6d
--- /dev/null
+++ 
b/core/camel-core/src/test/java/org/apache/camel/processor/DoTryEdgeCasesTest.java
@@ -0,0 +1,253 @@
+/*
+ * 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;
+
+import java.io.IOException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.camel.ContextTestSupport;
+import org.apache.camel.Exchange;
+import org.apache.camel.FailedToCreateRouteException;
+import org.apache.camel.Processor;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.spi.StepIdAware;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class DoTryEdgeCasesTest extends ContextTestSupport {
+
+    @Override
+    public boolean isUseRouteBuilder() {
+        return false;
+    }
+
+    @Test
+    public void testOnWhenThrowsException() throws Exception {
+        context.addRoutes(new RouteBuilder() {
+            @Override
+            public void configure() {
+                from("direct:start")
+                        .doTry()
+                        .throwException(new IOException("Forced"))
+                        .doCatch(IOException.class).onWhen(e -> {
+                            throw new IllegalArgumentException("onWhen 
failed");
+                        })
+                        .to("mock:catch")
+                        .doFinally()
+                        .to("mock:finally")
+                        .end();
+            }
+        });
+        context.start();
+
+        getMockEndpoint("mock:catch").expectedMessageCount(0);
+        getMockEndpoint("mock:finally").expectedMessageCount(1);
+
+        // the exchange must complete (and not hang)
+        Exchange out = template.asyncSend("direct:start", e -> 
e.getMessage().setBody("Hello"))
+                .get(10, TimeUnit.SECONDS);
+        assertMockEndpointsSatisfied();
+
+        IllegalArgumentException e = 
assertInstanceOf(IllegalArgumentException.class, out.getException());
+        assertEquals("onWhen failed", e.getMessage());
+        assertInstanceOf(IOException.class, e.getSuppressed()[0]);
+    }
+
+    @Test
+    public void testOnWhenOnEachCatch() throws Exception {
+        context.addRoutes(new RouteBuilder() {
+            @Override
+            public void configure() {
+                from("direct:start")
+                        .doTry()
+                            .throwException(new IOException("Forced"))
+                        
.doCatch(IOException.class).onWhen(header("code").isEqualTo("A"))
+                            .to("mock:io")
+                        
.doCatch(IllegalStateException.class).onWhen(header("code").isEqualTo("B"))
+                            .to("mock:ise")
+                        .end();
+            }
+        });
+        context.start();
+
+        // the onWhen of the 2nd doCatch must not replace the onWhen of the 
1st doCatch
+        getMockEndpoint("mock:io").expectedMessageCount(1);
+        template.sendBodyAndHeader("direct:start", "Hello", "code", "A");
+        assertMockEndpointsSatisfied();
+    }
+
+    @Test
+    public void testNestedDoTryKeepsTryBlock() throws Exception {
+        context.addRoutes(new RouteBuilder() {
+            @Override
+            public void configure() {
+                
errorHandler(deadLetterChannel("mock:dead").maximumRedeliveries(2).redeliveryDelay(0));
+
+                from("direct:start")
+                        .doTry()
+                            .to("direct:sub")
+                            .recipientList(constant("direct:fail")).end()
+                        .doCatch(Exception.class)
+                            .to("mock:catch")
+                        .end();
+
+                from("direct:sub")
+                        .doTry()
+                            .to("mock:inner")
+                        .doCatch(IOException.class)
+                            .to("mock:subCatch")
+                        .end();
+
+                from("direct:fail").errorHandler(noErrorHandler())
+                        .throwException(new 
IllegalArgumentException("Forced"));
+            }
+        });
+        context.start();
+
+        // the outer doCatch handles the failure (and not the dead letter 
channel)
+        getMockEndpoint("mock:catch").expectedMessageCount(1);
+        getMockEndpoint("mock:dead").expectedMessageCount(0);
+        template.sendBody("direct:start", "Hello");
+        assertMockEndpointsSatisfied();
+    }
+
+    @Test
+    public void testDoFinallyThrowsKeepsOriginal() throws Exception {
+        context.addRoutes(new RouteBuilder() {
+            @Override
+            public void configure() {
+                from("direct:start")
+                        .doTry()
+                            .throwException(new IOException("original"))
+                        .doCatch(IllegalArgumentException.class)
+                            .to("mock:catch")
+                        .doFinally()
+                            .throwException(new 
IllegalStateException("finally"))
+                        .end();
+            }
+        });
+        context.start();
+
+        Exchange out = template.send("direct:start", e -> 
e.getMessage().setBody("Hello"));
+        IOException e = assertInstanceOf(IOException.class, 
out.getException());
+        assertEquals("original", e.getMessage());
+        assertEquals(1, e.getSuppressed().length);
+        assertEquals("finally", e.getSuppressed()[0].getMessage());
+    }
+
+    @Test
+    public void testDoFinallyKeepsEarlierFailureDetails() throws Exception {
+        context.addRoutes(new RouteBuilder() {
+            @Override
+            public void configure() {
+                from("direct:start")
+                        // simulate the failure details of an earlier 
(handled) failure
+                        .setProperty(Exchange.FAILURE_ROUTE_ID, 
constant("earlier"))
+                        .doTry()
+                            .to("mock:try")
+                        .doCatch(Exception.class)
+                            .to("mock:catch")
+                        .end()
+                        .to("mock:result");
+            }
+        });
+        context.start();
+
+        
getMockEndpoint("mock:result").expectedPropertyReceived(Exchange.FAILURE_ROUTE_ID,
 "earlier");
+        template.sendBody("direct:start", "Hello");
+        assertMockEndpointsSatisfied();
+    }
+
+    @Test
+    public void testDoTryWithoutCatchOrFinally() throws Exception {
+        context.addRoutes(new RouteBuilder() {
+            @Override
+            public void configure() {
+                from("direct:start")
+                        .doTry()
+                            .to("mock:try")
+                        .end();
+            }
+        });
+        Exception e = assertThrows(Exception.class, () -> context.start());
+        FailedToCreateRouteException fe = 
assertInstanceOf(FailedToCreateRouteException.class, e);
+        assertTrue(fe.getCause().getMessage().startsWith("doTry must have one 
or more doCatch or doFinally blocks"),
+                fe.getCause().getMessage());
+    }
+
+    @Test
+    public void testOnWhenNotEvaluatedWhenHandled() throws Exception {
+        AtomicInteger evaluated = new AtomicInteger();
+        context.addRoutes(new RouteBuilder() {
+            @Override
+            public void configure() {
+                from("direct:start")
+                        .doTry()
+                        .throwException(new IOException("Forced"))
+                        .doCatch(IOException.class)
+                        .to("mock:io")
+                        .doCatch(Exception.class).onWhen(e -> {
+                            evaluated.incrementAndGet();
+                            return true;
+                        })
+                        .to("mock:other")
+                        .end();
+            }
+        });
+        context.start();
+
+        getMockEndpoint("mock:io").expectedMessageCount(1);
+        getMockEndpoint("mock:other").expectedMessageCount(0);
+        template.sendBody("direct:start", "Hello");
+        assertMockEndpointsSatisfied();
+        // the exception was handled by the first doCatch so the next doCatch 
should not be evaluated
+        assertEquals(0, evaluated.get());
+    }
+
+    @Test
+    public void testStepIdOnCatchAndFinally() throws Exception {
+        context.addRoutes(new RouteBuilder() {
+            @Override
+            public void configure() {
+                from("direct:start")
+                        .step("s1")
+                            .doTry()
+                                .to("mock:try")
+                            .doCatch(Exception.class).id("myCatch")
+                                .to("mock:catch")
+                            .doFinally().id("myFinally")
+                                .to("mock:finally")
+                            .end()
+                        .end();
+            }
+        });
+        context.start();
+
+        for (String id : new String[] { "myCatch", "myFinally" }) {
+            Processor p = context.getProcessor(id);
+            assertNotNull(p, id);
+            StepIdAware aware = assertInstanceOf(StepIdAware.class, p, id);
+            assertEquals("s1", aware.getStepId(), id);
+        }
+    }
+}
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 8fbb0a4c0b5e..92d26282ea77 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
@@ -3095,6 +3095,24 @@ endpoint-configured query instead of the header when the 
header is not honoured.
 from the message body before it looks at the header; with both options enabled 
the header is therefore still not
 reported.
 
+=== camel-core - doTry, doCatch and doFinally
+
+A `doTry` must have one or more `doCatch` or `doFinally` blocks, otherwise the 
route fails to start with
+`doTry must have one or more doCatch or doFinally blocks`. Previously such a 
`doTry` was accepted (which turned off
+the route error handler for its steps).
+
+In the Java DSL, `onWhen` on a `doCatch` now only applies to that `doCatch`. 
Previously `onWhen` was set on every
+`doCatch` of the `doTry` (including the `doCatch` blocks of nested `doTry` 
blocks), so the `onWhen` of the last
+`doCatch` replaced the `onWhen` of the earlier `doCatch` blocks. Routes now 
behave as written.
+
+When an exception is thrown in a `doFinally` block while an exception from the 
`doTry` block is not handled, then the
+original exception is kept and the exception from the `doFinally` block is 
added as a suppressed exception.
+Previously the exception from the `doFinally` block was lost.
+
+A nested `doTry` (or a `doTry` in a route called from a `doTry`) no longer 
ends the outer `doTry` block. Previously
+steps such as a `recipientList`, `multicast` or `split` after the nested 
`doTry` used the route error handler
+(such as redeliveries and the dead letter channel), instead of the `doCatch` 
of the outer `doTry`.
+
 === camel-core - the inheritErrorHandler attribute on circuitBreaker and 
failoverLoadBalancer is now a String
 
 `CircuitBreakerDefinition.inheritErrorHandler` and 
`FailoverLoadBalancerDefinition.inheritErrorHandler` are now

Reply via email to