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 967cce64e784 CAMEL-24974: the failed delivery log line says where the
failure is (#26804)
967cce64e784 is described below
commit 967cce64e7849a3a9e2ca637c9f4d86758d56c29
Author: Claus Ibsen <[email protected]>
AuthorDate: Wed Sep 23 21:57:03 2026 +0200
CAMEL-24974: the failed delivery log line says where the failure is (#26804)
The line every Camel user reads first named the message and the exchange
and nothing else: no route, no node, no source location. The facts were already
on the exchange - ExchangeHelper.captureFailureOrigin stores FAILURE_ROUTE_ID,
FAILURE_NODE_ID and FAILURE_LOCATION on every failure - and
DefaultErrorRegistry was their only reader.
The failure messages in RedeliveryErrorHandler now say where it happened:
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 it too; it is still before
any failure processor runs and adds its own entries to the message history.
ExchangeHelper.logIds is unchanged, and nothing is added when the properties
are absent, so the line keeps its shape.
Closes #26804
---
.../errorhandler/RedeliveryErrorHandler.java | 57 ++++++++---
.../FailedDeliveryOriginNoSourceLocationTest.java | 108 ++++++++++++++++++++
.../FailedDeliveryOriginRedeliveryTest.java | 109 +++++++++++++++++++++
.../camel/processor/FailedDeliveryOriginTest.java | 104 ++++++++++++++++++++
4 files changed, 364 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..cb294b6524ef 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..83eafbadd7ec
--- /dev/null
+++
b/core/camel-core/src/test/java/org/apache/camel/processor/FailedDeliveryOriginNoSourceLocationTest.java
@@ -0,0 +1,108 @@
+/*
+ * 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+");
+
+ /** The route and node alone, with nothing after them: node ids are
numbered per JVM, so match the shape. */
+ private static final Pattern NODE_ONLY = Pattern.compile(" at
foo\\[\\w+]\\.");
+
+ 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(NODE_ONLY.matcher(msg).find(), "Expected the route and node
in: " + 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/FailedDeliveryOriginRedeliveryTest.java
b/core/camel-core/src/test/java/org/apache/camel/processor/FailedDeliveryOriginRedeliveryTest.java
new file mode 100644
index 000000000000..2a8e8caec754
--- /dev/null
+++
b/core/camel-core/src/test/java/org/apache/camel/processor/FailedDeliveryOriginRedeliveryTest.java
@@ -0,0 +1,109 @@
+/*
+ * 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.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Every failed delivery message says where the failure happened, on a retry
as well as when the delivery is exhausted
+ * (CAMEL-24974).
+ */
+public class FailedDeliveryOriginRedeliveryTest 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 testEveryAttemptSaysWhere() throws Exception {
+ try {
+ template.sendBody("direct:start", "Hello World");
+ } catch (Exception e) {
+ // expected: the default error handler rethrows once redelivery is
exhausted
+ }
+
+ // two attempts and then the exhausted message, all built from the
captured failure origin
+ assertEquals(3, logger.messages.size(), "Expected two attempts and an
exhausted message: " + logger.messages);
+ for (String msg : logger.messages) {
+ assertTrue(msg.contains("Failed delivery for"), msg);
+ assertTrue(ORIGIN.matcher(msg).find(), "Expected the route, node
and source location in: " + msg);
+ }
+ assertTrue(logger.messages.stream().anyMatch(m -> m.contains("On
delivery attempt")), logger.messages.toString());
+ assertTrue(logger.messages.stream().anyMatch(m ->
m.contains("Exhausted after delivery attempt")),
+ logger.messages.toString());
+ }
+
+ @Override
+ protected RouteBuilder createRouteBuilder() {
+ return new RouteBuilder() {
+ @Override
+ public void configure() {
+ errorHandler(defaultErrorHandler()
+ .maximumRedeliveries(1).redeliveryDelay(0)
+
.logRetryAttempted(true).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(FailedDeliveryOriginRedeliveryTest.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);
+ }
+ }
+}
