Skip to content

Commit ba044ff

Browse files
allthingssecurityclaude
allthingssecurity
and
claude
authored
CAMEL-26333: camel-core - doFinally runs only its first step after a called route's error handler handled a failure (#28756)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 5938b0b commit ba044ff

4 files changed

Lines changed: 379 additions & 2 deletions

File tree

‎core/camel-core-processor/src/main/java/org/apache/camel/processor/FinallyProcessor.java‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import org.apache.camel.AsyncCallback;
2020
import org.apache.camel.Exchange;
21+
import org.apache.camel.ExchangeExtension;
2122
import org.apache.camel.ExchangePropertyKey;
2223
import org.apache.camel.Processor;
2324
import org.apache.camel.Traceable;
@@ -60,8 +61,26 @@ public boolean process(final Exchange exchange, final AsyncCallback callback) {
6061
}
6162
}
6263

64+
// a route called from the doTry (such as with direct) can have handled a failure with its error handler (dead
65+
// letter channel or a handled onException), which marks the exchange as handled by the error handler and as
66+
// failure handled. The route then does not continue after the doTry, but the doFinally block must still run all
67+
// its steps (a pipeline stops before its second step when the exchange is handled by the error handler), and a
68+
// failure in a route that the block calls must be handled by that route's error handler as usual (which does not
69+
// handle a new failure of an exchange that is failure handled). So the marks are cleared while the block runs,
70+
// and set again afterwards (as the onCompletion does)
71+
final ExchangeExtension ext = exchange.getExchangeExtension();
72+
// a primitive "set and true" snapshot, unlike the nullable Boolean of OnCompletionProcessor: only a failure that
73+
// an error handler handled needs clearing and restoring here
74+
final boolean errorHandlerHandled = ext.isErrorHandlerHandledSet() && ext.isErrorHandlerHandled();
75+
final boolean failureHandled = errorHandlerHandled && ext.isFailureHandled();
76+
if (errorHandlerHandled) {
77+
ext.setErrorHandlerHandled(null);
78+
ext.setFailureHandled(false);
79+
}
80+
6381
// continue processing
64-
return processor.process(exchange, new FinallyAsyncCallback(exchange, callback, exception));
82+
return processor.process(exchange,
83+
new FinallyAsyncCallback(exchange, callback, exception, errorHandlerHandled, failureHandled));
6584
}
6685

