gemmellr commented on code in PR #4429:
URL: https://github.com/apache/activemq-artemis/pull/4429#discussion_r1161654982
##########
artemis-junit/artemis-junit-commons/src/main/java/org/apache/activemq/artemis/junit/EmbeddedActiveMQDelegate.java:
##########
@@ -626,43 +625,51 @@ public void sendMessage(SimpleString address,
ClientMessage message) {
public ClientMessage receiveMessage(SimpleString address, long timeout,
boolean browseOnly) {
checkSession();
- ClientConsumer consumer = null;
- try {
- consumer = session.createConsumer(address, browseOnly);
- } catch (ActiveMQException amqEx) {
- throw new EmbeddedActiveMQResourceException(String.format("Failed
to create consumer for %s",
-
address.toString()),
- amqEx);
- }
-
- ClientMessage message = null;
- if (timeout > 0) {
- try {
- message = consumer.receive(timeout);
- } catch (ActiveMQException amqEx) {
- throw new
EmbeddedActiveMQResourceException(String.format("ClientConsumer.receive(
timeout = %d ) for %s failed",
-
timeout, address.toString()),
- amqEx);
+ EmbeddedActiveMQResourceException failureCause = null;
+
+ try (ClientConsumer consumer = session.createConsumer(address,
browseOnly)) {
+ ClientMessage message = null;
+ if (timeout > 0) {
+ try {
+ message = consumer.receive(timeout);
+ } catch (ActiveMQException amqEx) {
+ failureCause = new
EmbeddedActiveMQResourceException(String.format("ClientConsumer.receive(
timeout = %d ) for %s failed",
+
timeout, address.toString()), amqEx);
+ }
+ } else if (timeout == 0) {
+ try {
+ message = consumer.receiveImmediate();
+ } catch (ActiveMQException amqEx) {
+ failureCause = new
EmbeddedActiveMQResourceException(String.format("ClientConsumer.receiveImmediate()
for %s failed",
+
address.toString()), amqEx);
+ }
+ } else {
+ try {
+ message = consumer.receive();
+ } catch (ActiveMQException amqEx) {
+ failureCause = new
EmbeddedActiveMQResourceException(String.format("ClientConsumer.receive() for
%s failed",
+
address.toString()), amqEx);
+ }
}
- } else if (timeout == 0) {
- try {
- message = consumer.receiveImmediate();
- } catch (ActiveMQException amqEx) {
- throw new
EmbeddedActiveMQResourceException(String.format("ClientConsumer.receiveImmediate()
for %s failed",
-
address.toString()),
- amqEx);
+
+ if (message != null) {
+ try {
+ message.acknowledge();
+ } catch (ActiveMQException amqEx) {
+ failureCause = new
EmbeddedActiveMQResourceException(String.format("ClientMessage.acknowledge()
for %s from %s failed",
+
message, address.toString()), amqEx);
+ }
}
- } else {
- try {
- message = consumer.receive();
- } catch (ActiveMQException amqEx) {
- throw new
EmbeddedActiveMQResourceException(String.format("ClientConsumer.receive() for
%s failed",
-
address.toString()),
- amqEx);
+
+ return message;
+ } catch (ActiveMQException amqEx) {
Review Comment:
It only seems to look at and throws 'failureCause' if it catches
ActiveMQException in the outer catch wrapper. All the inner catch wrappers
swallow ActiveMQException while setting failureCause, meaning that failureCause
then wont be looked at since if they failed 'message' will just be null and the
overall method will have successfully returned null just above this.
--
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]
