Hi,

Thanks for the update. I'm building my application against 2.7.1-SNAPSHOT
now so I can try out a fix once you've had a time to look at this :-).

Thanks
Duncan

On 25/10/2012 18:15, "Sergey Beryozkin" <[email protected]> wrote:

>On 25/10/12 17:17, Sergey Beryozkin wrote:
>> Hi Duncan
>>
>> Thanks for the analysis, comments inline
>>
>> On 25/10/12 14:18, Duncan Keysell (dkeysell) wrote:
>>> Hi,
>>>
>>> In the application code for my REST service all the exceptions are
>>> wrapped in WebApplicationException and then thrown. I have a test case
>>> to check this is working and below is the resource called in that test
>>> case:
>>>
>>>
>>> @GET
>>>
>>> @Path("/plainBad")
>>>
>>> public MgmtResponse throwPlainException() throws Exception {
>>>
>>> throw new WebApplicationException(new Exception("Some lame message"));
>>>
>>> }
>>>
>>>
>>> I have an exception mapper (implementation of
>>> ExceptionMapper<WebApplicationException>) that creates the Response.
>>> This Response contains the message from the original exception as well
>>> as the exception type, up until I have moved to use 2.7.0.
>>>
>>>
>>> This is due to a change in the JAXRSUtils.convertFaultToResponse(T,
>>> Message) message between 2.6.2 and 2.7.0.
>>>
>>>
>>> Previously the method was (line 1217):
>>>
>>>
>>> public static<T extends Throwable> Response convertFaultToResponse(T
>>> ex, Message inMessage) {
>>>
>>>
>>>
>>> ExceptionMapper<T> mapper =
>>>
>>> 
>>>ProviderFactory.getInstance(inMessage).createExceptionMapper(ex.getClass
>>>(),
>>> inMessage);
>>>
>>> if (mapper != null) {
>>>
>>> try {
>>>
>>> return mapper.toResponse(ex);
>>>
>>> } catch (Exception mapperEx) {
>>>
>>> mapperEx.printStackTrace();
>>>
>>> return Response.serverError().build();
>>>
>>> }
>>>
>>> }
>>>
>>>
>>>
>>> return null;
>>>
>>>
>>>
>>> }
>>>
>>>
>>> And now it is (line 1352):
>>>
>>>
>>> @SuppressWarnings("unchecked")
>>>
>>> public static<T extends Throwable> Response convertFaultToResponse(T
>>> ex, Message inMessage) {
>>>
>>> ProviderFactory factory = ProviderFactory.getInstance(inMessage);
>>>
>>> ExceptionMapper<T> mapper = factory.createExceptionMapper(ex,
>>>inMessage);
>>>
>>> if (mapper != null) {
>>>
>>> if (ex.getClass() == WebApplicationException.class
>>>
>>> && mapper.getClass() != WebApplicationExceptionMapper.class) {
>>>
>>> WebApplicationException webEx = (WebApplicationException)ex;
>>>
>>> Class<?> exceptionClass =
>>> getWebApplicationExceptionClass(webEx.getResponse(),
>>>
>>> WebApplicationException.class);
>>>
>>> if (exceptionClass != WebApplicationException.class) {
>>>
>>> try {
>>>
>>> Constructor<?> ctr = exceptionClass.getConstructor(Response.class);
>>>
>>> ex = (T)ctr.newInstance(webEx.getResponse());
>>>
>>> } catch (Exception ex2) {
>>>
>>> ex2.printStackTrace();
>>>
>>> return Response.serverError().build();
>>>
>>> }
>>>
>>> }
>>>
>>> }
>>>
>>>
>>>
>>> try {
>>>
>>> return mapper.toResponse(ex);
>>>
>>> } catch (Exception mapperEx) {
>>>
>>> mapperEx.printStackTrace();
>>>
>>> return Response.serverError().build();
>>>
>>> } finally {
>>>
>>> factory.clearExceptionMapperProxies();
>>>
>>> }
>>>
>>> }
>>>
>>>
>>>
>>> return null;
>>>
>>>
>>>
>>> }
>>>
>>>
>>>
>>> So the problem is that the exception 'ex' and my 'mapper' pass the if
>>> statement (line 1356):
>>>
>>>
>>> if (ex.getClass() == WebApplicationException.class
>>>
>>> && mapper.getClass() != WebApplicationExceptionMapper.class)
>>>
>>>
>>> And then the ex gets over written and I loose all the details that the
>>> original WebApplicationException was holding. Shouldn't the if
>>> statement be changed not to allow any mapping implementing
>>> ExceptionMapper<WebApplicationException>? Is there some workaround for
>>> this?
>>>
>> I'm going to work on test case shortly. The actual change was needed to
>> get new JAX-RS 2.0 exceptions such as NotFoundException (alternative to
>> new WebApplicationException(404)), etc, captured by existing
>> ExceptionMapper<WebApplicationException> mappers if no a mapper like
>> ExceptionMapper<NotFoundException> exists, but a regression has been
>> introduced.
>>
>It is the other way around, if we have
>ExceptionMapper<NotFoundException> but the code has thrown "new
>WebApplicationException(404)" then NotFoundExceptionMapper should be
>chosen given that NotFoundException extends WebApplicationException.
>
>I can see that the exception is lost in all the cases where the origin
>ex is WebApplicationException and the mapper is not known to CXF, it is
>really to do with a wrong constructor check, still looking, but will
>likely fix early next week due to the long weekend coming in
>
>The tests  show that if you register say
>ExceptionMapper<InternalServerErrorException> and throw
>InternalServerErrorException then no cause exception is lost, in
>meantime I'll have a look at the other cases
>
>Cheers, Sergey
>
>> The possible workarounds: provide ExceptionMapper<ServerErrorException>
>> or extend CXF WebApplicationExceptionMapper. Another one is CXF specific
>> and it is to register a custom CXF invoker which will get a chance to
>> capture the original WebApplicationException, I guess it should be the
>> last resort option, I'll provide more info if doing other exception
>> mappers won't help
>>
>> Thanks, Sergey
>>
>>
>>
>>>
>>>
>>> Thanks
>>>
>>> Duncan
>>>
>>>
>>>
>>>
>>
>
>
>-- 
>Sergey Beryozkin
>
>Talend Community Coders
>http://coders.talend.com/
>
>Blog: http://sberyozkin.blogspot.com

Reply via email to