You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "gemmellr (via GitHub)" <gi...@apache.org> on 2023/04/10 11:45:12 UTC
[GitHub] [activemq-artemis] gemmellr commented on a diff in pull request #4429: ARTEMIS-4234 Fix some issue with Embedded Resource send and receive
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org