6786
@Override
@@ -114,16 +133,34 @@ private static final class FinallyAsyncCallback implements AsyncCallback {
114133
private final Exchange exchange;
115134
private final AsyncCallback callback;
116135
private final Exception exception;
136+
private final boolean errorHandlerHandled;
137+
private final boolean failureHandled;
117138

118-
FinallyAsyncCallback(Exchange exchange, AsyncCallback callback, Exception exception) {
139+
FinallyAsyncCallback(Exchange exchange, AsyncCallback callback, Exception exception,
140+
boolean errorHandlerHandled, boolean failureHandled) {
119141
this.exchange = exchange;
120142
this.callback = callback;
121143
this.exception = exception;
144+
this.errorHandlerHandled = errorHandlerHandled;
145+
this.failureHandled = failureHandled;
122146
}
123147

124148
@Override
125149
public void done(boolean doneSync) {
126150
try {
151+
if (errorHandlerHandled) {
152+
// the doFinally block has run, so the route does not continue after the doTry, as before
153+
// (unless the block marked it itself, such as a route it called that failed)
154+
final ExchangeExtension ext = exchange.getExchangeExtension();
155+
if (!ext.isErrorHandlerHandledSet()) {
156+
ext.setErrorHandlerHandled(true);
157+
}
158+
// one-way restore, unlike OnCompletionProcessor: set again but never cleared, so a failure that the
159+
// block handled itself stays marked (errorHandlerHandled without failureHandled does not occur)
160+
if (failureHandled) {
161+
ext.setFailureHandled(true);
162+
}
163+
}
127164
if (exception == null) {
128165
// only clear the failure details when the doFinally did not fail
129166
// (the doTry restores the failure details of an earlier failure)

‎core/camel-core/src/test/java/org/apache/camel/processor/DoFinallyAfterHandledCalledRouteTest.java‎

Lines changed: 204 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.camel.processor;
18+
19+
import java.time.Duration;
20+
21+
import org.apache.camel.ContextTestSupport;
22+
import org.apache.camel.Exchange;
23+
import org.apache.camel.builder.RouteBuilder;
24+
import org.junit.jupiter.api.Test;
25+
26+
import static org.junit.jupiter.api.Assertions.assertEquals;
27+
import static org.junit.jupiter.api.Assertions.assertNotEquals;
28+
import static org.junit.jupiter.api.Assertions.assertNull;
29+
import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
30+
31+
/**
32+
* The doTry calls a route whose error handler handles a failure (dead letter channel or a handled onException). The
33+
* doFinally block must run all its steps. The caller does not continue routing after the doTry, as before
34+
* (CAMEL-19441).
35+
*/
36+
class DoFinallyAfterHandledCalledRouteTest extends ContextTestSupport {
37+
38+
@Override
39+
public boolean isUseRouteBuilder() {
40+
return false;
41+
}
42+
43+
private Exchange send(String errorHandler, boolean calleeFails) throws Exception {
44+
context.addRoutes(new RouteBuilder() {
45+
@Override
46+
public void configure() {
47+
switch (errorHandler) {
48+
case "dlc" -> errorHandler(deadLetterChannel("mock:dead").redeliveryDelay(0));
49+
case "dlcAsync" -> errorHandler(deadLetterChannel("mock:dead").maximumRedeliveries(1)
50+
.redeliveryDelay(10).asyncDelayedRedelivery());
51+
default -> onException(IllegalArgumentException.class).handled(true).to("mock:dead");
52+
}
53+
54+
from("direct:start")
55+
.process(e -> e.setProperty("startThread", Thread.currentThread().getName()))
56+
.doTry()
57+
.to("direct:callee")
58+
.to("mock:afterCallee")
59+
.doCatch(Exception.class)
60+
.to("mock:catch")
61+
.doFinally()
62+
.to("mock:f1")
63+
.to("mock:f2")
64+
.setHeader("f3", constant("yes"))
65+
.process(e -> e.setProperty("finallyThread", Thread.currentThread().getName()))
66+
.to("mock:f4")
67+
.end()
68+
.to("mock:end");
69+
70+
if (calleeFails) {
71+
from("direct:callee").setBody(constant("in callee"))
72+
.throwException(new IllegalArgumentException("Forced"));
73+
} else {
74+
from("direct:callee").setBody(constant("in callee"));
75+
}
76+
}
77+
});
78+
context.start();
79+
80+
return assertTimeoutPreemptively(Duration.ofSeconds(10),
81+
() -> template.send("direct:start", e -> e.getMessage().setBody("Hello")), "the send did not complete");
82+
}
83+
84+
private void assertAllFinallySteps(Exchange out) {
85+
assertEquals(1, getMockEndpoint("mock:f1").getReceivedCounter(), "mock:f1");
86+
assertEquals(1, getMockEndpoint("mock:f2").getReceivedCounter(), "mock:f2");
87+
assertEquals(1, getMockEndpoint("mock:f4").getReceivedCounter(), "mock:f4");
88+
assertEquals("yes", out.getMessage().getHeader("f3"), "the header set by the doFinally");
89+
}
90+
91+
private void assertHandledByCalledRoute(Exchange out) {
92+
assertNull(out.getException(), "the called route handled the failure");
93+
assertEquals(1, getMockEndpoint("mock:dead").getReceivedCounter(), "mock:dead");
94+
assertEquals(0, getMockEndpoint("mock:afterCallee").getReceivedCounter(), "mock:afterCallee");
95+
assertEquals(0, getMockEndpoint("mock:catch").getReceivedCounter(), "there is no exception to catch");
96+
// the caller does not continue routing after the doTry (CAMEL-19441)
97+
assertEquals(0, getMockEndpoint("mock:end").getReceivedCounter(), "mock:end");
98+
}
99+
100+
@Test
101+
void deadLetterChannel() throws Exception {
102+
Exchange out = send("dlc", true);
103+
assertHandledByCalledRoute(out);
104+
assertAllFinallySteps(out);
105+
}
106+
107+
@Test
108+
void deadLetterChannelAsyncDelayedRedelivery() throws Exception {
109+
Exchange out = send("dlcAsync", true);
110+
assertHandledByCalledRoute(out);
111+
assertAllFinallySteps(out);
112+
// the redelivery, and so the doFinally block, ran on another thread than the one that started the route
113+
assertNotEquals(out.getProperty("startThread"), out.getProperty("finallyThread"));
114+
}
115+
116+
@Test
117+
void handledOnException() throws Exception {
118+
Exchange out = send("handled", true);
119+
assertHandledByCalledRoute(out);
120+
assertAllFinallySteps(out);
121+
}
122+
123+
@Test
124+
void controlCalledRouteDoesNotFail() throws Exception {
125+
Exchange out = send("dlc", false);
126+
assertNull(out.getException());
127+
assertEquals(0, getMockEndpoint("mock:dead").getReceivedCounter(), "mock:dead");
128+
assertEquals(1, getMockEndpoint("mock:afterCallee").getReceivedCounter(), "mock:afterCallee");
129+
assertAllFinallySteps(out);
130+
assertEquals(1, getMockEndpoint("mock:end").getReceivedCounter(), "mock:end");
131+
}
132+
133+
@Test
134+
void routeCalledFromTheDoFinallyHandlesItsOwnFailure() throws Exception {
135+
context.addRoutes(new RouteBuilder() {
136+
@Override
137+
public void configure() {
138+
errorHandler(deadLetterChannel("mock:dead").redeliveryDelay(0));
139+
140+
from("direct:start")
141+
.doTry()
142+
.to("direct:callee")
143+
.doFinally()
144+
.to("mock:f1")
145+
.to("direct:callee2")
146+
.to("mock:f2")
147+
.end()
148+
.to("mock:end");
149+
150+
from("direct:callee").throwException(new IllegalArgumentException("Forced"));
151+
from("direct:callee2").throwException(new IllegalStateException("Forced in doFinally"));
152+
}
153+
});
154+
context.start();
155+
156+
Exchange out = assertTimeoutPreemptively(Duration.ofSeconds(10),
157+
() -> template.send("direct:start", e -> e.getMessage().setBody("Hello")), "the send did not complete");
158+
// the dead letter channel of the route called from the doFinally handles that failure too, as when the doTry
159+
// did not fail before
160+
assertNull(out.getException(), "the called routes handled their failures");
161+
assertEquals(2, getMockEndpoint("mock:dead").getReceivedCounter(), "mock:dead");
162+
assertEquals(1, getMockEndpoint("mock:f1").getReceivedCounter(), "mock:f1");
163+
// a handled failure of a called route stops the caller, also in the doFinally block
164+
assertEquals(0, getMockEndpoint("mock:f2").getReceivedCounter(), "mock:f2");
165+
assertEquals(0, getMockEndpoint("mock:end").getReceivedCounter(), "mock:end");
166+
}
167+
168+
@Test
169+
void routeCalledFromTheDoFinallyContinuesItsOwnFailure() throws Exception {
170+
context.addRoutes(new RouteBuilder() {
171+
@Override
172+
public void configure() {
173+
errorHandler(deadLetterChannel("mock:dead").redeliveryDelay(0));
174+
onException(NumberFormatException.class).continued(true);
175+
176+
from("direct:start")
177+
.doTry()
178+
.to("direct:callee")
179+
.doFinally()
180+
.to("mock:f1")
181+
.to("direct:callee2")
182+
.to("mock:f2")
183+
.end()
184+
.to("mock:end");
185+
186+
from("direct:callee").throwException(new IllegalArgumentException("Forced"));
187+
from("direct:callee2").throwException(new NumberFormatException("Forced in doFinally"))
188+
.to("mock:callee2End");
189+
}
190+
});
191+
context.start();
192+
193+
Exchange out = assertTimeoutPreemptively(Duration.ofSeconds(10),
194+
() -> template.send("direct:start", e -> e.getMessage().setBody("Hello")), "the send did not complete");
195+
assertNull(out.getException(), "the called routes handled or continued their failures");
196+
assertEquals(1, getMockEndpoint("mock:dead").getReceivedCounter(), "mock:dead");
197+
// the onException of the route called from the doFinally continues its failure
198+
assertEquals(1, getMockEndpoint("mock:callee2End").getReceivedCounter(), "mock:callee2End");
199+
assertEquals(1, getMockEndpoint("mock:f1").getReceivedCounter(), "mock:f1");
200+
assertEquals(1, getMockEndpoint("mock:f2").getReceivedCounter(), "mock:f2");
201+
// the caller does not continue routing after the doTry (CAMEL-19441)
202+
assertEquals(0, getMockEndpoint("mock:end").getReceivedCounter(), "mock:end");
203+
}
204+
}

0 commit comments

Comments
 (0)