Issue #5939 Use unwrapped exception as exception type for error handling #5940
Conversation
Signed-off-by: olivier lamy <oliver.lamy@gmail.com>
Signed-off-by: olivier lamy <oliver.lamy@gmail.com>
Signed-off-by: olivier lamy <oliver.lamy@gmail.com>
| @@ -75,7 +75,19 @@ public String getErrorPage(HttpServletRequest request) | |||
| if (errorPage != null) | |||
| matchedThrowable = exClass; | |||
|
|
|||
| th = (th instanceof ServletException) ? ((ServletException)th).getRootCause() : null; | |||
| if (th instanceof ServletException) | |||
There was a problem hiding this comment.
If we change the ERROR_EXCEPTION here, then the LOG.DEBUG generated down at line 141/142 will be wrong.
Also, presumably jetty has had this behaviour for a very long time, so I'm wary about changing it ... @gregw WDYT?
There was a problem hiding this comment.
I think we need to provide a compliance mode to at least be able to switch back to the old behaviour, as we have offered this way for some time.
| th = ((ServletException)th).getRootCause(); | ||
| if (th != null) | ||
| { | ||
| request.setAttribute(Dispatcher.ERROR_EXCEPTION_TYPE, th.getClass()); |
There was a problem hiding this comment.
I don't think it is correct to do this here as we are walking the hierarchy.
I think the resetting of Dispatcher.ERROR_EXCEPTION_TYPE (and probably Dispatcher.ERROR_EXCEPTION) should be done below, only if we have an error page AND we are not in the compliance mode.
|
@olamy Do you want me to add the compliance mode switch for this? |
|
Also this needs to be fixed in 10 and merged to 11. We don't want 11 to diverge from 10 |
|
replaced by #5958 |
#5939
Signed-off-by: olivier lamy oliver.lamy@gmail.com