Hi,
On 22/05/12 17:56, Marek Potociar wrote:
>
> On May 18, 2012, at 11:29 AM, Sergey Beryozkin wrote:
>
>> Hi Bill,
>> On 17/05/12 15:27, Bill Burke wrote:
>>>
>>>
>>> On 5/16/12 7:57 AM, Sergey Beryozkin wrote:
>>>>> try {
>>>>> String response = target.request("text/plain").get(String.class);
>>>>> ...
>>>>> } catch (RedirectionException ex) {
>>>>> // ok, seems I have a wrong link, let's try the correct link, if
>>>>> available
>>>>> } catch (ClientErrorException ex) {
>>>>> // i sent a wrong request, let's try to fix it and resend
>>>>> } catch (ServerErrorException ex) {
>>>>> // uups, there's nothing i can do about it
>>>>> }
>>>>>
>>>>> The above seems to make sense in theory, but I admit I am not sure if
>>>>> it would work well in practice. Perhaps a flat structure with
>>>>> exceptions dedicated to specific error codes is enough in the end.
>>>>>
>>>> I think that might not be ideal when the writing the end-user (device)
>>>> code against say OAuth authorization servers, where the redirect is part
>>>> of the overall solution. IMHO the redirect is not a fault, it is the
>>>> server guiding the client to the appropriate resource. It might be
>>>> treated as the fault but it will not be the right approach for all the
>>>> redirects. May be we can get a new thread for discussing how the client
>>>> should react to redirects, is it an exception or a null return (in case
>>>> of typed responses), etc
>>>>
>>>
>>> IMO, you *have* to have redirect exceptions for the case of clients
>>> invocations that don't return a Response. Automatic redirection should
>>> not be default behavior as it can be perceived as a security risk.
>>
>> Agreed about the automatic redirection.
>>
>> These new exception classes are supposed to be used at both ends, and
>> as I said, the server redirecting the client is not the exceptional
>> flow in many cases. It's the normal application flow in many cases, I
>> guess in all sort of SSO enforcement points, OAuth authorization
>> servers, etc.
>
> I guess the redirection exceptions will be mostly used on the client
> side (even though I am not saying that they could not be used on server
> side as well, but client side is the primary use case IMO), specifically
> the case when a user uses the client API to directly retrieve an entity
> of some Java type expecting a 2xx response for the request:
>
> String response = client.target(...).request(...).get(String.class);
> // process response
>
> Here the runtime would throw an exception in case the response status
> code is not 2xx indicating that some important information about the
> response may be missed. So the client anticipating a redirect scenario
> would write something along these lines:
>
> String response;
> try {
> response = client.target(...).request(...).get(String.class);
> } catch (RedirectionException ex) {
> // handle redirect
> }
> // process response
>
> Note that some redirect scenarios (e.g. authentication/authorization)
> that you mentioned above may require updating client's or target's
> configuration instance with proper filters.
>
>>
>> The question is how to react to the redirect status on the client side
>> in cases when the custom type is expected is a bit orthogonal.
>
> Not sure I fully follow. Are you saying that a redirect message may
> contain an unexpected entity type? If so, that's not a problem, is it?
> The exception contains a response with all the headers including
> (hopefully) the response content type. So figuring out the the most
> suitable error response Java type might not be easy under certain
> circumstances but still doable. If anything, unmarshalling the response
> into a String or byte[] should always work.
>
>> I'd prefer the runtime to return null with the client code proceeding
>> to get the actual Response and checking the status. I'm not sure
>> everyone will agree though.
>
> I would not, I'm afraid :) The null should be IMO semantically reserved
> for "no response entity" in the cases discussed above.
>
>> Perhaps there's a case for the exception class, but for the one
>> specific to the client runtime, only, ex, ClientRedirectException,
>> which the client runtime will throw if the response type is not
>> Response and the status is 302 or 303, etc
>
> Why do you see a need for a client-specific exceptions in this case?
Because I agree with you suggesting above that it is mostly of interest
to the client runtime, in case when a specific application type is
expected back and the redirect status is returned. Fine by the way with
'null' reserved for no entity in response cases
> General-purpose ones should be able to fit in nicely IMO.
I'm kind of neutral on it, I think if we can recommend why would a
server code throw a redirect exception then it would definitely be a
general purpose one. I'm just not sure it would be a good idea to throw
the exception in order to get the client following the 'natural'
application path, as opposed to cases like 'resource moved
permanently/temp to'...
Cheers, Sergey
>
> Marek
>>
>>
>>
>>>
>>>
>>> IMO, there is no harm in having a family superclass. I would expect that
>>> most code would look like this:
>>>
>>> try {
>>> String response = target.request("text/plain").get(String.class);
>>> } catch (RedirectException ex) {
>>> // decide whether to redirect
>>> } catch (ServiceUnavailableException ex) {
>>> // check for Retry-After header and retry again
>>> } catch (WhateverTheRootExceptionIs failure) {
>>> // we failed log the failure
>>> }
>>>
>>> Also, might also be nice to have a static from(Response) method that
>>> converts a Response to the approriate exception:
>>>
>>> i.e.
>>>
>>> Response response = target.request().get();
>>> if (!res.getStatus() == OK) {
>>> throw WhateverRootExceptionIs.from(response);
>>> }
>>>
>> Have a look please at exceptions2.patch - the validation is enforced
>> internally on individual exceptions. On the client side the runtime
>> would indeed need to do something like
>>
>> WhateverRootExceptionIs.from(response);
>>
>> but I'm not sure that it can not be done at the individual
>> implementation level ?
>>
>>>
>>>>> In any case, what I am still missing in the solution is a proposal how
>>>>> to handle cases when application code throws directly a WAE with a
>>>>> code covered by a specific exception. Seems to me that for the
>>>>> exception mappers and client side to work correctly, we need to catch
>>>>> these and re-throw as the correct sub-type. Right?
>>>>>
>>>
>>> I thought the root class was going to be WAE?
>>
>> The question is how to make sure an exception mapper for say
>> NotFoundException is invoked when the code throws new
>> WebApplicationException(404)
>>
>> Cheers, Sergey
>>
>>>
>>> Bill
>>>
>