jsr339-experts@jax-rs-spec.java.net

[jsr339-experts] Re: [jax-rs-spec users] Re: Re: Re: ExceptionMappers and WebApplicationExceptions

From: Sergey Beryozkin <sberyozkin_at_talend.com>
Date: Fri, 2 Nov 2012 15:34:04 +0000

Agreed,

Sergey
On 02/11/12 14:35, Marek Potociar wrote:
>
> On Nov 2, 2012, at 3:23 PM, Sergey Beryozkin<sberyozkin_at_talend.com> wrote:
>
>> On 02/11/12 14:05, Marek Potociar wrote:
>>> IMO, taking into account compatibility with existing applications, we
>>> should not modify the exceptions thrown by these applications. So IMO,
>>> this is what we should do:
>>>
>>> - JAX-RS runtime MUST throw a specific WAE subtype. This must be
>>> indicated in the spec.
>>
>> OK
>>
>>> - JAX-RS runtime MUST NOT modify WAE instances raised by the application
>>> code (to avoid compatibility issues).
>>>
>>> I filed a new Jira issue to track this:
>>> http://java.net/jira/browse/JAX_RS_SPEC-306
>>>
>>> The above will ensure that JAX-RS as well as new applications are able
>>> to leverage the new exception hierarchy. It also ensures that existing
>>> applications that throw a WAE today will not se a change in the behavior
>>> with the respect to application-thrown exceptions. Also, it avoids the
>>> need to do too much magic at runtime.
>>>
>>
>> Basically, this means the server-side exception mapping algorithm remains unchanged, right ?
>
> Yes.
>
>> I guess I'm fine with it - given it is simpler.
>>
>> FYI, I do not see how modifying ('promoting') application-thrown WebApplicationExceptions (note, only WebApplicationExceptions) to subclass instances will affect the compatibility, the promoted instance will still be handled by an existing WebApplicationException mapper.
>>
>> I guess it basically makes the idea of users having individual WebApplicationException Mappers not working. For example, one will have to have either a single, catch-all, WebApplicationException mapper, or,
>> mappers for all individual WebApplicationException subclass mappers.
>> As long as WebApplicationException is there the individual subclass mappers will miss on the exceptions they are meant to handle because WebApplicationException mapper will always be preferred...
>
> I assume that new applications will be coded to throw the right sub-types. So the EM hierarchy should work well for them. Old applications are not using the hierarchy anyway. IMO it's more confusing to throw a WAE and then see it being catched by some exception mapper other than the one for WAE.
>
>>
>> We probably need to clarify it in the docs/spec
>
> That's part of the issue I filed.
>
> Marek
>
>>
>> Sergey
>>
>>> Marek
>>>
>>> On Nov 2, 2012, at 1:35 PM, Sergey Beryozkin<sberyozkin_at_talend.com
>>> <mailto:sberyozkin_at_talend.com>> wrote:
>>>
>>>> Not that it should matter for the group and indeed for the spec :-),
>>>> but I need to decide whether to optimize the existing (and buggy) CXF
>>>> code which deals with mapping WebApplicationExceptions to subclass
>>>> mappers on the server (and causes headaches for the users :-)) or
>>>> simply revert to the original 1.1 code, which, for example,
>>>>
>>>> given a thrown WebApplicationExceptions(404) and the existence of
>>>> WebApplicationException and NotFoundException mappers, will ignore
>>>> NotFoundException mapper and select WebApplicationException mapper,
>>>> while, a thrown NotFoundException will be mapped to NotFoundException
>>>> mapper.
>>>>
>>>> As I said before, given that WebApplicationException(404) and
>>>> NotFoundException represent the same error condition, and given the
>>>> specific fact the runtime 'knows' about it by the virtue of both
>>>> exception classes being part of API, it makes sense to make an
>>>> exception to the algorithm specifically and only for the
>>>> WebApplicationException hierarchy,
>>>>
>>>> but I won't be to upset with reverting to the original code either
>>>>
>>>> Cheers, Sergey
>>>>
>>>>
>>>>
>>>> On 30/10/12 10:41, Sergey Beryozkin wrote:
>>>>> On 25/10/12 22:23, Sergey Beryozkin wrote:
>>>>>> On 25/10/12 22:16, Marek Potociar wrote:
>>>>>>> FWIW, what I recall we did agree upon, which is however not yet in the
>>>>>>> spec is that we will make sure that all internal server-side WAEs will
>>>>>>> be changed to proper sub-types.
>>>>>>
>>>>>> Can you clarify that a bit please ? What would be a difference between
>>>>>> the internal code throwing WebApplicationException(500) and application
>>>>>> code doing the same, as far as mapping either of these instances to a
>>>>>> registered InternalServerErrorException mapper ?
>>>>>
>>>>> I'd like to get the agreement on this please. What makes me feel that
>>>>> mapping of WebApplicationException to more specific exception mappers on
>>>>> the server side should work is because say
>>>>>
>>>>> WebApplicationException(500) and InternalServerErrorException are meant
>>>>> to represent exactly the same error condition, so, even though having
>>>>> the current WebApplicationException handled by
>>>>> InternalServerErrorException mapper does not work with the current
>>>>> exception mapping algorithm, the exception has to be made specifically
>>>>> for WebApplicationException hierarchy - it would just avoid the
>>>>> ambiguities, example,
>>>>>
>>>>> "I want to have a generic exception mapping code, implemented with
>>>>> WebApplicationException mapper, and something more specific done in case
>>>>> of 500, implemented with InternalServerErrorException mapper".
>>>>>
>>>>> I know it all can be easily done at the WebApplicationException mapper
>>>>> level itself, but the introduction of the new exception hierarchy will
>>>>> inevitably lead to users wishing to write a cleaner base code for
>>>>> handling all the exceptions without "ifs"
>>>>>
>>>>> Sergey
>>>>>
>>>>>>
>>>>>>> Similarly, we agreed that on client side, proper sub-type will be
>>>>>>> thrown based on response error code instead of a generic WAE.
>>>>>>>
>>>>>> That definitely makes sense
>>>>>>
>>>>>>> As for your question, I'm not sure it is wise to interfere with
>>>>>>> application-thrown exceptions. (And I still cannot recall what we
>>>>>>> decided...)
>>>>>>>
>>>>>> The question is whether some specific treatment is applied to WAE
>>>>>> instances (irrespectively of where they originated from) or not, I've no
>>>>>> strong opinion, in fact I've already implemented what I thought we
>>>>>> might've agreed :-), but that is not important, I can revert, we just
>>>>>> need to agree
>>>>>>
>>>>>> Sergey
>>>>>>
>>>>>>> Marek
>>>>>>>
>>>>>>> On Oct 25, 2012, at 11:10 PM, Marek
>>>>>>> Potociar<marek.potociar_at_oracle.com
>>>>>>> <mailto:marek.potociar_at_oracle.com>> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> On Oct 25, 2012, at 10:57 PM, Sergey
>>>>>>>> Beryozkin<sberyozkin_at_talend.com<mailto:sberyozkin_at_talend.com>>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi
>>>>>>>>>
>>>>>>>>> I would like to clarify the details of the way
>>>>>>>>> WebApplicationExceptions are mapped on the server.
>>>>>>>>>
>>>>>>>>> Here is what I recall we talked about the other day.
>>>>>>>>>
>>>>>>>>> The code throws "new WebApplicationException(404)", and both
>>>>>>>>> WebApplicationException and NotFoundException mappers are available.
>>>>>>>>>
>>>>>>>>> Given NotFoundException is effectively WebApplicationException(404),
>>>>>>>>> NotFoundException mapper is chosen.
>>>>>>>>
>>>>>>>> Hmm... are you referring to a specific section in the spec? I cannot
>>>>>>>> recall we agreed on that one, but I may be wrong.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Similarly, if the code throws "new ServerErrorException(500)" and
>>>>>>>>> both
>>>>>>>>> ServerErrorException and InternalServerErrorException mappers are
>>>>>>>>> available, InternalServerErrorException gets chosen.
>>>>>>>>>
>>>>>>>>> Is it the way it should work ? This obviously is an exception to the
>>>>>>>>> default mapping algorithm, but it appears it is logical given that
>>>>>>>>> the runtime understands the relationship between various API
>>>>>>>>> exception classes
>>>>>>>>
>>>>>>>> As I said, I don't recall such agreement, but my memory is not
>>>>>>>> flawless... If you see it in the spec, then we did agree on that :)
>>>>>>>> (Which still doesn't mean we didn't if you don't see it there...)
>>>>>>>>
>>>>>>>> Marek
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Cheers. Sergey
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>
>