Clarify error page mechanism with regards to committed and async

[gregw]

The error page mechanism defined in section 10.9.2 can be invoked either by an explicit call to sendError or by an exception being thrown from a dispatch to filter(s) and servlet.

However, the current text is not clear what should be done in all possible states with regard to being committed and/or asynchronous states:

• exceptions thrown when the response is already committed
• exceptions thrown when the request is async started (or states beyond)
• sendError called by async thread
• sendError called during dispatch when async started

[gregw]

exceptions thrown when the response is already committed

What should the container do if the response is already committed when an exception is thrown from the filter(s)/servlet?
An error page cannot be generated, but for some protocols it is possible to indicate a non-nominal response:

• reset stream in HTTP/2
• close without final chunk in HTTP/1.1
• close without writing Content-Length (if set) bytes

Should the error page dispatch be done regardless to inform the application?
I think not, because the application already had to opportunity to catch the exception. Thus I think the spec needs to be clarified to indicate that the error page mechanism will only be invoked for non-async requests if the response is not committed. If the response is committed then the container should use whatever protocol options are available to indicate a non-nominal response.

However, if the request is asynchronous, then it is not possible for the container to safely check the committed status of the response, as that will be a race with any asynchronous processing. Thus exceptions thrown for async requests need to be handled in a way that does not directly depend on committed status (see next comment).

[gregw]

exceptions thrown when the request is async started (or states beyond)

What should the container do if the request is async when an exception is thrown from the filter(s)/servlet? This is currently specified in 2.3.3.3:

Any errors or exceptions that may occur during the execution of the dispatch
methods MUST be caught and handled by the container as follows:
i. invoke the AsyncListener.onError(AsyncEvent) method for all instances
of the AsyncListener registered with the ServletRequest for which the
AsyncContext was created and make the Throwable available via the
AsyncEvent.getThrowable() .
ii. If none of the listeners called AsyncContext.complete or any of the
AsyncContext.dispatch methods, then perform an error dispatch with a
status code equal to HttpServletResponse.SC_INTERNAL_SERVER_ERROR
and make the Throwable available as the value of the
RequestDispatcher.ERROR_EXCEPTION request attribute.
iii. If no matching error page is found, or the error page does not call
AsyncContext.complete() or any of the AsyncContext.dispatch methods,
then the container MUST call AsyncContext.complete .

However, this does not discuss how the exception should be handled if the async request has already been completed or dispatched?

If the async is started, then handling of an exception can be done as per 2.3.3.3 above - ie call onError handlers and if none of them call complete/dispatch to an ERROR dispatch. Note that it is impossible to tell if a complete/dispatch call is done by the onError calls or by an asynchronous thread doing normal processing, so error handling is in a race with normal async processing. Applications need to be told that calls to onError need to be implemented so as to atomically abort asynchronous threads from completing/dispatching the response.

If AsyncContext.complete() or AsyncContext.dispatch() has already been called when the exception is caught, then race described above is already lost and the response may have already been generated, and it may or may not have been committed. So it would be possible in these state specially, however because of the race above any special handling would be problematic. Thus it is best that 2.3.3.3 processing is always done - ie call the onError handlers and then do an ERROR dispatch of neither complete or dispatch have been called. If they were actually called before the onError handlers, then this is an intrinsic race that has to be resolved by the Atomic behaviours of application onError listeners and/or AsyncContext itself. Ie after calling onError, dispatch may be seen to have been called, but was actually called by the original async processing thread - it doesn't matter and the container should dispatch it anyway. Ditto, if complete has been called, it doesn't matter if it was called by the onError handlers or the async handling, the request is complete and should be handled as such.

Note that if the 2.3.3.3 processing determines that an error dispatch should be done, then the committed problem of the previous comment still applies. Thus we should modify 2.3.3.3ii to say that if the response is committed, the container will attempt to whatever protocol options are available to indicate a non-nominal response and will not do the error dispatch.

[gregw]

sendError called by an async thread

sendError can be called by an asynchronous thread with the request in at least the following states:

• The original dispatch has already completed.
• The original dispatch is active and ends normally
• The original dispatch is active and throws an exception after the sendError call

** sendError called during dispatch when async started**

• The original dispatch calls sendError while async is started
• The original dispatch calls sendError after AsyncContext.complete has already been called.

Note that the original dispatch calls sendError after AsyncContext.dispatch has already been called can be handled with a ISE as this is essentially attempting a dispatch of a committed response.

Note also that it is essentially impossible to distinguish between these two sets of cases as it is not generally possible to know if sendError is being called by the normal asynchronous processing or by the original dispatch.

A well behaved async thread should call sendError as follows:

    response.sendError(code);
    asyncContext.complete();

Since the sendError handling may involve a error dispatch, which itself may do startAsync and do asynchronous handling, then the sendError handling needs to be deferred until after the complete call, so that it does not race/interfere any async error dispatch handling.

Furthermore, as sendError processing can involve an error dispatch and it is not allowable to do this when there is already a dispatch active, then any sendError processing needs to also be deferred until after the original dispatch has returned.

Once the original dispatch has returned normally and complete() has been called, them the normal error page mechanism as per 10.9.2 can be invoked.

But what if the original dispatch throws an exception after sendError has been called? Should it wait for complete() to be called before handling either error, on the assumption that it was the async thread that called sendError and that it will soon be calling complete(), which may interfere with any error dispatch if we don't wait. But what if the sendError was called by the original dispatch just before it threw and this no call to complete() is coming? I think it is impossible to distinguish between those two cases, so as the former is more common we should assume that complete() will be called after sendError and if it is not then a timeout will need to save us.

Thus 2.3.3.3 needs to be made to delay the error dispatch of sendError has been called, until complete() is called. I propose the following text:

Any errors or exceptions that may occur during the execution of the dispatch 
methods MUST be caught and handled by the container as follows:

  i. invoke the AsyncListener.onError(AsyncEvent) method for all instances
  of the AsyncListener registered with the ServletRequest for which the
  AsyncContext was created and make the Throwable available via the
  AsyncEvent.getThrowable().  If AsyncContext.sendError is invoked either
  before or after all the listeners have called, then there is no further processing
  of the thrown exception and the handling will continue when either 
  AsyncContext.complete() is called or the AsyncContext timeout applies.
  
  ii. if there have been no calls to AsyncContext.complete or any of the 
  AsyncContext.dispatch methods, then either: for an already committed response 
  use whatever protocol mechanisms are available to indicate a non nominal 
  response (eg incomplete response); or for non committed responses perform 
  an error dispatch with a status code equal to 
  HttpServletResponse.SC_INTERNAL_SERVER_ERROR and make the 
  Throwable available as the value of the 
  RequestDispatcher.ERROR_EXCEPTION request attribute.

  iv. If no matching error page is found, or the error page does not call
  AsyncContext.complete() or any of the AsyncContext.dispatch methods,
  then the container MUST call AsyncContext.complete .