Hi
I think it would be confusing if there is a setFault but no getFault?
But I understand the reason when you want to use OUT for both regular
OUT and FAULT. Where a header determine the type.
As its uncommon to set a fault we could as James suggested have a
factory method on Exchange to create a new one:
Message msg = exchange.createFault();
msg.setBody("Unknown order id " + id);
exchange.setOut(msg);
This could reduce any get/set fault etc. on the Exchange to only the
factory method.
Then what is left is probably a either to
- keep the hasFault on exchange
- or move it to Message and name it isFault()
Then we only get IN and OUT on the Exchange, and the API is reduced.
On Thu, Jul 9, 2009 at 4:35 PM, Hadrian Zbarcea<[email protected]> wrote:
> My intention with this thread is to try to cleanup the api for 2.0.
> Implementation we can fix in any 2.x.
> Although I agree with having a Fault being stored as an out with a flag,
> it's not very relevant right now for me.
>
> As api changes, if I understand correctly, you are suggesting;
>
> Message getOut();
> boolean hasOut();
> void setOut(Message out);
>
> boolean hasFault();
> void setFault(boolean value);
>
> These methods go:
> Message getOut(boolean lazyCreate);
> Message getFault();
> Message getFault(boolean lazyCreate);
> void removeFault();
>
> The equivalency being:
>
> Message fault = getFault();
> equivalent to:
> if (hasFault())
> Message fault = getOut();
>
> or to set the fault instead of the lazyCreate thing:
> setOut(message);
> setFault(true);
>
> removeFault() becomes:
> setOut(null);
>
> I am perfectly fine with this. It's the responsibility of the caller to
> check if the out is a fault or not, which may or may not be relevant
> depending on the component. We just need to clearly document that.
>
> +1
> Hadrian
>
>
>
> On Jul 9, 2009, at 6:06 AM, James Strachan wrote:
>
>> A few random thoughts.
>>
>> So we definitely need a way to determine if the output on an exchange
>> is an OUT, fault, exception. We could have an OUT and a Fault Message;
>> or have a single OUT Message with a boolean fault property.
>>
>> We could store an Exception as a body of the OUT maybe, though I can't
>> help feel that an Exception is a bit different to an OUT/Fault (which
>> are messages). e.g. you might want to clear the exception but keep the
>> OUT?
>>
>> To process the output in a pipeline we could do something like...
>>
>> Throwable e = exchange.getException();
>> if (e != null) {
>> // process the exception
>> }
>> else {
>> // we should now have now valid output
>> Message out = exchange.getOut();
>> if (out == null) {
>> // no output created, so reuse input?
>> out = exchange.getIn();
>> }
>>
>> // we might not care if its a fault or out, we might just wanna
>> return it anyway
>> if (out.isFault()) {
>> // do some fault specific coding here...
>> }
>> }
>>
>> So we could use the OUT for a fault or a regular OUT (as you could say
>> its an output message, whether a fault or real response) so code might
>> not care if its a fault or not? So maybe a flag on OUT is neater?
>>
>> Exceptions seem different though; its typically something you'd wanna
>> look at (and might wanna know what the OUT was when the exception
>> threw), so having that as a property on Exchange feels right to me.
>>
>>
>> Main problem seems to be the lazy create stuff. (Bad James!). Maybe we
>> just need OUT to be null and if someone wants to create an OUT there's
>> a factory method...
>>
>> Message out = exchange.createOut();
>>
>> (it could maybe be smart enough that if there's already an OUT defined
>> it returns that one?). Once someone calls createOut() then the OUT is
>> set and getOut() returns a non null value?
>>
>> Then if its a fault...
>>
>> out.setFault(true);
>>
>>
>> Then if folks call exchange.getOut() it will typically return null and
>> not do any lazy creation or gobble up messages etc as Claus says?
>>
>>
>> 2009/7/9 Claus Ibsen <[email protected]>:
>>>
>>> On Thu, Jul 9, 2009 at 9:03 AM, Guillaume Nodet<[email protected]> wrote:
>>>>
>>>> In web services world, faults are not exceptions, but usually an xml
>>>> payload. In the java world, faults would be like checked exceptions
>>>> and errors runtime exceptions. However, distinguishing a fault from
>>>> an out message by (instanceof Exception) is imho not sufficient
>>>
>>> Yeah I was about to say the same. I think the FAULT message makes
>>> sense. Fits better with the web service if you have declared faults in
>>> the wsdl.
>>> Then you can construct a XML message and set it as getFault().setBody().
>>>
>>> And I would also think it is much more confusing to set a fault
>>> message, wrapped as an exception on the OUT message as opposed to use
>>> setException on the exchange instead. That would be odd.
>>>
>>> So the API is good, my only griefs are
>>> a) the OUT creating empty messages.
>>> b) and that people might think as if they can during routing processes
>>> piece by piece assemble an OUT message.
>>>
>>> a)
>>> See previous mails about this.
>>>
>>> b)
>>> An example to illustrate this. For instance in the route below A, B, C
>>> is independent steps that enrich a message with order details.
>>> Suppose the input message is an order id. And the expected message is
>>> details about this order.
>>>
>>> from("direct:start").process(A).process(B).process(C);
>>>
>>> from
>>> IN = 123
>>> OUT = null
>>>
>>> A
>>> IN = 123
>>> OUT = Orderid: 123.
>>>
>>> B
>>> IN = 123
>>> OUT = Orderid: 123. Status = IN PROGRESS
>>>
>>> C
>>> IN = 123
>>> OUT = Orderid: 123. Status = IN PROGRESS. ETA: 2009/08/13
>>>
>>> Client receives
>>> OUT: Orderid: 123. Status = IN PROGRESS. ETA: 2009/08/13
>>>
>>>
>>> But the nature of pipes and filter, this is what happens
>>>
>>> from
>>> IN = 123
>>> OUT = null
>>>
>>> A
>>> IN = 123
>>> OUT = Orderid: 123.
>>>
>>> B
>>> IN = Orderid: 123.
>>> OUT = null
>>>
>>> Kabom!!! now we got a partly OUT message as IN instead of the original
>>> IN message.
>>>
>>> This is right according to the pipes and filters, where previous OUT
>>> should be next IN.
>>>
>>>
>>> But people should just be aware with this in relation to IN and OUT -
>>> that the OUT is not something that you can piece by piece assemble.
>>> And OUT is really not that useable.
>>>
>>>
>>>
>>>
>>>>
>>>> On Thu, Jul 9, 2009 at 07:35, Hadrian Zbarcea<[email protected]> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Comments inline.
>>>>>
>>>>> Hadrian
>>>>>
>>>>> On Jul 9, 2009, at 12:54 AM, Claus Ibsen wrote:
>>>>>
>>>>>> On Wed, Jul 8, 2009 at 9:52 PM, Hadrian Zbarcea<[email protected]>
>>>>>> wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> As we approach the 2.0, there is one more hanging issue I would like
>>>>>>> addressed, if possible. It's the thorny issue of Faults and
>>>>>>> Exceptions
>>>>>>> that
>>>>>>> started in
>>>>>>> http://issues.apache.org/activemq/browse/CAMEL-316 (see also the
>>>>>>> related
>>>>>>> nabble thread linked in the issue description).
>>>>>>>
>>>>>>> I am less concerned about how the DefaultExchange is implemented and
>>>>>>> I
>>>>>>> hope
>>>>>>> to reach an agreement on what the Exchange api should be (please find
>>>>>>> the
>>>>>>> list of Exchange methods below).
>>>>>>>
>>>>>>> As far as faults/exceptions are concerned, Roman thinks that the
>>>>>>> whole
>>>>>>> concept of in/out/fault/exception is artificial, and only one payload
>>>>>>> (message) api should be enough (Roman please correct me if I
>>>>>>> misinterpret
>>>>>>> your position). My opinion is that we *must* distinguish between
>>>>>>> persistent
>>>>>>> (fault) and transient (exception) errors for the simple reason that
>>>>>>> they
>>>>>>> have different semantics. As Roman correctly points out, faults are
>>>>>>> more
>>>>>>> like outputs, have more of application level semantics and are
>>>>>>> normally
>>>>>>> handled by the client, where exceptions (transient errors) are
>>>>>>> something
>>>>>>> camel could try to recover from, without much knowledge about the
>>>>>>> application. I think that the presence of fault in the camel api is
>>>>>>> not
>>>>>>> due
>>>>>>> to it being explicitly modeled by jbi and wsdl, as Roman suggests,
>>>>>>> and
>>>>>>> Camel
>>>>>>> simply copying that, but it's modeled in Camel for the same reason
>>>>>>> it's
>>>>>>> modeled in jbi and wsdl, to differentiate transient from persistent
>>>>>>> errors
>>>>>>> in a non ambiguous way.
>>>>>>
>>>>>> I am one of the persons that would love the Camel Exchange / Message
>>>>>> API to be a bit simpler. It has a fair shares of methods.
>>>>>>
>>>>>> Having listening and discussing with Hadrian on this and doing my own
>>>>>> investigations and whatnot I do belive that Hadrian is absolutely
>>>>>> right when it comes to FAULT. It has a good place in the API. I am +1
>>>>>> on having FAULT as we do now.
>>>>>>
>>>>>> The grief I have left is that the IN and OUT. It makes sense to have
>>>>>> them and they provide a good value. However they have a big drawnback
>>>>>> in how they are routed in Camel with the Pipeline processor, that
>>>>>> mimics the pipes and filters EIP. And as a result the OUT will be used
>>>>>> as IN
>>>>>> in the next step in the route. So its not like you can steadily build
>>>>>> up an OUT message on-the-fly during many steps in the route path.
>>>>>>
>>>>>> Example
>>>>>> from("direct:start").process(new Processor()).to("log:foo");
>>>>>>
>>>>>> a) From
>>>>>> IN = Hello World
>>>>>> OUT = null
>>>>>>
>>>>>> b) Processor
>>>>>> IN Hello World
>>>>>> OUT = Bye World
>>>>>>
>>>>>> c) Log
>>>>>> IN = Bye World
>>>>>> OUT = null
>>>>>>
>>>>> Yes, from an external observer's perspective, this is precisely what
>>>>> happens. How we decide to store it, how many fields we need, is an
>>>>> implementation detail of the DefaultExchange. I don't think the copy
>>>>> from
>>>>> out/in is too expensive, but I would be ok with having only one field
>>>>> to
>>>>> store the current message in DefaultExchange (I assume that's what you
>>>>> propose). However, my question is about what the api should be.
>>>>>
>>>>>
>>>>>>
>>>>>> And then the getOut() method that lazy creates a new empty OUT message
>>>>>> is also a pita, as it can lead to people loosing their messages if
>>>>>> they do some System out logging of their own
>>>>>>
>>>>>> public void process(Exchange e) {
>>>>>> System.out.println(exchange.getIn());
>>>>>> System.out.println(exchange.getOut());
>>>>>> // boom you lost your message when its routed to next node in route
>>>>>> path, as getOut() created a new empty OUT message that will by used in
>>>>>> the pipes and filters EIP routed with the Pipeline
>>>>>> }
>>>>>>
>>>>>> We had this IN OUT discussion a while back and at that time we ended
>>>>>> up with a compromise of having a hasOut() method so you should do, to
>>>>>> be safe:
>>>>>> System.out.println(exchange.getIn());
>>>>>> if (exchange.hasOut()) {
>>>>>> System.out.println(exchange.getOut());
>>>>>> }
>>>>>>
>>>>>> Still a pita with the lazy creation IMHO.
>>>>>
>>>>> The lazyCreate methods are actually deprecated, and imho should be
>>>>> removed
>>>>> now. This would eliminate the confusion.
>>>>>
>>>>>
>>>>>>>
>>>>>>> If we were to go with only get/setMessage() api, we would still need
>>>>>>> methods
>>>>>>> (or some ways) to distinguish between the kind of message we are
>>>>>>> dealing
>>>>>>> with (in/out/fault/exception) so we'd only move the problem somewhere
>>>>>>> else.
>>>>>>>
>>>>>>> So the question becomes if we leave the api the way it is, or we
>>>>>>> replace
>>>>>>> the
>>>>>>> get/setFault apis with get/setOut, in which case we'll need something
>>>>>>> like:
>>>>>>> boolean isFault();
>>>>>>> method in the Message api or keep the hasFault() method on the
>>>>>>> Exchange.
>>>>>>
>>>>>> Good question
>>>>>>
>>>>>> If you use OUT instead then we need to add a isFault() on the
>>>>>> org.apache.camel.Message API that
>>>>>> the IN message also implements.
>>>>>>
>>>>>> It could make sense to use OUT as well for FAULT.
>>>>>> But how should the API look like to set an OUT as Fault?
>>>>>>
>>>>>> Something a like this?
>>>>>>
>>>>>> getOut().setBody("Unknown bank account number.");
>>>>>> getOut().setFault(true);
>>>>>
>>>>> Not quite, I had something like this in mind (there is no setFault()
>>>>> method):
>>>>> getOut().setBody(java-lang-Exception-derivedObject);
>>>>>
>>>>> boolean isFault() {
>>>>> return getBody() instanceof Exception;
>>>>> }
>>>>>
>>>>> Personally I am ok with the limitation not being able to have an out of
>>>>> a
>>>>> java.lang.Exception type (that would then be a Fault). I can't imagine
>>>>> a
>>>>> case where an Exception would be an expected out, and in such cases one
>>>>> could always serialize or wrap it. The fact that the Fault would be an
>>>>> Exception type would be a camel convention that needs to be followed by
>>>>> all
>>>>> components.
>>>>>
>>>>> Another option would be add header HAS_FAULT or something like that,
>>>>> in
>>>>> which case both the out and the fault could be of any type.
>>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Thoughts?
>>>>>>>
>>>>>>>
>>>>>>> ExchangePattern getPattern();
>>>>>>> void setPattern(ExchangePattern pattern);
>>>>>>>
>>>>>>> Object getProperty(String name);
>>>>>>> <T> T getProperty(String name, Class<T> type);
>>>>>>> void setProperty(String name, Object value);
>>>>>>> Object removeProperty(String name);
>>>>>>> Map<String, Object> getProperties();
>>>>>>>
>>>>>>> Message getIn();
>>>>>>> void setIn(Message in);
>>>>>>>
>>>>>>> Message getOut();
>>>>>>> boolean hasOut();
>>>>>>> Message getOut(boolean lazyCreate);
>>>>>>> void setOut(Message out);
>>>>>>>
>>>>>>> Message getFault();
>>>>>>> boolean hasFault();
>>>>>>> Message getFault(boolean lazyCreate);
>>>>>>> void removeFault();
>>>>>>> // removeFault() is only used in one place
>>>>>>
>>>>>> +1 to remove it. You can just do setFault(null) instead. I will fix it
>>>>>> asap.
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Exception getException();
>>>>>>> <T> T getException(Class<T> type);
>>>>>>> void setException(Exception e);
>>>>>>> boolean isFailed();
>>>>>>>
>>>>>>> boolean isTransacted();
>>>>>>> boolean isRollbackOnly();
>>>>>>>
>>>>>>> CamelContext getContext();
>>>>>>>
>>>>>>> Exchange newInstance();
>>>>>>> Exchange copy();
>>>>>>> Exchange newCopy(boolean handoverOnCompletion);
>>>>>>> void copyFrom(Exchange source);
>>>>>>>
>>>>>>> Endpoint getFromEndpoint();
>>>>>>> void setFromEndpoint(Endpoint fromEndpoint);
>>>>>>>
>>>>>>> UnitOfWork getUnitOfWork();
>>>>>>> void setUnitOfWork(UnitOfWork unitOfWork);
>>>>>>>
>>>>>>> String getExchangeId();
>>>>>>> void setExchangeId(String id);
>>>>>>>
>>>>>>> void addOnCompletion(Synchronization onCompletion);
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Claus Ibsen
>>>>>> Apache Camel Committer
>>>>>>
>>>>>> Open Source Integration: http://fusesource.com
>>>>>> Blog: http://davsclaus.blogspot.com/
>>>>>> Twitter: http://twitter.com/davsclaus
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Cheers,
>>>> Guillaume Nodet
>>>> ------------------------
>>>> Blog: http://gnodet.blogspot.com/
>>>> ------------------------
>>>> Open Source SOA
>>>> http://fusesource.com
>>>>
>>>
>>>
>>>
>>> --
>>> Claus Ibsen
>>> Apache Camel Committer
>>>
>>> Open Source Integration: http://fusesource.com
>>> Blog: http://davsclaus.blogspot.com/
>>> Twitter: http://twitter.com/davsclaus
>>>
>>
>>
>>
>> --
>> James
>> -------
>> http://macstrac.blogspot.com/
>>
>> Open Source Integration
>> http://fusesource.com/
>
>
--
Claus Ibsen
Apache Camel Committer
Open Source Integration: http://fusesource.com
Blog: http://davsclaus.blogspot.com/
Twitter: http://twitter.com/davsclaus
