[#1199] Non transient remote exceptions transport fix #1743
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sandjelkovic
added
Type: Bug
|
|
|
SonarCloud Quality Gate failed. |
piyushchauhan2011
approved these changes
Mar 28, 2021
smcvb
requested changes
Apr 8, 2021
There was a problem hiding this comment.
Bunch of comments worth going through. Conceptually I have a good feeling about this addition though! Did you per chance also to a local form of integration test to validate retries occurred when using AxonServer/JGroups/SpringCloud to distribute the command bus?
...ework/axonserver/connector/command/AxonServerNonTransientRemoteCommandHandlingException.java
Outdated
Show resolved
Hide resolved
...framework/axonserver/connector/query/AxonServerNonTransientRemoteQueryHandlingException.java
Outdated
Show resolved
Hide resolved
...framework/axonserver/connector/query/AxonServerNonTransientRemoteQueryHandlingException.java
Outdated
Show resolved
Hide resolved
...ework/axonserver/connector/command/AxonServerNonTransientRemoteCommandHandlingException.java
Outdated
Show resolved
Hide resolved
...ework/axonserver/connector/command/AxonServerNonTransientRemoteCommandHandlingException.java
Outdated
Show resolved
Hide resolved
...onnector/src/test/java/org/axonframework/axonserver/connector/util/ErrorCodeDeciderTest.java
Outdated
Show resolved
Hide resolved
messaging/src/main/java/org/axonframework/commandhandling/distributed/ReplyMessage.java
Outdated
Show resolved
Hide resolved
messaging/src/main/java/org/axonframework/messaging/RemoteExceptionDescription.java
Outdated
Show resolved
Hide resolved
messaging/src/main/java/org/axonframework/messaging/RemoteExceptionDescription.java
Outdated
Show resolved
Hide resolved
messaging/src/main/java/org/axonframework/messaging/RemoteNonTransientHandlingException.java
Outdated
Show resolved
Hide resolved
|
Also, SonarCloud is complaining about some issues too. So please take a look at those too :-) |
…er/connector/query/AxonServerNonTransientRemoteQueryHandlingException.java Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
…er/connector/command/AxonServerNonTransientRemoteCommandHandlingException.java Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
smcvb
requested changes
May 10, 2021
...framework/axonserver/connector/query/AxonServerNonTransientRemoteQueryHandlingException.java
Outdated
Show resolved
Hide resolved
...connector/src/main/java/org/axonframework/axonserver/connector/query/AxonServerQueryBus.java
Outdated
Show resolved
Hide resolved
...connector/src/main/java/org/axonframework/axonserver/connector/query/AxonServerQueryBus.java
Outdated
Show resolved
Hide resolved
messaging/src/main/java/org/axonframework/messaging/RemoteExceptionDescription.java
Outdated
Show resolved
Hide resolved
...framework/axonserver/connector/query/AxonServerNonTransientRemoteQueryHandlingException.java
Outdated
Show resolved
Hide resolved
...ework/axonserver/connector/command/AxonServerNonTransientRemoteCommandHandlingException.java
Outdated
Show resolved
Hide resolved
smcvb
reviewed
May 10, 2021
...ework/axonserver/connector/command/AxonServerNonTransientRemoteCommandHandlingException.java
Outdated
Show resolved
Hide resolved
...framework/axonserver/connector/query/AxonServerNonTransientRemoteQueryHandlingException.java
Outdated
Show resolved
Hide resolved
smcvb
approved these changes
May 11, 2021
There was a problem hiding this comment.
Believe that's it. Thanks for the effort @sandjelkovic! Let's wait for a second pair of eyes, to see if he/she comes up with the same conclusion :-)
|
SonarCloud Quality Gate failed. |
|
SonarCloud complains about the inheritance level of the Exceptions added in this PR. |
smcvb
added
Status: Resolved
smcvb
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR will resolve #1199
Exception transiency property wasn't correctly sent or received during transportation between applications.
To be able to distinguish which type of exception was thrown at the handling side, the command/query sending side will deserialize the message and expose this information in the
causefield of *ExecutionExceptions. To support this, a new exceptionRemoteNonTransientHandlingExceptionhas been created to indicate the non-transient nature of a remote exception.The result is that the
CommandExecutionExceptionandQueryExecutionExceptionwill remain the same, but the retry callback and retry scheduler will be able to determine if a command should be retried based on the cause ofCommandExecutionException