You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by calohmn <gi...@git.apache.org> on 2018/08/22 08:09:36 UTC
[GitHub] activemq-artemis pull request #2258: ARTEMIS-2044 Add onSendError, onMessage...
GitHub user calohmn opened a pull request:
https://github.com/apache/activemq-artemis/pull/2258
ARTEMIS-2044 Add onSendError, onMessageRouteError to broker plugin
This fixes [ARTEMIS-2044](https://issues.apache.org/jira/browse/ARTEMIS-2044).
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/bsinno/activemq-artemis PR/brokerplugin_new_errorhandling_methods
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/2258.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2258
----
commit 60fea84d0a61ef254fbe5a8de192406a865ee3e3
Author: Carsten Lohmann <ca...@...>
Date: 2018-08-22T08:06:30Z
ARTEMIS-2044 Add onSendError, onMessageRouteError to ActiveMQServerMessagePlugin
----
---
[GitHub] activemq-artemis issue #2258: ARTEMIS-2044 Add onSendError, onMessageRouteEr...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/2258
LGTM, just would change the method names to on....Exception as its an exception being caught, and follows naming of other bits e.g. onException in JMS spec, CompletionListener
---
[GitHub] activemq-artemis pull request #2258: ARTEMIS-2044 Add onSendError, onMessage...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/2258
---
[GitHub] activemq-artemis pull request #2258: ARTEMIS-2044 Add onSendError, onMessage...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2258#discussion_r211868062
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/plugin/ActiveMQServerMessagePlugin.java ---
@@ -128,6 +143,21 @@ default void afterMessageRoute(Message message, RoutingContext context, boolean
}
+ /**
+ * When there was an error routing the message
+ *
+ * @param message
+ * @param context
+ * @param direct
+ * @param rejectDuplicates
+ * @param e the exception that occurred during message routing
+ * @throws ActiveMQException
+ */
+ default void onMessageRouteError(Message message, RoutingContext context, boolean direct, boolean rejectDuplicates,
--- End diff --
Nit: onMessageRouteException
---
[GitHub] activemq-artemis issue #2258: ARTEMIS-2044 Add onSendError, onMessageRouteEr...
Posted by calohmn <gi...@git.apache.org>.
Github user calohmn commented on the issue:
https://github.com/apache/activemq-artemis/pull/2258
@cshannon Do you mean the changes in `PostOfficeImpl` and `ServerSessionImpl`?
The added whitespace there is because of a new try-catch block in each class, adding indentation to the lines moved into that block.
And it's all added spaces there, no tabs. IDE is also configured to use spaces (setting coming from the imported artemis-codestyle).
---
[GitHub] activemq-artemis issue #2258: ARTEMIS-2044 Add onSendError, onMessageRouteEr...
Posted by calohmn <gi...@git.apache.org>.
Github user calohmn commented on the issue:
https://github.com/apache/activemq-artemis/pull/2258
Ok, I've renamed the methods and amended the commit.
---
[GitHub] activemq-artemis pull request #2258: ARTEMIS-2044 Add onSendError, onMessage...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2258#discussion_r211867898
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/plugin/ActiveMQServerMessagePlugin.java ---
@@ -65,6 +65,21 @@ default void afterSend(ServerSession session, Transaction tx, Message message, b
this.afterSend(tx, message, direct, noAutoCreateQueue, result);
}
+ /**
+ * When there was an exception sending the message
+ *
+ * @param session
+ * @param tx
+ * @param message
+ * @param direct
+ * @param noAutoCreateQueue
+ * @param e the exception that occurred when sending the message
+ * @throws ActiveMQException
+ */
+ default void onSendError(ServerSession session, Transaction tx, Message message, boolean direct, boolean noAutoCreateQueue,
--- End diff --
Nit: onSendException
---
[GitHub] activemq-artemis issue #2258: ARTEMIS-2044 Add onSendError, onMessageRouteEr...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/2258
Looks like the failed build in this pr is just travis, being its normal self.
I created a branch of this and caused a new build https://github.com/apache/activemq-artemis/pull/2268 and it passed.
As such ill look to merge soon
---
[GitHub] activemq-artemis issue #2258: ARTEMIS-2044 Add onSendError, onMessageRouteEr...
Posted by cshannon <gi...@git.apache.org>.
Github user cshannon commented on the issue:
https://github.com/apache/activemq-artemis/pull/2258
@calohmn - ah ok, yeah that makes sense I didn't look closely enough so if it's spaces then all should be good
---
[GitHub] activemq-artemis issue #2258: ARTEMIS-2044 Add onSendError, onMessageRouteEr...
Posted by cshannon <gi...@git.apache.org>.
Github user cshannon commented on the issue:
https://github.com/apache/activemq-artemis/pull/2258
Adding exception callbacks seems good to me, one thing I noticed is a lot of white space changes. My guess is your IDE is set to use tabs instead of spaces so I would fix that before merging
---