Camel
  1. Camel
  2. CAMEL-4978

Dead Letter Channel: add onError with correspondence to onRedelivery

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.9.0
    • Fix Version/s: Future
    • Component/s: camel-core
    • Labels:
      None
    • Estimated Complexity:
      Moderate

      Description

      When an error occurs there can be situation when some actions should be performed before next redelivery attempt. These actions can be custom logging (in case of Dead Letter Channel we don't have control on logger name and format), creating incident request in internal company system, etc.

      Current version of Dead Letter Channel provides support for specifying processor which is called when next redelivery attempt is performed. However, if delay between attempts is long, this can be too late.

      I think we can implement support for specifying processor which will be called when error occurs, e.g., onError property. Then this processor with information from provided Exchange can take appropriate actions. For example, it can save this message for further investigation, while Dead Letter Channel will keep going on redelivering it.

      If this feature is worth to be implemented, I can prepare a patch. I implemented a work-around in my current project using onRedelivery, but I don't like it because of latency in redelivery interval.

        Activity

        Hide
        Andrey Kazantsev added a comment -

        Ok, I'll try to implement this. I like your attribute name, so let's go with it.

        Show
        Andrey Kazantsev added a comment - Ok, I'll try to implement this. I like your attribute name, so let's go with it.
        Hide
        Claus Ibsen added a comment -

        @Andrey

        Yeah patches is welcome. Since this is error handling, then we need unit tests as well, to ensure it works, and to test against regressions etc.

        About the name, maybe it should be named

        • onFailedDeliveryRef to be in line with the onRedeliveryRef name.
        Show
        Claus Ibsen added a comment - @Andrey Yeah patches is welcome. Since this is error handling, then we need unit tests as well, to ensure it works, and to test against regressions etc. About the name, maybe it should be named onFailedDeliveryRef to be in line with the onRedeliveryRef name.
        Hide
        Andrey Kazantsev added a comment -

        Yes, I understand this. However, what do you think about my proposal? Does it have sense? Maybe I've just overlooked something.

        About Camel 2.x, I think it's worth implementing this feature as additional attribute, and later in Camel 3.0 we can refactor it. We can pick up attribute name processErrorRef, which will reference Processor. This attribute can be set on both errorHandler and onException.

        Show
        Andrey Kazantsev added a comment - Yes, I understand this. However, what do you think about my proposal? Does it have sense? Maybe I've just overlooked something. About Camel 2.x, I think it's worth implementing this feature as additional attribute, and later in Camel 3.0 we can refactor it. We can pick up attribute name processErrorRef , which will reference Processor . This attribute can be set on both errorHandler and onException .
        Hide
        Claus Ibsen added a comment -

        We cannot change the DSL / API in Camel 2.x as too many users depend upon it being stable.

        For Camel 3.0 we can consider tithing the API a bit to make it more clear in some areas.

        Show
        Claus Ibsen added a comment - We cannot change the DSL / API in Camel 2.x as too many users depend upon it being stable. For Camel 3.0 we can consider tithing the API a bit to make it more clear in some areas.
        Hide
        Andrey Kazantsev added a comment -

        Claus

        Thanks for your answer and time!

        I agree that introducing new property for onException is not a good idea. I think current design of onException with redelivery is ambiguous. Take a look at it definition:

        <onException redeliveryPolicyRef="redeliveryPolicy">
        <exception>java.io.IOException</exception>
        <to ref="someEndpoint" />
        </onException>

        It seems that when exception occurs, message is sent to someEndpoint. However, if redeliveryPolicyRef is defined message will be re-delivered, and actions, defined in onException are executed only when redelivery attempts are exhausted.

        Also, if we want to prepare message before redelivery, we define onRedelivery:

        <onException onRedeliveryRef="someProcessor" redeliveryPolicyRef="redeliveryPolicy">
        <exception>java.io.IOException</exception>
        <to ref="someEndpoint" />
        </onException>

        But why we can't define onRedelivery in redeliveryPolicyRef? This parameter has sense only in case if redeliveryPolicyRef is set.

        I think my proposal will look very destructive to current error handling design, but it seems to be much clear. Here ir is.

        onException is used for overriding error-handling for some exceptions, with possible redeliveries. So we should move all settings related to redelivery out of onException to RedeliveryPolicy. In this case all actions, described in onException will be executed every time exception occurs. In the same time, RedeliveryPolicy will have two properties: onRedeliveryRef and onRedeliveryFailRef. First one is called before every redelivery (the same meaning as now), and second one is called when redelivery attempts are exhausted - those actions, currently described in onException.

        What do you think?

        Show
        Andrey Kazantsev added a comment - Claus Thanks for your answer and time! I agree that introducing new property for onException is not a good idea. I think current design of onException with redelivery is ambiguous. Take a look at it definition: <onException redeliveryPolicyRef="redeliveryPolicy"> <exception>java.io.IOException</exception> <to ref="someEndpoint" /> </onException> It seems that when exception occurs, message is sent to someEndpoint . However, if redeliveryPolicyRef is defined message will be re-delivered, and actions, defined in onException are executed only when redelivery attempts are exhausted. Also, if we want to prepare message before redelivery, we define onRedelivery : <onException onRedeliveryRef="someProcessor" redeliveryPolicyRef="redeliveryPolicy"> <exception>java.io.IOException</exception> <to ref="someEndpoint" /> </onException> But why we can't define onRedelivery in redeliveryPolicyRef ? This parameter has sense only in case if redeliveryPolicyRef is set. I think my proposal will look very destructive to current error handling design, but it seems to be much clear. Here ir is. onException is used for overriding error-handling for some exceptions, with possible redeliveries. So we should move all settings related to redelivery out of onException to RedeliveryPolicy . In this case all actions, described in onException will be executed every time exception occurs . In the same time, RedeliveryPolicy will have two properties: onRedeliveryRef and onRedeliveryFailRef . First one is called before every redelivery (the same meaning as now), and second one is called when redelivery attempts are exhausted - those actions, currently described in onException . What do you think?
        Hide
        Claus Ibsen added a comment -

        Andrey

        Sorry for not getting back sooner.
        You use-case seems compelling. The draw-back is that we would need to add more options to the error handling in Camel. And to find a good term for the new feature, to avoid confusion.

        If there is a onError then people may get confused/mixed up with onException and what the difference would be.
        And what is the difference between error/exception? Isn't it the same?

        And should you be able to configure onError on both the errroHandler and the onException like you can do with other options such as redelivery options, and onRedelivery.

        Is there not a better term than onError? I think it should start with onXXX just like onRedelivery. But something that will not confuse people.

        Show
        Claus Ibsen added a comment - Andrey Sorry for not getting back sooner. You use-case seems compelling. The draw-back is that we would need to add more options to the error handling in Camel. And to find a good term for the new feature, to avoid confusion. If there is a onError then people may get confused/mixed up with onException and what the difference would be. And what is the difference between error/exception? Isn't it the same? And should you be able to configure onError on both the errroHandler and the onException like you can do with other options such as redelivery options, and onRedelivery. Is there not a better term than onError? I think it should start with onXXX just like onRedelivery. But something that will not confuse people.
        Hide
        Andrey Kazantsev added a comment -

        Please consider my example. I don't have access to code right now, so I'll describe in common words.

        I've default error handler for my main route, which by default routes a message to some route where logic is executed. Default error handler is configured for Unrecoverable error, so we don't try to redelivery the message.

        <errorHandler id='defaultErrorHandler' ... deadLetterUri='direct:unrecoverableErrors' />
        <route id='mainRoute' errorHandler='defaultErrorHandler' />

        However, there are some errors which are Recoverable, and we need to redeliver that messages. So I configure onException:

        <onException>
        <exception>TransientException</exception>
        <redeliveryPolicy redeliveryAttemps='3' />
        <to uri='direct:recoverableErrors' />
        </onException>

        With this configuration, all messages with unrecoverable errors are going to defaultErrorHandler, and messages with recoverable errors are trying to redeliver, and when redelivery attempts are exhausted, they sent to direct:recoverableErrors. However, I need to execute some specific logic when recoverable error occur, and before redelivery attempt. To accomplish this in any way, I use onRedelivery processor, but I've a latency while next redelivery will be performed.

        Am I doing something completely wrong and overlooked hove this can be done with current Camel means?

        Show
        Andrey Kazantsev added a comment - Please consider my example. I don't have access to code right now, so I'll describe in common words. I've default error handler for my main route, which by default routes a message to some route where logic is executed. Default error handler is configured for Unrecoverable error, so we don't try to redelivery the message. <errorHandler id='defaultErrorHandler' ... deadLetterUri='direct:unrecoverableErrors' /> <route id='mainRoute' errorHandler='defaultErrorHandler' /> However, there are some errors which are Recoverable , and we need to redeliver that messages. So I configure onException: <onException> <exception>TransientException</exception> <redeliveryPolicy redeliveryAttemps='3' /> <to uri='direct:recoverableErrors' /> </onException> With this configuration, all messages with unrecoverable errors are going to defaultErrorHandler , and messages with recoverable errors are trying to redeliver, and when redelivery attempts are exhausted, they sent to direct:recoverableErrors . However, I need to execute some specific logic when recoverable error occur, and before redelivery attempt. To accomplish this in any way, I use onRedelivery processor, but I've a latency while next redelivery will be performed. Am I doing something completely wrong and overlooked hove this can be done with current Camel means?
        Hide
        Andrey Kazantsev added a comment -

        Ok, maybe I don't have clear understanding of what Camel can. Could you please explain in greater detail how can I accomplish following scenario:

        1) Exception occurs in route
        2) Some actions should be performed right after this, to inform external systems (through either forwarding copy of message to another route either to some Processor)
        3) Try to redeliver this message several times
        4) After redeliveries are exhausted, take another set of actions

        My problem is that with DLC any logic which is written in onException is executed only when redeliveries are exhausted and message is moved to DLQ, while I need a way to execute some logic just after error, and after that try to redeliver.

        I will appreciate very much if you can point me a way of accomplishing this with current means Camel already provides!

        Show
        Andrey Kazantsev added a comment - Ok, maybe I don't have clear understanding of what Camel can. Could you please explain in greater detail how can I accomplish following scenario: 1) Exception occurs in route 2) Some actions should be performed right after this, to inform external systems (through either forwarding copy of message to another route either to some Processor) 3) Try to redeliver this message several times 4) After redeliveries are exhausted, take another set of actions My problem is that with DLC any logic which is written in onException is executed only when redeliveries are exhausted and message is moved to DLQ, while I need a way to execute some logic just after error, and after that try to redeliver. I will appreciate very much if you can point me a way of accomplishing this with current means Camel already provides!
        Hide
        Claus Ibsen added a comment -

        I am -1 on this, as the error handling in Camel already have a lot of bells and whistles. And we should be careful to add more to it. As it just confuses people, and there is a lot more to test and ensure works as expected, etc. And people may configure and use it in ways we did not anticipate, causing new "issues".

        In your use-case, you can use onException to trigger logic to occur when an exception is thrown.
        And if you are exhausted and the DLC moves the message to its DLQ, then you can use a direct endpoint to route to a route, where you can execute logic to occur when the message is moved to the DLQ.

        Show
        Claus Ibsen added a comment - I am -1 on this, as the error handling in Camel already have a lot of bells and whistles. And we should be careful to add more to it. As it just confuses people, and there is a lot more to test and ensure works as expected, etc. And people may configure and use it in ways we did not anticipate, causing new "issues". In your use-case, you can use onException to trigger logic to occur when an exception is thrown. And if you are exhausted and the DLC moves the message to its DLQ, then you can use a direct endpoint to route to a route, where you can execute logic to occur when the message is moved to the DLQ.

          People

          • Assignee:
            Unassigned
            Reporter:
            Andrey Kazantsev
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development