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? General-purpose ones should be able to fit in nicely IMO.
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
>>