gnodet-bot commented on code in PR #29344:
URL: https://github.com/apache/camel/pull/29344#discussion_r4494308352
##########
components/camel-activemq/src/main/java/org/apache/camel/component/activemq/ActiveMQComponent.java:
##########
@@ -258,7 +270,17 @@ protected void doStop() throws Exception {
}
singleConnectionFactoryList.clear();
- super.doStop();
+ super.doShutdown();
+ }
+
+ private static void invokePooledConnectionFactory(Object
pooledConnectionFactory, String method) {
+ try {
+ // invoke the method if it exists
+ Method m = pooledConnectionFactory.getClass().getMethod(method);
+ org.apache.camel.support.ObjectHelper.invokeMethod(m,
pooledConnectionFactory);
+ } catch (Exception e) {
+ // ignore
+ }
Review Comment:
🔍 **Low:** The single `catch (Exception e)` swallows both
`NoSuchMethodException` (acceptable — the pool may not have this method) and
any exception thrown during invocation (e.g. a pool that fails to release
connections during `clear()`/`stop()`). Splitting the catches makes the "method
not found" case explicit and logs real failures at debug level for diagnostics.
The stop/shutdown sequence can still continue on error.
```suggestion
private static void invokePooledConnectionFactory(Object
pooledConnectionFactory, String method) {
try {
// invoke the method if it exists
Method m = pooledConnectionFactory.getClass().getMethod(method);
org.apache.camel.support.ObjectHelper.invokeMethod(m,
pooledConnectionFactory);
} catch (NoSuchMethodException e) {
// the connection factory does not have this method, ignore
} catch (Exception e) {
LOG.debug("Error invoking {} on pooled connection factory: {}",
method, e.getMessage(), e);
}
}
```
##########
components/camel-activemq6/src/main/java/org/apache/camel/component/activemq6/ActiveMQComponent.java:
##########
@@ -258,7 +270,17 @@ protected void doStop() throws Exception {
}
singleConnectionFactoryList.clear();
- super.doStop();
+ super.doShutdown();
+ }
+
+ private static void invokePooledConnectionFactory(Object
pooledConnectionFactory, String method) {
+ try {
+ // invoke the method if it exists
+ Method m = pooledConnectionFactory.getClass().getMethod(method);
+ org.apache.camel.support.ObjectHelper.invokeMethod(m,
pooledConnectionFactory);
+ } catch (Exception e) {
+ // ignore
+ }
Review Comment:
🔍 **Low:** The single `catch (Exception e)` swallows both
`NoSuchMethodException` (acceptable — the pool may not have this method) and
any exception thrown during invocation (e.g. a pool that fails to release
connections during `clear()`/`stop()`). Splitting the catches makes the "method
not found" case explicit and logs real failures at debug level for diagnostics.
The stop/shutdown sequence can still continue on error.
```suggestion
private static void invokePooledConnectionFactory(Object
pooledConnectionFactory, String method) {
try {
// invoke the method if it exists
Method m = pooledConnectionFactory.getClass().getMethod(method);
org.apache.camel.support.ObjectHelper.invokeMethod(m,
pooledConnectionFactory);
} catch (NoSuchMethodException e) {
// the connection factory does not have this method, ignore
} catch (Exception e) {
LOG.debug("Error invoking {} on pooled connection factory: {}",
method, e.getMessage(), e);
}
}
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
