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<hzbar...@gmail.com> 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 <claus.ib...@gmail.com>: >>> >>> On Thu, Jul 9, 2009 at 9:03 AM, Guillaume Nodet<gno...@gmail.com> 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<hzbar...@gmail.com> 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<hzbar...@gmail.com> >>>>>> 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