-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Fatal exceptions in Combining Blocking and Non-Blocking Retries #2457
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Closing as invalid - the exception classifier always goes up the class hierarchy to find a classification, before traversing causes. |
Reinstated; I found a solution. |
garyrussell
added a commit
to garyrussell/spring-kafka
that referenced
this issue
Oct 24, 2022
Resolves spring-projects#2457 Previously, classifying `RuntimeException` either for no retries or for blocking retries would cause undesirable effects - its classification would be found first because the classifier first traverses up the class hierarchy to find a match before traversing down the cause links. When classifying exception for retry, unwrap the 'LEFE' cause from a `TimestampedException` and/or `ListenerExecutionFailedException` so that the classification is made on that cause. By default, the retry classifier has no classified exceptions when used in "classify for retry" mode (instead of the default "classify for no retry" mode). This means that, if `retryOn(RuntimeException.class)` is used, then all `RuntimeException`s will be retried (including those that are usually considered fatal). With mixed blocking/non-blocking retries, change the behavior to include the standard fatal exceptions in case the user configures all `RuntimeException`s to use blocking retries. Also tested with a Boot app to see that a conversion exception bypasses all retries and goes straight to the DLT.
garyrussell
added a commit
to garyrussell/spring-kafka
that referenced
this issue
Oct 24, 2022
Resolves spring-projects#2457 Previously, classifying `RuntimeException` either for no retries or for blocking retries would cause undesirable effects - its classification would be found first because the classifier first traverses up the class hierarchy to find a match before traversing down the cause links. When classifying exception for retry, unwrap the 'LEFE' cause from a `TimestampedException` and/or `ListenerExecutionFailedException` so that the classification is made on that cause. By default, the retry classifier has no classified exceptions when used in "classify for retry" mode (instead of the default "classify for no retry" mode). This means that, if `retryOn(RuntimeException.class)` is used, then all `RuntimeException`s will be retried (including those that are usually considered fatal). With mixed blocking/non-blocking retries, change the behavior to include the standard fatal exceptions in case the user configures all `RuntimeException`s to use blocking retries. Also tested with a Boot app to see that a conversion exception bypasses all retries and goes straight to the DLT.
artembilan
pushed a commit
that referenced
this issue
Oct 24, 2022
Resolves #2457 Previously, classifying `RuntimeException` either for no retries or for blocking retries would cause undesirable effects - its classification would be found first because the classifier first traverses up the class hierarchy to find a match before traversing down the cause links. When classifying exception for retry, unwrap the 'LEFE' cause from a `TimestampedException` and/or `ListenerExecutionFailedException` so that the classification is made on that cause. By default, the retry classifier has no classified exceptions when used in "classify for retry" mode (instead of the default "classify for no retry" mode). This means that, if `retryOn(RuntimeException.class)` is used, then all `RuntimeException`s will be retried (including those that are usually considered fatal). With mixed blocking/non-blocking retries, change the behavior to include the standard fatal exceptions in case the user configures all `RuntimeException`s to use blocking retries. Also tested with a Boot app to see that a conversion exception bypasses all retries and goes straight to the DLT.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Discussed in #2455
Originally posted by jgslima October 20, 2022
Documentation reference: link.
I was not able to understand how to configure something like this:
RuntimeException
(which I have done by makingblockingRetries.retryOn(RuntimeException.class)
);RuntimeException
to go directly do DLT. For that, I tried to add them inmanageNonBlockingFatalExceptions()
.But, as far as I can verify here, exceptions added to
manageNonBlockingFatalExceptions()
, are used to skip just the "non blocking" retries, not the blocking ones. For example, when just makingblockingRetries.retryOn(RuntimeException.class)
if the listener throws aMessageConversionException
(which is a pre-defined fatal exception) it goes through the blocking retries (and after that, it correctly skips the non blocking retries). This is not in accordance to the example code of the documentation, in regard toShouldSkipBothRetriesException
.Also, I think the documentation above is a bit confusing. In the code example "Here’s an example with both configurations working together:", I do not understand how the behaviour for
ShouldRetryOnlyBlockingException
andShouldRetryViaBothException
would be different, because both are just being passed inblockingRetries.retryOn()
.Thank you.
The text was updated successfully, but these errors were encountered: