You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "Felix Meschberger (JIRA)" <ji...@apache.org> on 2014/11/12 14:22:35 UTC
[jira] [Reopened] (SLING-4143) Exception or Error in error handler
script/servlet should cause HTTP status 500
[ https://issues.apache.org/jira/browse/SLING-4143?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Felix Meschberger reopened SLING-4143:
--------------------------------------
Sorry to reopen.
I basically like this restructuring to the DefaultErrorHandler and the forwarding. I have just concerns with:
* Is Java 6 really required for this change ? Else, lets not change something that is not required to be changed.
* Special casing of Throwable: To my knowledge there is no Throwable which is neither an Error nor an Exception. Hence not handling Throwable but Error and Exception is pointless (in the SlingServletResolver)
* Likewise catchting Error and Exception but not Throwable is also pointless because all of them are Throwable and there are not other Throwables (in the DefaultErrorHandler)
* The biggest issue, though, I have is that if the actual error handler fails, an error is sent about the failing error handler instead of the actual error having happened (in the DefaultErrorHandler). This is really not very helpful: If an error happens that error should be reported. And if the selected error handler fails, the fallback reporting should take place. Of course, logging failure of the selected error handler makes perfect sense.
> Exception or Error in error handler script/servlet should cause HTTP status 500
> -------------------------------------------------------------------------------
>
> Key: SLING-4143
> URL: https://issues.apache.org/jira/browse/SLING-4143
> Project: Sling
> Issue Type: Bug
> Components: Engine
> Affects Versions: Servlets Resolver 2.3.2, Engine 2.3.8
> Reporter: Bertrand Delacretaz
> Assignee: Bertrand Delacretaz
> Fix For: Servlets Resolver 2.3.4, Engine 2.3.10
>
>
> It looks like Sling does not set the response status if a custom error handler script is present which does not set the status itself. See examples below.
> I think Sling should detect this and set a sensible non-200 status in this case.
> Example 1: script doesn't set status _(update: after discussion on list we won't change this)_
> {code}
> $ cat > 404.jsp
> <html>Custom 404 page</html>
> $ curl -u admin:admin -T 404.jsp http://localhost:8888/apps/sling/servlet/errorhandler/404.jsp
> $ curl -D - http://localhost:8888/nonexisting
> HTTP/1.1 200 OK
> ...
> <html>Custom 404 page</html>
> {code}
> Example 2, scripts throws a RuntimeException - blank response with 200 status - this is wrong
> {code}
> $ cat > 404.jsp
> <% if(true) throw new RuntimeException("404 page failed"); %>
> $ curl -u admin:admin -T 404.jsp http://localhost:8888/apps/sling/servlet/errorhandler/404.jsp
> $ curl -D - http://localhost:8888/nonexisting
> HTTP/1.1 200 OK
> ...
> Content-Length: 0
> Server: Jetty(7.x.y-SNAPSHOT)
> {code}
> Example 3: scripts sets status - this one looks correct.
> {code}
> $ cat > 404.jsp
> <% response.setStatus(421); %>
> Custom 421 page
> $ curl -u admin:admin -T 404.jsp http://localhost:8888/apps/sling/servlet/errorhandler/404.jsp
> $ curl -D - http://localhost:8888/nonexisting
> HTTP/1.1 421 421
> ...
> Server: Jetty(7.x.y-SNAPSHOT)
> Custom 421 page
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)