Clarify whether javax.servlet.error.exception_type and javax.servlet.error.exception refer to a wrapped or unwrapped ServletException

[janbartel]

From section 10.9.2 Error Pages:

If no error-page declaration containing an exception-type fits using the class-
hierarchy match, and the exception thrown is a ServletException or subclass
thereof, the container extracts the wrapped exception, as defined by the
ServletException.getRootCause method. A second pass is made over the error
page declarations, again attempting the match against the error page declarations,
but using the wrapped exception instead.

This section does not make clear when a ServletException is thrown that contains a wrapped exception, if the values of the request attributes javax.servlet.error.exception_type and javax.servlet.error.exception should be the unwrapped exception or not.

Setting them to the unwrapped exception will remove stacktrace information that may be valuable.

On the other hand, the current servlet-tck seems to test that the values are unwrapped - see https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/src/com/sun/ts/tests/servlet/spec/errorpage/URLClient.java#L76

[markt-asf]

I think the TCK is checking that the original exception was thrown without wrapping.
Section 9.5 sets out which exceptions should be wrapped if the target of a RequestDispacther throws an exception. My understanding is that a Servlet is expected to behave the same way. i.e.:

• throw RuntimeException, ServletException and IOException
• wrap everything else in ServletException and re-throw

Looking at the specification I don't see that made explicit anywhere. I think it should be. We can also make clear that jakarta.servlet.error.exception and friends should be set to the exception that was thrown (i.,e. not unwrapped).

[gregw]

@markt-asf my understanding of that TCK test is that the servlet throws the wrapped exception - ie it throws new ServletException(new IllegalStateException()).

To my mind it is wrong to unwrap a ServletException, because it throws away application context of who did the throwing and only gives us the root cause, which may not be the servlet that was called.
In fact the only place I can see the spec talk about unwrapping is if we are searching for an error page and there is not one defined for ServletException.

[janbartel]

@markt-asf the errorpage tests are invoked via reflection from the HttpTCKServlet class. When the test throws the exception, the HttpTCKServlet catches it and wraps it in a ServletException:
https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/src/com/sun/ts/tests/servlet/common/servlets/HttpTCKServlet.java#L93

[olamy]

My concern is the consistency of the content of attributes related to the mechanism of unwrapped if it's ServletException.
If you have

  <error-page>
    <exception-type>java.lang.IllegalArgumentException</exception-type>
    <location>/howToManageMyIllegalArgumentException</location>
  </error-page>

with as the TCK does new ServletException(new IllegalArgumentException()).

As per specification the request is redirected to /howToManageMyIllegalArgumentException but:

• attribute jakarta.servlet.error.exception contains the ServletException instance.
• attribute javax.servlet.error.exception_type TCK test IllegalArgumentException.class but not defined by the specs.

I guess we need at least consistency (I would say attribute jakarta.servlet.error.exception should contain the IllegalArgumentException instance)

[markt-asf]

@janbartel Tx for the pointer. I'll take a closer look.

[markt-asf]

I've been digging through the history and I think the current behaviour on unwrapping is as intended. The following change was made in Tomcat 4 back when it was the reference implementation: https://svn.apache.org/viewvc?view=revision&revision=301155

There is also the following text in the current servlet specification:

These attributes allow the servlet to generate specialized content depending on the status code, the
exception type, the error message, the exception object propagated, and the URI of the request
processed by the servlet in which the error occurred (as determined by the getRequestURI call), and the
logical name of the servlet in which the error occurred.

My reading of this is that it is expected that the error page might provide details specific to an exception type and therefore if the unwrapped exception is used to determine the error page, it is the unwrapped exception that needs to be set in the request attributes. I can image this being done for exceptions like IllegalFormatConversionException or more likely custom application provided exceptions.

That the TCK is testing this is a further indication that this is the intended behaviour.

I confess I was initially surprised by this behaviour. It was only when I dug into the history did it start to make sense.

Whether we agree the current behaviour is correct or needs to change, I think we need to clarify 10.9.1 and make it explicit that exceptions throws by Servlets should be optionally wrapped using the same rules as set out in 9.5.

@olamy In Tomcat, at least, jakarta.servlet.error.exception and jakarta.servlet.error.exception_type are always consistent (as are the Java EE equivalents). The TCK test expects them to be consistent.