You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Bertrand Delacretaz <bd...@apache.org> on 2014/11/07 16:21:34 UTC
Custom error handler scripts return HTTP status 200 by default,
sounds wrong to me
Hi,
See SLING-4143 for the details.
I think we should set a non-200 HTTP status if an error handler script
does not set the it, what do people think?
-Bertrand
Re: Custom error handler scripts return HTTP status 200 by default,
sounds wrong to me
Posted by Dominique Jaeggi <dj...@adobe.com>.
On Mon, Nov 10, 2014 at 1:09 PM, Felix Meschberger <fm...@adobe.com> wrote:
> I have the guts feeling that the Sling error handling mechanism should not do anything. Rather we should expect the error handler script to set the status appropriately.
related: https://issues.apache.org/jira/browse/SLING-4049
Re: Custom error handler scripts return HTTP status 200 by default,
sounds wrong to me
Posted by Felix Meschberger <fm...@adobe.com>.
Hi
> Am 12.11.2014 um 14:45 schrieb Bertrand Delacretaz <bd...@apache.org>:
>
> Hi Felix,
>
> On Wed, Nov 12, 2014 at 2:24 PM, Felix Meschberger <fm...@adobe.com> wrote:
>> Consider this case:
>> * Client browses non-existing /a/b/xyz.html
>> * Not found, hence 404
>> * 404 handler fails due to some bad programming
>> * client gets 500
>
>> I don’t think that is ok...
>
> I understand your point but what would you consider ok then?
As I said: log the error handler failure but still send the original error to the client. To the client it is better to get a not-so-nice-but-correct response than to get a confusing response.
>
> The error handler failing is a somewhat catastrophic situation, which
> IMO deserves a 500 status.
Yes, but ...
>
> If we return 404 a monitoring system might not notice the problem.
.. whom are you sending the error message to ? The monitoring system or the client browsing the server ? The client is not interested in seeing a failing system. The client is interested in a direct response to his/her actions. Hence 404 is the right reaction to the not-found-situation.
The failing error handler is an internal implementation detail and must not be exposed to the client.
But yes, I agree, we have to record the failure. This is done in the error log and the monitoring system is primarily looking at the error log not at the client response.
Regards
Felix
>
> -Bertrand
Re: Custom error handler scripts return HTTP status 200 by default,
sounds wrong to me
Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi Felix,
On Wed, Nov 12, 2014 at 2:24 PM, Felix Meschberger <fm...@adobe.com> wrote:
> Consider this case:
> * Client browses non-existing /a/b/xyz.html
> * Not found, hence 404
> * 404 handler fails due to some bad programming
> * client gets 500
> I don’t think that is ok...
I understand your point but what would you consider ok then?
The error handler failing is a somewhat catastrophic situation, which
IMO deserves a 500 status.
If we return 404 a monitoring system might not notice the problem.
-Bertrand
Re: Custom error handler scripts return HTTP status 200 by default,
sounds wrong to me
Posted by Felix Meschberger <fm...@adobe.com>.
Hi
> Am 12.11.2014 um 13:43 schrieb bdelacretaz@apache.org:
>
> On Mon, Nov 10, 2014 at 1:50 PM, Bertrand Delacretaz
> <bd...@apache.org> wrote:
>>
>> ...in some
>> cases Sling returns a 200 status if the error script fails, as in my
>> SLING-4143 "example 2". I think we should at least fix that one...
>
> I have fixed that now, details in SLING-4143 - my changes only cause a
> 500 status if the error handling script or servlet throws an Exception
> or Error.
Sorry 'bout that, but I have to reopen: You must not report the failing error handler to the client but only the original issue. You may certainly log the failing error handler.
Consider this case:
* Client browses non-existing /a/b/xyz.html
* Not found, hence 404
* 404 handler fails due to some bad programming
* client gets 500
I don’t think that is ok.
Regards
Felix
Re: Custom error handler scripts return HTTP status 200 by default,
sounds wrong to me
Posted by Bertrand Delacretaz <bd...@apache.org>.
On Mon, Nov 10, 2014 at 1:50 PM, Bertrand Delacretaz
<bd...@apache.org> wrote:
>
> ...in some
> cases Sling returns a 200 status if the error script fails, as in my
> SLING-4143 "example 2". I think we should at least fix that one...
I have fixed that now, details in SLING-4143 - my changes only cause a
500 status if the error handling script or servlet throws an Exception
or Error.
-Bertrand
Re: Custom error handler scripts return HTTP status 200 by default,
sounds wrong to me
Posted by Bertrand Delacretaz <bd...@apache.org>.
On Mon, Nov 10, 2014 at 1:09 PM, Felix Meschberger <fm...@adobe.com> wrote:
>
> ...OTOH the Servlet API states that the setStatus is actually disabled when calling the error handling JSP (if it
> is a JSP) and thus probably implies that the servlet container presets the status to the actual status code
> and doesn’t allow the error handler to change it....
Right, so that's another point for making sure Sling sets the default
status code, according to what's going on, in case the script doesn't.
But as Carsten says that's tricky.
I'm still not sure either what the best way is in general, but in some
cases Sling returns a 200 status if the error script fails, as in my
SLING-4143 "example 2". I think we should at least fix that one.
-Bertrand
Re: Custom error handler scripts return HTTP status 200 by default,
sounds wrong to me
Posted by Felix Meschberger <fm...@adobe.com>.
Hi
I have the guts feeling that the Sling error handling mechanism should not do anything. Rather we should expect the error handler script to set the status appropriately.
OTOH the Servlet API states that the setStatus is actually disabled when calling the error handling JSP (if it is a JSP) and thus probably implies that the servlet container presets the status to the actual status code and doesn’t allow the error handler to change it.
I am not really sure, whether we should actually go as far as setting the status in the Sling error handling before calling the actual error handler and then disable setStatus ...
Regards
Felix
> Am 08.11.2014 um 11:53 schrieb Carsten Ziegeler <cz...@apache.org>:
>
> Am 07.11.14 um 18:32 schrieb Bertrand Delacretaz:
>> On Fri, Nov 7, 2014 at 5:03 PM, Carsten Ziegeler <cz...@apache.org> wrote:
>>> Am 07.11.14 um 16:21 schrieb Bertrand Delacretaz:
>>>> ... I think we should set a non-200 HTTP status if an error handler script
>>>> does not set the it, what do people think?
>>>>
>>> I'm not sure - I would imagine that if an error handler can handle the
>>> error and return a "correct" response, 200 would be a valid status...
>>
>> We could wrap the response's setStatus (and related) methods so as to
>> detect whether the script has explicitly set a status, and if not set
>> one.
>>
> The point is, if a handler creates a valid response it does not call
> setStatus with the 200 value as it simply assumes that this is the
> status anyway.
>
> So I think we should leave it up to the error handler to correctly
> handle the status and not try to be smarter than everyone else.
>
> Carsten
>
>> Or, if calling response.setStatus several times is allowed (I think
>> that's the case but haven't tried) simply make an initial call to that
>> setting the status that the error handler is called for.
>>
>> -Bertrand
>>
>
>
> --
> Carsten Ziegeler
> Adobe Research Switzerland
> cziegeler@apache.org
Re: Custom error handler scripts return HTTP status 200 by default,
sounds wrong to me
Posted by Carsten Ziegeler <cz...@apache.org>.
Am 07.11.14 um 18:32 schrieb Bertrand Delacretaz:
> On Fri, Nov 7, 2014 at 5:03 PM, Carsten Ziegeler <cz...@apache.org> wrote:
>> Am 07.11.14 um 16:21 schrieb Bertrand Delacretaz:
>>> ... I think we should set a non-200 HTTP status if an error handler script
>>> does not set the it, what do people think?
>>>
>> I'm not sure - I would imagine that if an error handler can handle the
>> error and return a "correct" response, 200 would be a valid status...
>
> We could wrap the response's setStatus (and related) methods so as to
> detect whether the script has explicitly set a status, and if not set
> one.
>
The point is, if a handler creates a valid response it does not call
setStatus with the 200 value as it simply assumes that this is the
status anyway.
So I think we should leave it up to the error handler to correctly
handle the status and not try to be smarter than everyone else.
Carsten
> Or, if calling response.setStatus several times is allowed (I think
> that's the case but haven't tried) simply make an initial call to that
> setting the status that the error handler is called for.
>
> -Bertrand
>
--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org
Re: Custom error handler scripts return HTTP status 200 by default,
sounds wrong to me
Posted by Bertrand Delacretaz <bd...@apache.org>.
On Fri, Nov 7, 2014 at 5:03 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> Am 07.11.14 um 16:21 schrieb Bertrand Delacretaz:
>>... I think we should set a non-200 HTTP status if an error handler script
>> does not set the it, what do people think?
>>
> I'm not sure - I would imagine that if an error handler can handle the
> error and return a "correct" response, 200 would be a valid status...
We could wrap the response's setStatus (and related) methods so as to
detect whether the script has explicitly set a status, and if not set
one.
Or, if calling response.setStatus several times is allowed (I think
that's the case but haven't tried) simply make an initial call to that
setting the status that the error handler is called for.
-Bertrand
Re: Custom error handler scripts return HTTP status 200 by default,
sounds wrong to me
Posted by Carsten Ziegeler <cz...@apache.org>.
Am 07.11.14 um 16:21 schrieb Bertrand Delacretaz:
> Hi,
>
> See SLING-4143 for the details.
>
> I think we should set a non-200 HTTP status if an error handler script
> does not set the it, what do people think?
>
I'm not sure - I would imagine that if an error handler can handle the
error and return a "correct" response, 200 would be a valid status.
And in that case you usually don't explicitely set this in the error
handler. So if we do as suggested, we might break working error handlers
Carsten
--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org