Improve dead-letter handling

[maccac]

It would make sense to add alternatives to just deadlettering every message if Amazon is unavailable.

For this I would propose:

• Catching all recoverable exceptions, e.g. AmazonClientException or HttpException
• Allowing the setting of a TTL on deadletter queues or; Enable spring-retry by default, or and throwing the recoverable exceptions. Of just document the properties in the README spring.rabbitmq.listener.simple.retry.enabled=true spring.rabbitmq.listener.simple.retry.initial-interval=5000 spring.rabbitmq.listener.simple.retry.max-attempts=5 spring.rabbitmq.listener.simple.retry.multiplier=1.5 spring.rabbitmq.listener.simple.retry.max-interval=30000

[maccac]

A particular annoyance with how spring has done the retry policies is that it does allow for control of the Exception types that trigger Retries.

If we wish to provide finer gain control we would need to add the following bean:

   @Bean(name = arrayOf("rabbitListenerContainerFactory"))
    fun simpleRabbitListenerContainerFactory(
            configurer: SimpleRabbitListenerContainerFactoryConfigurer,
            connectionFactory: ConnectionFactory): SimpleRabbitListenerContainerFactory {
        val factory = SimpleRabbitListenerContainerFactory()
        configurer.configure(factory, connectionFactory)

        val retryConfig = properties.listener.simple.retry
        if (retryConfig.isEnabled) {
            val builder = RetryInterceptorBuilder.stateless()
            val simpleRetryPolicy = SimpleRetryPolicy(retryConfig.maxAttempts, mapOf<Class<out Throwable>, Boolean>(
                    AmqpRejectAndDontRequeueException::class.java to false,
                    SdkBaseException::class.java to true
            ), true)
            builder.retryPolicy(simpleRetryPolicy)
                   .backOffOptions(retryConfig.initialInterval.toMillis(), retryConfig.multiplier, retryConfig.maxInterval.toMillis())
            val recoverer = RejectAndDontRequeueRecoverer()
            builder.recoverer(recoverer)
            factory.setAdviceChain(builder.build())
        }

        return factory
    }

Another approach may be to not bother with these retries and throw any SdkBaseException in a way that causes spring to nack the message to rabbit. This will just cause them to keep being replayed. The only annoying thing is that this will create a lot of logging spam, but maybe we can just tackle that by adjusting the logging settings.

[rajeshgpai]

@maccac I'd prefer not customising in this app. The applications which use this can always extend as per their needs. I'll create a pull request with the work that we did on friday to include retries if that is fine with you

[rajeshgpai]

@maccac Just noticed that we had completed a local commit on the machine that we were pairing on last friday. I'll just push that once you confirm.

[mc-tyro]

From the code we had does the following hold true?

• Recoverable errors will be retried indefinitely until the problem is fixed
• Unrecoverable errors will be dead-lettered

Whether we support customisable delays / exponential backoffs when these errors are happening can be treated as a seperate issue. I would like to ask a question on spring to see why the retry policy isn't configurable beyond the max attempts.

The only thing to be potentially cautious around would be the amount or errors being logged cause cause disk or i/O issues.

[mc-tyro]

We noticed that certain SQS errors are wrapped in a spring messaging exception. We are re-opening so as to handle this properly.