fault handling logic

["Glynn, Eoghan" <eo...@iona.com>]

Dan,

I've run into a lot of frustration in getting the fault handing logic to work in the decoupled case. Here's a summary of the main issues I've encountered, and some ideas for moving forward. Let me know what you think.

1. On a server-side fault, AbstractFaultChainInitiatorObserver.onMessage() copies the *entire* message content of the incoming message into the fault message. This can result in the headers of the incoming message being re-used on the outbound message, which breaks correlation. I don't see why any of this content is relevant to the outgoing message, apart from the exception that caused the fault. 

Proposal: just copy the content of type Exception.class.

2. On the client-side dispatch of an incoming fault, in order to jump to the in-fault-chain, ReadHeaderInterceptor.handleMessage() attempts to retrieve the Endpoint from the exchange *before* correlation occurs, i.e. when the real Exchange for the message is *unknown*. As a result the attempt to abort the current interceptor chain traversal and jump over to the in-fault-chain fails (as getInFaultObserver() can't be called on a non-existent Endpoint). So the SoapxxFaultInInterceptor is never reached and the message.setContent(Exception.class, ...) call never occurs. Hence the message is incorrectly categorized as a partial response by the ClientImpl.

Proposal: if the Endpoint is unavailable in ReadHeaderInterceptor, have the ReadHeaderInterceptor mark the message with a "deferred.fault.observer.notification" property and delay and the jump to the in-fault-chain until after the correlation occurs (in the WS-A layer).  

3. In lots of places in the code we determine if a message is inbound or outbound by comparing the current message to getExchange().getOutMessage(). However, this fails if the message is an *outbound fault* message, as this is stored in a different field on Exchange, i.e. getFaultMessage(), with no directional indicator. Simply checking if the current message equals getExchange().getOutMessage() OR getExchange.getFaultMessage() isn't sufficient, as this would falsely categorize an incoming fault message as outbound. 

Proposal: get rid of the separate FaultMessage field on Exchange, and just use the InMessage and OutMessage fields for both normal or fault messages. See point #4 for how faults may be distinguished from normal messages.  

4. The semantics of Interceptor.handleFault() are gratuitously different from those of JAX-WS Handler.handleFault(). As far as I can tell, no existing interceptor does anything useful in handleFault(), whereas the WS-A and WS-RM layers both incorrectly expect the handleFault() semantics to be consistent with the JAX-WS case. Also I don't think the JAX-WS wrapper interceptors actually call Handler.handleFault() appropriately (i.e. Handler.handleMessage() is called instead).

Proposal: when a fault occurs on the in-chain, we currently unwind the current chain by calling handleFault() on each previously traversed interceptor and then jump to the out-fault-chain, calling handleMessage() on each interceptor with the fault message. I'd propose changing this to drop the unwind, instead call faultMessage() on each interceptor in the out-fault-chain. As a result, it would be clear to the interceptor that:
a) a fault has occurred
b) the direction of traversal (ì.e. inbound or bound) 

Cheers,
Eoghan

PS: if replying over the weekend, please CC my gmail address.

Re: fault handling logic

[Dan Diephouse <da...@envoisolutions.com>]

Hi Eoghan,

Glynn, Eoghan wrote:

>Dan,
>
>I've run into a lot of frustration in getting the fault handing logic to work in the decoupled case. Here's a summary of the main issues I've encountered, and some ideas for moving forward. Let me know what you think.
>
>1. On a server-side fault, AbstractFaultChainInitiatorObserver.onMessage() copies the *entire* message content of the incoming message into the fault message. This can result in the headers of the incoming message being re-used on the outbound message, which breaks correlation. I don't see why any of this content is relevant to the outgoing message, apart from the exception that caused the fault. 
>
>Proposal: just copy the content of type Exception.class.
>
>  
>
OK, sounds good.

>2. On the client-side dispatch of an incoming fault, in order to jump to the in-fault-chain, ReadHeaderInterceptor.handleMessage() attempts to retrieve the Endpoint from the exchange *before* correlation occurs, i.e. when the real Exchange for the message is *unknown*. As a result the attempt to abort the current interceptor chain traversal and jump over to the in-fault-chain fails (as getInFaultObserver() can't be called on a non-existent Endpoint). So the SoapxxFaultInInterceptor is never reached and the message.setContent(Exception.class, ...) call never occurs. Hence the message is incorrectly categorized as a partial response by the ClientImpl.
>
>Proposal: if the Endpoint is unavailable in ReadHeaderInterceptor, have the ReadHeaderInterceptor mark the message with a "deferred.fault.observer.notification" property and delay and the jump to the in-fault-chain until after the correlation occurs (in the WS-A layer).  
>
>  
>
Do we want to always defer this and build in correlation into CXF more? 
i.e. we could have some default correlation mechanism and a 
CorrelatorInterceptor. It would check to see if the message is a fault 
and kick it off there instead of kicking off the fault chain in 
ReadHeadersInterceptor.

>3. In lots of places in the code we determine if a message is inbound or outbound by comparing the current message to getExchange().getOutMessage(). However, this fails if the message is an *outbound fault* message, as this is stored in a different field on Exchange, i.e. getFaultMessage(), with no directional indicator. Simply checking if the current message equals getExchange().getOutMessage() OR getExchange.getFaultMessage() isn't sufficient, as this would falsely categorize an incoming fault message as outbound. 
>
>Proposal: get rid of the separate FaultMessage field on Exchange, and just use the InMessage and OutMessage fields for both normal or fault messages. See point #4 for how faults may be distinguished from normal messages.  
>  
>
Ok, sounds good.

>4. The semantics of Interceptor.handleFault() are gratuitously different from those of JAX-WS Handler.handleFault(). As far as I can tell, no existing interceptor does anything useful in handleFault(), whereas the WS-A and WS-RM layers both incorrectly expect the handleFault() semantics to be consistent with the JAX-WS case. Also I don't think the JAX-WS wrapper interceptors actually call Handler.handleFault() appropriately (i.e. Handler.handleMessage() is called instead).
>
>Proposal: when a fault occurs on the in-chain, we currently unwind the current chain by calling handleFault() on each previously traversed interceptor and then jump to the out-fault-chain, calling handleMessage() on each interceptor with the fault message. I'd propose changing this to drop the unwind, instead call faultMessage() on each interceptor in the out-fault-chain. As a result, it would be clear to the interceptor that:
>a) a fault has occurred
>b) the direction of traversal (ì.e. inbound or bound) 
>
>  
>
Yes, the semantics are very different, however if you don't think about 
the JAX-WS stuff it makes sense. handleFault gets called only if 
somewhere down the chain a fault occurred. This allows the interceptor 
to clean up any resources if it needs to. For instance, a database 
transaction might be rolled back. While this could happen in the fault 
chain, what happens if there is an error sending the fault message and 
the necessary interceptor isn't reached? This provides a full proof way 
to make sure something is cleaned up.

handleFault refers to a fault that occurs within a chain. Under the 
above thinking we would want to use handleMessage in the fault chain and 
then handleFault gets called if something goes wrong in the fault chain.

Does that make more sense?

So to address the concerns:
1. This is never used: most of the time it is never needed.
2. It is confusing - i.e. WS-A/RM expect the wrong semantics: We should 
clarify/document the semantics and fix the interceptors IMO

>PS: if replying over the weekend, please CC my gmail address.
>
>  
>
Sorry Eoghan, but I can't seem to find your gmail address anywhere in my 
inbox or on the CXF list :-(

Regards,

- Dan

-- 
Dan Diephouse
(616) 971-2053
Envoi Solutions LLC
http://netzooid.com