Create a possibility to choose which exceptions should be sent to the retry queue and then discarded

[juliofalbo]

Steps:

1: Create an attribute in the @EnableRabbitRetryAndDlq annotation for the Exceptions that will be discarded after the max attempts.

https://github.com/Tradeshift/spring-rabbitmq-tuning/blob/master/src/main/java/com/tradeshift/amqp/annotation/EnableRabbitRetryAndDlq.java#L14 2: Change the logic that is responsible to choose which message should go to Retry queue.

https://github.com/Tradeshift/spring-rabbitmq-tuning/blob/master/src/main/java/com/tradeshift/amqp/annotation/EnableRabbitRetryAndDlqAspect.java#L48 Example:

@RabbitListener(containerFactory = "some-event", queues = "${spring.rabbitmq.custom.some-event.queue}")
@EnableRabbitRetryAndDlq(event = "some-event", onlyRetry = { IllegalArgumentException.class, RuntimeException.class }, 
retryAndDiscart  = { BeanDefinitionValidationException.class, NoSuchBeanDefinitionException.class })
public void onMessage(Message message) {
    ...
}

[andrelugomes]

How about?

@EnableRabbitRetryAndDlq(event = "some-event", 
retryWhen = { IllegalArgumentException.class, RuntimeException.class }, 
discartWhen  = { BeanDefinitionValidationException.class, NoSuchBeanDefinitionException.class },
directToDlqWhen  = { CriticalException.class })

Something like that.

[juliofalbo]

@andrelugomes , IMO it is really nice!

[wesleyegberto]

Also, it would be nice if we had an option to define if the exception verification should use the instanceof operator to allow it to capture children. So if we use checkInheritance = true, retryWhen = { IllegalArgumentException.class } then it should capture NumberFormatException.

[wesleyegberto]

I've started an implementation for this feature and also included some testing.

https://github.com/wesleyegberto/spring-rabbitmq-tuning/tree/issue-14

[juliofalbo]

@wesleyegberto really nice suggestion!!

Thank you very much for your help!

[wesleyegberto]

Just commenting some thoughts about this:

1) Checking priority that should be used In my commit I used the priority bellow, as you can check here.

• Should be discarted
• Should be sent to retry
• Should be sent to DLQ
• Otherwise discart

2) Backwards compatibility In the commit I kept the compatibility by doing this, allowing this code @EnableRabbitRetryAndDlq(event = "some-event") to work without modifications.

By doing so, and if the lib uses the checking priority defined in 1, it won't allow this code to work @EnableRabbitRetryAndDlq(event = "some-event", directToDlqWhen = NumberFormatException.class) (case where we would ignore only one exception and retry any other) because the code exceptions.contains(Exception.class) would be true before the DLQ list checking.

Here is a test case to demonstrate.

The priority bellow solves the problem:

• Should be discarted
• Should be sent to DLQ
• Should be sent to retry
• Otherwise discart

IMO the DLQ list should be verified before as I think we would define the exceptions of our retry list.

[juliofalbo]

@wesleyegberto I think you are already implemented this feature. Can you open the PR please?

[juliofalbo]

By doing so, and if the lib uses the checking priority defined in 1, it won't allow this code to work @EnableRabbitRetryAndDlq(event = "some-event", directToDlqWhen = NumberFormatException.class) (case where we would ignore only one exception and retry any other) because the code exceptions.contains(Exception.class) would be true before the DLQ list checking.

About this, we should verify only if we have the argument.

Example: @EnableRabbitRetryAndDlq(event = "some-event") -> Will retry the children of Exception.class

@EnableRabbitRetryAndDlq(event = "some-event", directToDlqWhen = NumberFormatException.class) -> Will verify if exceptions is null, if it is, we need to check only discardWhen

[juliofalbo]

@wesleyegberto, based on your code, I finalized the task!

Thank you very much!

I'm looking forward to merge your other fix!