Hi Cyrille
Thanks for posting this proposal/analysis, please see some comments
prefixed with S.B. below...
cheers, Sergey
Hello all,
Here is a proposal of refactoring of both the JAXRS client-side and
server-side, these refactoring could be separated one from the other.
Please, let me know if it worth continuing this work.
SERVER SIDE
============
Move the ExceptionMapper handling from the JAXRSInvoker to a new
JAXRSFaultOutInterceptor.
Description : If an exception is associated with a Response via an
ExceptionMapper, the fault interceptors chain is aborted and a new
chain is triggered to render the Response.
Pros : consistency between the JAXRS and JAXWS interceptor chains, for
example, the ResponseTimeFeature can now count exceptions mapped to
responses.
Cons : a third interceptors chain is introduced for exceptions that
are mapped to Response. It is a bit weird :-)
S.B :
It looks like the right approach going forward from a technical perspective. Note that at the moment JAXRSInvoker, in
JAXRSInInterceptor and out JAXRSOutInterceptor are all trying to map exceptions to Responses given that the exceptions may be thrown
from the application code (JAXRSInvoker mapping), from JAXRS message body readers or custom in filters (JAXRSInInterceptor mapping)
or from JAXRS message body writers (JAXRSOutInterceptor mapping).
Perhaps, the ExceptionMapper handling can indeed be moved from the JAXRSInvoker and JAXRSInInterceptor to the fault interceptor, but
this fault interceptor should basically reuse the JAXRSOutInterceptor if/after it has managed to map a fault to the Response given
that a Response created by a given ExceptionMapper still has to go through the chain of custom out filters and JAXRS writers. But
there are few more things to consider :
- JAXRSInInterceptor/JAXRInvoker in its final block clears thread local proxies (representing UriInfo/etc) which may've been
injected into custom providers, including exception mappers, so these proxies will need to be available for these mappers and for
JAXRS writers/outFilters (in case they need to handle the exception mapper Responses) if they will be invoked from the fault
interceptors. So the fault interceptor will need to take care of clearing all the proxies injected into various providers in the
end.
- At the moment PhaseInterceptorChain will log all the faults which are coming through it. This is something which users may not
always want. For example, a JAXRS application code might've logged the fact that a certain resource is not available and throw
BookNotFoundException and expect a custom mapper to quietly turn it into 404. At the moment it will work as expected but if we move
the mapping code from JAXRSInvoker and JAXRSInInterceptor to the fault one then more runtime logging will get done. I think one of
CXF users was thinking of customizing PhaseInterceptorChain so that 'quiet' loggers can be plugged in but nothing has been done yet
there AFAIK.
So it all should work quite well, but we need to do a bit more analysis of what it would take to complete this refactoring on the
server side...
CLIENT SIDE
===========
Extract the marshalling and exception processing logic from the jaxrs
client to interceptors ; I only worked on the ClientProxyImpl, the
work on the WebClient is still to do.
Description :
* the JAXRSResponseInterceptor builds the Response object (currently
with the inputstream object). Remaining : complete the Response
processing with the unmarshalling of the inputstream
S.B. I guess this one should probably be handling both proxy and WebClient
responses ?
* the JAXRSCheckFaultInterceptor handles faults and the
ResponseExceptionMapper processing
S.B : one thing to be aware of here is that if either a code using proxy or WebClient explicitly expects a JAXRS Response object
then it should get Response...
Pros : consistency betwen JAXRS and JAXWS interceptor chains, for
example, the ResponseTimeFeature can now count exceptions mapped to
responses.
Cons : I didn't find any :-)
S.B : sounds good :-)
Todo : complete the cleanup of the client
Note : the ClientFaultConverter should NOT be declared as an "In Fault
Interceptor" for JAXRS endpoints (specially important for the client)
as the ClientFaultConverter tries to unmarshall a SOAP XML exception.
Cyrille
--
Cyrille Le Clerc
[email protected]
http://blog.xebia.fr
On Mon, Dec 21, 2009 at 6:54 PM, Sergey Beryozkin <[email protected]> wrote:
Hi Cyrille
Please see comments inline
Dear all,
I am looking at the consistency of exception handling among JAX-WS
and JAX-RS. My primary goal is to ensure cxf management metrics (JMX)
are consistent.
Here are few questions :
SERVER SIDE JAXRS EXCEPTION MAPPER
====================================
If an ExceptionMapper handles the exception :
1) The JAXRSInvoker returns a Response instead of throwing an Exception
Yes, this is for JAXRS message body writers be able to handle whatever Response
entity a given mapper might've set up.
2) Thus PhaseInterceptorChain does NOT see that an exception occurred
during the invocation
Yes
3) Thus the "Out Interceptors" are not replaced by the "Out Fault
Interceptors" and these "Out Interceptors" are called on
#handleMessage() with the outMessage (ie the response created by the
ExceptionMapper) instead of being called on #handleFaultMessage() with
the inMessage when information like the FaultMode is still holded by
the inMessage
Yes
4) Interceptors like the ResponseTimeMessageOutInterceptor who rely on
the faultMode attribute located on the Message that is being passed to
handleMessage/handleFault are lost, they don't find the information
they look for
I see...
Questions :
* Wouldn't it make sense to call the "Out Fault Interceptors" if a
JAX-RS exception is mapped to a custom response ?
Now that you suggested it, perhaps, one alternative in mapping exceptions to
exception mappers would be to
register JAX-RS specific fault interceptors which will do the mapping, instead of doing it in the JAXRSInInterceptor or
JAXRSInvoker...
So other registered fault interceptors will get their chance as well...
What complicates things a bit is that JAXRS users can have ResponseHandler filteres registered which can override the
ExceptionMapper responses...
* which message should be given to the handleFaultMessage() ? The
inMessage that caused the exception and that holds the exception as an
attribute (it would be consistent with JAX-WS) or the outMessage as
currently done ?
Perhaps we should consider introducing JAXRS fault interceptors ? They will do Exception Mapping and abort the chain if the
mapping has been found ? I'm not yet sure how feasible and/or sensitive such a change might be, but may be it will be the right
step forward
CLIENT SIDE JAXRS EXCEPTION HANDLING
=============================================
ClientProxyImpl handles exceptions after calling the interceptors
when, with JAX-WS, exception handling (CheckFaultInterceptor) is
performed in the POST_PROTOCOL phase.
Due to this, the "In Interceptors Chain" is called instead of the "In
Fault Interceptors Chain" and interceptors like
ResponseTimeMessageInInterceptor don't see the Response as an
exception.
Question :
Can we imagine to refactor jaxrs client side exception handling as a
post protocol interceptor ?
The client side needs some refactoring going forward....Some of its code would definitely need to be moved to some isolated
interceptors. However, please see JAXRSSoapBookTest, Eamonn did quite a few tests with faulty features/interceptors/server
faults...
I hope this email was not too long ; it took me few hours to check all
these use cases and figure out how it worked :-)
No problems :-), please type as long a message as you'd like to :-), thanks for
starting this thread
cheers, Sergey
Cyrille
--
Cyrille Le Clerc
[email protected]
