Mailing-List: contact dev-help@camel.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@camel.apache.org
Message-ID: <22082636.38411278761873552.JavaMail.jira@thor>
Date: Sat, 10 Jul 2010 07:37:53 -0400 (EDT)
From: "Martin Krasser (JIRA)" <jira@apache.org>
To: camel-dev@activemq.apache.org
Subject: [jira] Issue Comment Edited: (CAMEL-2935) Broken failure reporting
 via DefaultProucerTemplate.asyncCallback and Synchronization.onFailure
In-Reply-To: <11958053.38251278756171780.JavaMail.jira@thor>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/activemq/browse/CAMEL-2935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=60575#action_60575 ] 

Martin Krasser edited comment on CAMEL-2935 at 7/10/10 7:36 AM:
----------------------------------------------------------------

Claus, I'd like to first suggest a solution as implemented by {{on-failure.patch}}. This is a solution that fixes the problem with only minimal changes (i.e. without introducing new {{sendAsync}} methods). In this patch I simply moved

{code:java}
            if (exchange.getException() != null) {
                exchange.setException(wrapCamelExecutionException(exchange, exchange.getException()));
            }
{code} 

from {{ProducerCache.send(Endpoint, Exchange)}} to {{ProducerTemplate.send(Endpoint, Exchange)}} because it is the *only* user of  {{ProducerCache.send(Endpoint, Exchange)}}. In {{ProducerCache.asyncCallback(Endpoint, Exchange, Synchronization)}} the {{ProducerCache.send(Endpoint, Exchange)}} is now used directly (and this again re-introduces the same behaviour as in Camel 2.3). All the camel-core tests run through locally (except SplitterStreamCache test but it also fails without the changes ... so it must be something else).

Although, I see this issue as resolved with this patch, I still don't understand why the if-statement above was introduced only for {{ProducerCache.send(Endpoint, Exchange)}} and not for the other {{ProducerCache.send}} methods as well. This looks somehow inconsistent to me. Maybe I just miss something obvious. 

If you are fine with the provided patch, please let me know and I'll commit the changes.

      was (Author: mrt1nz):
    Claus, I'd like to first suggest a solution as implemented by {{on-failure.patch}}. This is a solution that fixes the problem with only minimal changes (i.e. without introducing new {{sendAsync}} methods). In this patch I simply moved

{code:java}
            if (exchange.getException() != null) {
                exchange.setException(wrapCamelExecutionException(exchange, exchange.getException()));
            }
{code} 

from {{ProducerCache.send(Endpoint, Exchange)}} to {{ProducerTemplate.send(Endpoint, Exchange)}} because it is the *only* user of  {{ProducerCache.send(Endpoint, Exchange)}}. In {{ProducerCache.asyncCallback(Endpoint, Exchange, Synchronization)}} the {{ProducerCache.send(Endpoint, Exchange)}} is now used directly (and this again re-introduces the same behaviour as in Camel 2.3). All the camel-core tests run through locally (except SplitterStreamCache test but it also fails without the changes ... so it must be something else).

Although, I see this issue as resolved with this patch, I still don't understand why the if-statement above was introduced only for {{ProducerCache.send(Endpoint, Exchange)}} and not for the other {{ProducerCache.send}} methods as well. This looks somehow inconsistent to me. Maybe I just miss something obvious. 

Btw, I also added one more tests method in the patch, so the first patch file is obsolete. If you are fine with the provided patch, please let me know and I'll commit the changes.
  
> Broken failure reporting via DefaultProucerTemplate.asyncCallback and Synchronization.onFailure
> -----------------------------------------------------------------------------------------------
>
>                 Key: CAMEL-2935
>                 URL: https://issues.apache.org/activemq/browse/CAMEL-2935
>             Project: Apache Camel
>          Issue Type: Bug
>          Components: camel-core
>    Affects Versions: 2.3.0
>            Reporter: Martin Krasser
>             Fix For: 2.4.0
>
>         Attachments: on-failure.patch
>
>
> The recent change in {{ProducerCache.send}}
> {code:java|title=ProducerCache.java}
>     public void send(Endpoint endpoint, Exchange exchange) {
>         try {
>             sendExchange(endpoint, null, null, exchange);
>             // RECENT CHANGE HERE:
>             // ensure that CamelExecutionException is always thrown
>             if (exchange.getException() != null) {
>                 exchange.setException(wrapCamelExecutionException(exchange, exchange.getException()));
>             }
>         } catch (Exception e) {
>             throw wrapCamelExecutionException(exchange, e);
>         }
>     }
> {code}
> that throws a {{CamelExecutionException}} if {{exchange.getException}} is not null, makes it impossible for {{DefaultProducerTemplate.asyncCallback}} to report failures (other than fault messages) asynchronously via {{Synchronization.onFailure}}
> {code:java|title=DefaultProducerTemplate.java}
>     public Future<Exchange> asyncCallback(final Endpoint endpoint, final Exchange exchange, final Synchronization onCompletion) {
>         Callable<Exchange> task = new Callable<Exchange>() {
>             public Exchange call() throws Exception {
>                 // FIXME: exception is thrown in Camel 2.4 where a normal return with answer.getException != null was done in Camel 2.3
>                 Exchange answer = send(endpoint, exchange);
>                 if (answer.isFailed()) {
>                     onCompletion.onFailure(answer);
>                 } else {
>                     // ...
>                 }
>                 return answer;
>             }
>         };
>         // ...
>     }
> {code}
> This was working in Camel 2.3 (but unfortunately there wasn't any test case for it). I attached a patch for {{DefaultProducerTemplateAsyncTest}} that demonstrates the problem. I didn't commit a fix yet because I'm unsure at the moment about the best way to fix that. Of course I tried a naive fix in the DefaultProducerTemplate.asyncCallback methods which causes the test (in the patch) to pass but I'd like to hear other opinions before I continue.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.