You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by da...@apache.org on 2023/05/19 16:35:01 UTC
[camel] branch camel-3.14.x updated: CAMEL-19371: camel-core - Avoid suppressed exception added for same callsite causing deep nesting if having many redelivery attempts.
This is an automated email from the ASF dual-hosted git repository.
davsclaus pushed a commit to branch camel-3.14.x
in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/camel-3.14.x by this push:
new 34aee0c39df CAMEL-19371: camel-core - Avoid suppressed exception added for same callsite causing deep nesting if having many redelivery attempts.
34aee0c39df is described below
commit 34aee0c39df7cd0fccaaf2e671e8a7ce33f503ab
Author: Claus Ibsen <cl...@gmail.com>
AuthorDate: Fri May 19 18:15:21 2023 +0200
CAMEL-19371: camel-core - Avoid suppressed exception added for same callsite causing deep nesting if having many redelivery attempts.
---
.../component/jms/JmsTransferExceptionTest.java | 41 +++++++++++-
.../errorhandler/RedeliveryErrorHandler.java | 26 +++++++-
.../ErrorHandlerSuppressExceptionTest.java | 75 ++++++++++++++++++++++
3 files changed, 138 insertions(+), 4 deletions(-)
diff --git a/components/camel-jms/src/test/java/org/apache/camel/component/jms/JmsTransferExceptionTest.java b/components/camel-jms/src/test/java/org/apache/camel/component/jms/JmsTransferExceptionTest.java
index e34e993912a..87a0500805e 100644
--- a/components/camel-jms/src/test/java/org/apache/camel/component/jms/JmsTransferExceptionTest.java
+++ b/components/camel-jms/src/test/java/org/apache/camel/component/jms/JmsTransferExceptionTest.java
@@ -22,6 +22,8 @@ import org.apache.camel.CamelContext;
import org.apache.camel.RuntimeCamelException;
import org.apache.camel.builder.RouteBuilder;
import org.apache.camel.test.junit5.CamelTestSupport;
+import org.apache.camel.LoggingLevel;
+import org.apache.camel.spi.CamelLogger;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -33,6 +35,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
public class JmsTransferExceptionTest extends CamelTestSupport {
private static int counter;
+ protected MyErrorLogger errorLogger = new MyErrorLogger();
protected String getUri() {
return "activemq:queue:foo?transferException=true";
@@ -66,7 +69,15 @@ public class JmsTransferExceptionTest extends CamelTestSupport {
assertNotNull(e.getCause().getStackTrace(), "Should contain a remote stacktrace");
// we still try redeliver
- assertEquals(3, counter);
+ assertEquals(5, counter);
+
+ // it's all the same exception so no suppressed
+ assertEquals(0, e.getSuppressed().length);
+
+ // and check what camel logged
+ Throwable t = errorLogger.getException();
+ assertNotNull(t);
+ assertEquals(0, t.getSuppressed().length);
}
@Override
@@ -84,7 +95,7 @@ public class JmsTransferExceptionTest extends CamelTestSupport {
return new RouteBuilder() {
@Override
public void configure() throws Exception {
- errorHandler(defaultErrorHandler().maximumRedeliveries(2));
+ errorHandler(defaultErrorHandler().maximumRedeliveries(4).logger(errorLogger));
from(getUri())
.process(exchange -> {
@@ -100,4 +111,30 @@ public class JmsTransferExceptionTest extends CamelTestSupport {
};
}
+ private static class MyErrorLogger extends CamelLogger {
+
+ private Throwable exception;
+ private String message;
+ private LoggingLevel loggingLevel;
+
+ @Override
+ public void log(String message, Throwable exception, LoggingLevel loggingLevel) {
+ super.log(message, exception, loggingLevel);
+ this.message = message;
+ this.exception = exception;
+ this.loggingLevel = loggingLevel;
+ }
+
+ public Throwable getException() {
+ return exception;
+ }
+
+ public String getMessage() {
+ return message;
+ }
+
+ public LoggingLevel getLoggingLevel() {
+ return loggingLevel;
+ }
+ }
}
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 65ab7d57ad3..7ded73449fb 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
@@ -500,7 +500,18 @@ public abstract class RedeliveryErrorHandler extends ErrorHandlerSupport
}
}
if (!found) {
- e.addSuppressed(previous);
+ // okay before adding suppression then we must be sure its not referring to same method
+ // which otherwise can lead to add the same exception over and over again
+ StackTraceElement[] ste1 = e.getStackTrace();
+ StackTraceElement[] ste2 = previous.getStackTrace();
+ boolean same = false;
+ if (ste1 != null && ste2 != null && ste1.length > 0 && ste2.length > 0) {
+ same = ste1[0].getClassName().equals(ste2[0].getClassName())
+ && ste1[0].getLineNumber() == ste2[0].getLineNumber();
+ }
+ if (!same) {
+ e.addSuppressed(previous);
+ }
}
}
@@ -984,7 +995,18 @@ public abstract class RedeliveryErrorHandler extends ErrorHandlerSupport
}
}
if (!found) {
- e.addSuppressed(previous);
+ // okay before adding suppression then we must be sure its not referring to same method
+ // which otherwise can lead to add the same exception over and over again
+ StackTraceElement[] ste1 = e.getStackTrace();
+ StackTraceElement[] ste2 = previous.getStackTrace();
+ boolean same = false;
+ if (ste1 != null && ste2 != null && ste1.length > 0 && ste2.length > 0) {
+ same = ste1[0].getClassName().equals(ste2[0].getClassName())
+ && ste1[0].getLineNumber() == ste2[0].getLineNumber();
+ }
+ if (!same) {
+ e.addSuppressed(previous);
+ }
}
}
diff --git a/core/camel-core/src/test/java/org/apache/camel/processor/onexception/ErrorHandlerSuppressExceptionTest.java b/core/camel-core/src/test/java/org/apache/camel/processor/onexception/ErrorHandlerSuppressExceptionTest.java
new file mode 100644
index 00000000000..c9d3503db1b
--- /dev/null
+++ b/core/camel-core/src/test/java/org/apache/camel/processor/onexception/ErrorHandlerSuppressExceptionTest.java
@@ -0,0 +1,75 @@
+/*
+ * 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 org.apache.camel.ContextTestSupport;
+import org.apache.camel.Exchange;
+import org.apache.camel.Processor;
+import org.apache.camel.builder.RouteBuilder;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class ErrorHandlerSuppressExceptionTest extends ContextTestSupport {
+
+ @Test
+ public void testSuppressException() throws Exception {
+ Exchange out = template.send("direct:start", new Processor() {
+ @Override
+ public void process(Exchange exchange) throws Exception {
+ exchange.getIn().setBody("Hello World");
+ }
+ });
+
+ Assertions.assertTrue(out.isFailed());
+ Exception t = out.getException();
+ Assertions.assertNotNull(t);
+ Assertions.assertEquals("Forced error during handling", t.getMessage());
+ // only 1 suppressed to avoid the same exception being added multiple times
+ Assertions.assertEquals(1, t.getSuppressed().length);
+ Assertions.assertEquals("Root exception", t.getSuppressed()[0].getMessage());
+ }
+
+ @Override
+ protected RouteBuilder createRouteBuilder() throws Exception {
+ return new RouteBuilder() {
+ @Override
+ public void configure() throws Exception {
+ onException(Exception.class).maximumRedeliveries(3).redeliveryDelay(0)
+ .process(new Processor() {
+ @Override
+ public void process(Exchange exchange) throws Exception {
+ // throw a new exception while handling an exception
+ // this should not leak with the same exception being nested
+ // as suppressed exception
+ throw new IllegalArgumentException("Forced error during handling");
+ }
+ });
+
+ from("direct:start")
+ .process(new Processor() {
+ @Override
+ public void process(Exchange exchange) throws Exception {
+ throw new IOException("Root exception");
+ }
+ });
+ }
+ };
+ }
+
+}