This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch fix/CAMEL-24974 in repository https://gitbox.apache.org/repos/asf/camel.git
commit eacf7237eb5da3c928348fffe661b682f5a08092 Author: Claus Ibsen <[email protected]> AuthorDate: Wed Sep 23 18:53:40 2026 +0200 CAMEL-24974: the failed delivery log line says where the failure is The line every Camel user reads first named the message and the exchange and nothing else: Failed delivery for (MessageId: x on ExchangeId: y). Exhausted after delivery attempt: 1 caught: java.lang.IllegalArgumentException No route, no node, no source location - so the reader turns to the message history below to find out where it happened. The facts were already on the exchange: ExchangeHelper.captureFailureOrigin stores FAILURE_ROUTE_ID, FAILURE_NODE_ID and FAILURE_LOCATION on every failure, and until now DefaultErrorRegistry was their only reader. The failure messages in RedeliveryErrorHandler now say it: Failed delivery for (MessageId: x on ExchangeId: y) at route1[to3] orders.camel.yaml:18. Exhausted after delivery attempt: 1 caught: ... The capture of the failure origin moves above the "On delivery attempt" message in the same block, so that message carries the origin too. It is still before any failure processor runs and adds its own entries to the message history, which is what its placement requires. ExchangeHelper.logIds is unchanged: it has callers outside this class, and a failure origin only means something where one was captured. Nothing is added when the properties are absent, so the line keeps its shape. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Bp3538HRBPMQkb5ta9xRaj --- .../errorhandler/RedeliveryErrorHandler.java | 57 ++++++++--- .../FailedDeliveryOriginNoSourceLocationTest.java | 105 +++++++++++++++++++++ .../camel/processor/FailedDeliveryOriginTest.java | 104 ++++++++++++++++++++ 3 files changed, 252 insertions(+), 14 deletions(-) diff --git a/core/camel-core-processor/src/main/java/org/apache/camel/processor/errorhandler/RedeliveryErrorHandler.java b/core/camel-core-processor/src/main/java/org/apache/camel/processor/errorhandler/RedeliveryErrorHandler.java index 6e325518c49b..3ffa9d1158cc 100644 --- a/core/camel-core-processor/src/main/java/org/apache/camel/processor/errorhandler/RedeliveryErrorHandler.java +++ b/core/camel-core-processor/src/main/java/org/apache/camel/processor/errorhandler/RedeliveryErrorHandler.java @@ -629,7 +629,7 @@ public abstract class RedeliveryErrorHandler extends ErrorHandlerSupport reactiveExecutor.schedule(callback); // create log message - String msg = "Failed delivery for " + ExchangeHelper.logIds(exchange); + String msg = "Failed delivery for " + ExchangeHelper.logIds(exchange) + failureOrigin(exchange); msg = msg + ". Caught: " + caught; if (isDeadLetterChannel && deadLetterUri != null) { msg = msg + ". Handled by DeadLetterChannel: [" + URISupport.sanitizeUri(deadLetterUri) + "]"; @@ -672,7 +672,7 @@ public abstract class RedeliveryErrorHandler extends ErrorHandlerSupport reactiveExecutor.schedule(callback); // create log message - String msg = "Failed delivery for " + ExchangeHelper.logIds(exchange); + String msg = "Failed delivery for " + ExchangeHelper.logIds(exchange) + failureOrigin(exchange); msg = msg + ". Caught: " + caught; if (processor != null) { if (deadLetterUri != null) { @@ -898,7 +898,7 @@ public abstract class RedeliveryErrorHandler extends ErrorHandlerSupport if (redeliveryPolicy.isLogExhausted()) { // create log message - String msg = "Failed delivery for " + ExchangeHelper.logIds(exchange); + String msg = "Failed delivery for " + ExchangeHelper.logIds(exchange) + failureOrigin(exchange); msg = msg + ". Exhausted after delivery attempt: 1 caught: " + exchange.getException(); // log that we failed delivery as we are exhausted @@ -916,7 +916,7 @@ public abstract class RedeliveryErrorHandler extends ErrorHandlerSupport } if (exchange.isRollbackOnly() || exchange.isRollbackOnlyLast()) { - String msg = "Rollback " + ExchangeHelper.logIds(exchange); + String msg = "Rollback " + ExchangeHelper.logIds(exchange) + failureOrigin(exchange); Throwable cause = exchange.getException() != null ? exchange.getException() : exchange.getProperty(ExchangePropertyKey.EXCEPTION_CAUGHT, Throwable.class); if (cause != null) { @@ -1257,7 +1257,7 @@ public abstract class RedeliveryErrorHandler extends ErrorHandlerSupport // keep the Exchange.EXCEPTION_CAUGHT as property so end user knows the caused exception // create log message - String msg = "Failed delivery for " + ExchangeHelper.logIds(exchange); + String msg = "Failed delivery for " + ExchangeHelper.logIds(exchange) + failureOrigin(exchange); msg = msg + ". Exhausted after delivery attempt: " + redeliveryCounter + " caught: " + caught; msg = msg + ". Handled and continue routing."; @@ -1370,18 +1370,19 @@ public abstract class RedeliveryErrorHandler extends ErrorHandlerSupport } } + // store the route, node and location where the exception happened, before any failure processor + // (onException, dead letter channel, ...) runs and adds its own entries to the message history, + // and before the message below is built so that it can say where the failure is (CAMEL-24974) + ExchangeHelper.captureFailureOrigin(exchange); + // only log if not failure handled or not an exhausted unit of work if (!ExchangeHelper.isFailureHandled(exchange) && !ExchangeHelper.isUnitOfWorkExhausted(exchange)) { - String msg = "Failed delivery for " + ExchangeHelper.logIds(exchange) + String msg = "Failed delivery for " + ExchangeHelper.logIds(exchange) + failureOrigin(exchange) + ". On delivery attempt: " + redeliveryCounter + " caught: " + e; logFailedDelivery(true, false, false, false, isDeadLetterChannel(), exchange, msg, e); } redeliveryCounter = incrementRedeliveryCounter(exchange); - - // store the route, node and location where the exception happened, before any failure processor - // (onException, dead letter channel, ...) runs and adds its own entries to the message history - ExchangeHelper.captureFailureOrigin(exchange); } /** @@ -1545,7 +1546,7 @@ public abstract class RedeliveryErrorHandler extends ErrorHandlerSupport reactiveExecutor.schedule(callback); // create log message - String msg = "Failed delivery for " + ExchangeHelper.logIds(exchange); + String msg = "Failed delivery for " + ExchangeHelper.logIds(exchange) + failureOrigin(exchange); msg = msg + ". Exhausted after delivery attempt: " + redeliveryCounter + " caught: " + caught; if (isDeadLetterChannel && deadLetterUri != null) { msg = msg + ". Handled by DeadLetterChannel: [" + URISupport.sanitizeUri(deadLetterUri) + "]"; @@ -1588,7 +1589,7 @@ public abstract class RedeliveryErrorHandler extends ErrorHandlerSupport reactiveExecutor.schedule(callback); // create log message - String msg = "Failed delivery for " + ExchangeHelper.logIds(exchange); + String msg = "Failed delivery for " + ExchangeHelper.logIds(exchange) + failureOrigin(exchange); msg = msg + ". Exhausted after delivery attempt: " + redeliveryCounter + " caught: " + caught; if (processor != null) { if (deadLetterUri != null) { @@ -1761,7 +1762,7 @@ public abstract class RedeliveryErrorHandler extends ErrorHandlerSupport } String msg = message; if (msg == null) { - msg = "New exception " + ExchangeHelper.logIds(exchange); + msg = "New exception " + ExchangeHelper.logIds(exchange) + failureOrigin(exchange); // special for logging the new exception if (e != null) { msg = msg + " due: " + e.getMessage(); @@ -1774,7 +1775,7 @@ public abstract class RedeliveryErrorHandler extends ErrorHandlerSupport logger.log(msg, newLogLevel); } } else if (exchange.isRollbackOnly() || exchange.isRollbackOnlyLast()) { - String msg = "Rollback " + ExchangeHelper.logIds(exchange); + String msg = "Rollback " + ExchangeHelper.logIds(exchange) + failureOrigin(exchange); Throwable cause = exchange.getException() != null ? exchange.getException() : exchange.getProperty(ExchangePropertyKey.EXCEPTION_CAUGHT, Throwable.class); if (cause != null) { @@ -2024,4 +2025,32 @@ public abstract class RedeliveryErrorHandler extends ErrorHandlerSupport ServiceHelper.stopAndShutdownServices(deadLetter, output, outputAsync, taskFactory); } + /** + * Where the failure happened, as the route and node the exchange was at and where that node is in the source, such + * as {@code at route1[to3] orders.camel.yaml:18}. Empty when nothing was captured - message history or source + * location can be off - so the message keeps its shape (CAMEL-24974). + */ + private static String failureOrigin(Exchange exchange) { + String routeId = exchange.getProperty(ExchangePropertyKey.FAILURE_ROUTE_ID, String.class); + String nodeId = exchange.getProperty(ExchangePropertyKey.FAILURE_NODE_ID, String.class); + String location = exchange.getProperty(ExchangePropertyKey.FAILURE_LOCATION, String.class); + if (routeId == null && nodeId == null && location == null) { + return ""; + } + StringBuilder sb = new StringBuilder(64).append(" at "); + if (routeId != null) { + sb.append(routeId); + } + if (nodeId != null) { + sb.append("[").append(nodeId).append("]"); + } + if (location != null) { + if (routeId != null || nodeId != null) { + sb.append(" "); + } + sb.append(location); + } + return sb.toString(); + } + } diff --git a/core/camel-core/src/test/java/org/apache/camel/processor/FailedDeliveryOriginNoSourceLocationTest.java b/core/camel-core/src/test/java/org/apache/camel/processor/FailedDeliveryOriginNoSourceLocationTest.java new file mode 100644 index 000000000000..b81561c246d3 --- /dev/null +++ b/core/camel-core/src/test/java/org/apache/camel/processor/FailedDeliveryOriginNoSourceLocationTest.java @@ -0,0 +1,105 @@ +/* + * 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.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.regex.Pattern; + +import org.apache.camel.CamelContext; +import org.apache.camel.ContextTestSupport; +import org.apache.camel.LoggingLevel; +import org.apache.camel.builder.RouteBuilder; +import org.apache.camel.spi.CamelLogger; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * With source location off there is no line to name, so the failed delivery message says the route and the node and + * stops there rather than inventing one (CAMEL-24974). + */ +public class FailedDeliveryOriginNoSourceLocationTest extends ContextTestSupport { + + /** Such as: at foo[throwException1] ContextTestSupport:480 */ + private static final Pattern ORIGIN = Pattern.compile(" at foo\\[\\w+] \\S+:\\d+"); + + private final RecordingLogger logger = new RecordingLogger(); + + @Override + protected CamelContext createCamelContext() throws Exception { + CamelContext context = super.createCamelContext(); + context.setMessageHistory(true); + context.setSourceLocationEnabled(false); + return context; + } + + @Test + public void testTheRouteAndNodeWithoutASourceLocation() throws Exception { + getMockEndpoint("mock:dead").expectedMessageCount(1); + template.sendBody("direct:start", "Hello World"); + assertMockEndpointsSatisfied(); + + String msg = lastFailure(); + assertTrue(msg.contains("Failed delivery for"), msg); + assertTrue(msg.contains(" at foo[throwException1]"), msg); + assertFalse(ORIGIN.matcher(msg).find(), "There is no line to name, so none is named: " + msg); + } + + private String lastFailure() { + assertFalse(logger.messages.isEmpty(), "The error handler should have logged the exhausted delivery"); + return logger.messages.get(logger.messages.size() - 1); + } + + @Override + protected RouteBuilder createRouteBuilder() { + return new RouteBuilder() { + @Override + public void configure() { + errorHandler(deadLetterChannel("mock:dead").logExhausted(true).logger(logger)); + + from("direct:start").routeId("foo") + .to("log:before") + .throwException(new IllegalArgumentException("Forced error")); + } + }; + } + + /** Keeps what the error handler logged, so the test can read the message instead of the log file. */ + private static final class RecordingLogger extends CamelLogger { + + private final List<String> messages = new CopyOnWriteArrayList<>(); + + private RecordingLogger() { + super(LoggerFactory.getLogger(FailedDeliveryOriginNoSourceLocationTest.class), LoggingLevel.ERROR); + } + + @Override + public void log(String message, Throwable exception, LoggingLevel loggingLevel) { + messages.add(message); + super.log(message, exception, loggingLevel); + } + + @Override + public void log(String message, LoggingLevel loggingLevel) { + messages.add(message); + super.log(message, loggingLevel); + } + } +} diff --git a/core/camel-core/src/test/java/org/apache/camel/processor/FailedDeliveryOriginTest.java b/core/camel-core/src/test/java/org/apache/camel/processor/FailedDeliveryOriginTest.java new file mode 100644 index 000000000000..05131334d92d --- /dev/null +++ b/core/camel-core/src/test/java/org/apache/camel/processor/FailedDeliveryOriginTest.java @@ -0,0 +1,104 @@ +/* + * 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.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.regex.Pattern; + +import org.apache.camel.CamelContext; +import org.apache.camel.ContextTestSupport; +import org.apache.camel.LoggingLevel; +import org.apache.camel.builder.RouteBuilder; +import org.apache.camel.spi.CamelLogger; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * The failed delivery message says where the failure happened - the route, the node, and where that node is in the + * source - so the reader does not have to work it out from the message history below (CAMEL-24974). + */ +public class FailedDeliveryOriginTest extends ContextTestSupport { + + /** Such as: at foo[throwException1] ContextTestSupport:480 */ + private static final Pattern ORIGIN = Pattern.compile(" at foo\\[\\w+] \\S+:\\d+"); + + private final RecordingLogger logger = new RecordingLogger(); + + @Override + protected CamelContext createCamelContext() throws Exception { + CamelContext context = super.createCamelContext(); + context.setMessageHistory(true); + context.setSourceLocationEnabled(true); + return context; + } + + @Test + public void testFailedDeliverySaysWhere() throws Exception { + getMockEndpoint("mock:dead").expectedMessageCount(1); + template.sendBody("direct:start", "Hello World"); + assertMockEndpointsSatisfied(); + + String msg = lastFailure(); + assertTrue(msg.contains("Failed delivery for"), msg); + assertTrue(ORIGIN.matcher(msg).find(), "Expected the route, node and source location in: " + msg); + } + + private String lastFailure() { + assertFalse(logger.messages.isEmpty(), "The error handler should have logged the exhausted delivery"); + return logger.messages.get(logger.messages.size() - 1); + } + + @Override + protected RouteBuilder createRouteBuilder() { + return new RouteBuilder() { + @Override + public void configure() { + errorHandler(deadLetterChannel("mock:dead").logExhausted(true).logger(logger)); + + from("direct:start").routeId("foo") + .to("log:before") + .throwException(new IllegalArgumentException("Forced error")); + } + }; + } + + /** Keeps what the error handler logged, so the test can read the message instead of the log file. */ + private static final class RecordingLogger extends CamelLogger { + + private final List<String> messages = new CopyOnWriteArrayList<>(); + + private RecordingLogger() { + super(LoggerFactory.getLogger(FailedDeliveryOriginTest.class), LoggingLevel.ERROR); + } + + @Override + public void log(String message, Throwable exception, LoggingLevel loggingLevel) { + messages.add(message); + super.log(message, exception, loggingLevel); + } + + @Override + public void log(String message, LoggingLevel loggingLevel) { + messages.add(message); + super.log(message, loggingLevel); + } + } +}
