JSchoreels commented on a change in pull request #4384:
URL: https://github.com/apache/camel/pull/4384#discussion_r501596540
##########
File path:
components/camel-rabbitmq/src/main/java/org/apache/camel/component/rabbitmq/RabbitConsumer.java
##########
@@ -326,9 +340,13 @@ public void reconnect() throws Exception {
} else if (channel == null || !isAutomaticRecoveryEnabled()) {
LOG.info("Attempting to open a new rabbitMQ channel");
Connection conn = consumer.getConnection();
- channel = openChannel(conn);
- // Register the channel to the tag
- start();
+ try {
+ stop();
Review comment:
The case I had during my integration test was that in the stop(), there
is a call to "basicCancel", which throws an unhandled exception if the channel
or the connection was already closed, which lead to "start()" never called, and
thus the consumers never handled new message afterward.
Maybe the exception handling could be more specific to the basicCancel and
not on all the "stop()", but I thought that start should always be called no
matter how stop finishes.
I'm open to other way of handling that.
##########
File path:
components/camel-rabbitmq/src/main/java/org/apache/camel/component/rabbitmq/RabbitConsumer.java
##########
@@ -326,9 +340,13 @@ public void reconnect() throws Exception {
} else if (channel == null || !isAutomaticRecoveryEnabled()) {
LOG.info("Attempting to open a new rabbitMQ channel");
Connection conn = consumer.getConnection();
- channel = openChannel(conn);
- // Register the channel to the tag
- start();
+ try {
+ stop();
Review comment:
The case I had during my integration test was that in the stop(), there
is a call to "basicCancel", which throws an unhandled exception if the channel
or the connection was already closed (in my case due to the fact I called
this.channel.getConnection().close() with the debugger during the handling of
the message), which lead to "start()" never called, and thus the consumers
never handled new message afterward.
Maybe the exception handling could be more specific to the basicCancel and
not on all the "stop()", but I thought that start should always be called no
matter how stop finishes.
I'm open to other way of handling that.
##########
File path:
components/camel-rabbitmq/src/main/java/org/apache/camel/component/rabbitmq/RabbitConsumer.java
##########
@@ -326,9 +340,13 @@ public void reconnect() throws Exception {
} else if (channel == null || !isAutomaticRecoveryEnabled()) {
LOG.info("Attempting to open a new rabbitMQ channel");
Connection conn = consumer.getConnection();
- channel = openChannel(conn);
- // Register the channel to the tag
- start();
+ try {
+ stop();
Review comment:
The case I had during my integration test was that in the stop(), there
is a call to "basicCancel", which throws an unhandled exception
(AlreadyClosedException if I recall correctly) if the channel or the connection
was already closed (in my case due to the fact I called
this.channel.getConnection().close() with the debugger during the handling of
the message), which lead to "start()" never called, and thus the consumers
never handled new message afterward.
Maybe the exception handling could be more specific to the basicCancel and
not on all the "stop()", but I thought that start should always be called no
matter how stop finishes.
I'm open to other way of handling that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
