Spring Integration
  1. Spring Integration
  2. INT-2943

RequestHandlerRetryAdvice wraps endpoint exception in a MessagingException, harder to do exception classifcation based retry

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Complete
    • Affects Version/s: 2.2.1
    • Fix Version/s: 3.0 RC1
    • Component/s: Core
    • Labels:

      Description

      When adding retry advice to an endpoint you probably want to retry for some exceptions and fail immediately for other ones.

      This is supported easily by RetryTemplate with the default BasicRetryPolicy which uses an exception classifier.

      However, the RetryCallback constructed by RequestHandlerRetryAdvice wraps the actual business exception thrown by the endpoint with a MessagingException, making the default configuration-only classification impossible.

        Issue Links

          Activity

          Hide
          Gary Russell added a comment - - edited

          I assume you mean SimpleRetryPolicy

          Can I also assume you are simply asking for us to provide a retry policy that works the same way as the SimpleRetryPolicy but unwraps the exception?

          You'd still have to configure a RetryTemplate with an instance of the new policy (configured with the causes you are interested in).

          Or, we could allow setting the exception list on the Advice directly (and use the new policy), to make configuration easier.

          Show
          Gary Russell added a comment - - edited I assume you mean SimpleRetryPolicy Can I also assume you are simply asking for us to provide a retry policy that works the same way as the SimpleRetryPolicy but unwraps the exception? You'd still have to configure a RetryTemplate with an instance of the new policy (configured with the causes you are interested in). Or, we could allow setting the exception list on the Advice directly (and use the new policy), to make configuration easier.
          Hide
          Artem Bilan added a comment -

          Gary, I had in mind do not wrap real exception to MessagingException on each retry:
          1. We can wrap in the catch on the retryTemplate.execute
          2. We always have "message" attribute in the RetryContext. It's for ErrorMessageSendingRecoverer

          Show
          Artem Bilan added a comment - Gary, I had in mind do not wrap real exception to MessagingException on each retry: 1. We can wrap in the catch on the retryTemplate.execute 2. We always have "message" attribute in the RetryContext . It's for ErrorMessageSendingRecoverer
          Hide
          Assaf Berg added a comment -

          Yes, I meant SimpleRetryPolicy.
          I don't think the RetryCallback should be wrapping the real exception with a MessagingException. At least, it should be configurable.

          Show
          Assaf Berg added a comment - Yes, I meant SimpleRetryPolicy. I don't think the RetryCallback should be wrapping the real exception with a MessagingException. At least, it should be configurable.
          Hide
          zyro added a comment -

          depending on the used integration components, the real exception may not only be wrapped with a MessagingException.

          e.g. using a int-amqp:channel-inbound-adapter, the wrapping goes like ListenerExecutionFailedException --> MessagingException --> MyRealException.

          in this case, the ListenerExecutionFailedException is the one that arrives in the retryTemplate and is classified.

          Show
          zyro added a comment - depending on the used integration components, the real exception may not only be wrapped with a MessagingException. e.g. using a int-amqp:channel-inbound-adapter, the wrapping goes like ListenerExecutionFailedException --> MessagingException --> MyRealException. in this case, the ListenerExecutionFailedException is the one that arrives in the retryTemplate and is classified.
          Show
          Artem Bilan added a comment - PR: https://github.com/spring-projects/spring-integration/pull/896
          Hide
          Gary Russell added a comment - - edited

          @zyro - the AMQP issue you raise is similar, but needs a different solution because the retry interceptor is low down in the message listener container and outside of SI.

          We are looking at a number of solutions for that; please see AMQP-334

          Show
          Gary Russell added a comment - - edited @zyro - the AMQP issue you raise is similar, but needs a different solution because the retry interceptor is low down in the message listener container and outside of SI. We are looking at a number of solutions for that; please see AMQP-334
          Hide
          Gary Russell added a comment -

          Merged

          Show
          Gary Russell added a comment - Merged
          Hide
          zyro added a comment -

          nice work guys. @gary: thanks for creating AMQP-334 as amqp-specific follow-up.

          Show
          zyro added a comment - nice work guys. @gary: thanks for creating AMQP-334 as amqp-specific follow-up.

            People

            • Assignee:
              Artem Bilan
              Reporter:
              Assaf Berg
            • Votes:
              3 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: