Re: [servlet-dev] Async sendError with error pages?

[Mark Thomas <markt@xxxxxxxxxx>]

Hi,

There are a fair number of permutations and combinations to test here.
Even more once you add h2 and/or async IO into the mix which I haven't
done yet.

The factors my tests cover so far are:
- sync or async servlet
- error triggered by sendError or unhandled Exception

Further, for async servlet
- Use complete() or dispatch()
- Use AsyncContext.start(Runnable) or app provided Thread
- When to trigger the error
  - Thread A before startAsync
  - Thread A after startAsync, before Thread B is started
  - Thread A after startAsync, after Thread B is started
  - Thread B before complete()/dispatch()
  - Thread B after complete()/dispatch()

I've found one edge case we could handle better: Unhandled Exception on
Thread B started via AsyncContext.start(Runnable).

Generally, the understanding set out below is correct. I've added a few
comments.

<snip/>

>     On 09/07/2019 14:15, Greg Wilkins wrote:
<snip/>

>     > I think that jetty will change to follow the implementations of Tomcat
>     > and Undertow.   Which if I understand correctly is as follows for an
>     > non-async caller:
>     >
>     >   * Thread A Normal REQUEST dispatch
>     >   * Thread A calls sendError (state changed, output closed, but
>     response
>     >     not committed)
>     >   * Thread A returns from REQUEST dispatch
>     >   * Thread A output reopened, ERROR dispatch.
>     >   * Thread A generates error page (perhaps calling startAsync and an
>     >     extended lifecycle).

Correct.

>     > But we'd also like to handle async callers of sendError.   Do your
>     > containers handling it like this?:
>     >
>     >   * Thread A Normal REQUEST dispatch
>     >   * Thread A calls startAsync
>     >   * Thread B calls sendError  (state changed, output closed, but
>     >     response not committed)
>     >   * Winner of race between Thread A returning from REQUEST
>     >     dispatch and Thread B calling complete() does noop
>     >   * Loser of race reopens output and does ERROR dispatch

noop glosses over some internal complexities but it is a reasonable
description from the apps point of view.

>     >   * Thread A/B generates page (perhaps calling startAsync and an
>     >     extended lifecycle).
>     >   * Thread A/B calls onComplete when ERROR dispatch returns or second
>     >     async lifecycle completes.
>     >
>     > I think that works for an async thread calling sendError then
>     complete,
>     > but there are some edge cases that need to be considered.  What if it
>     > calls sendError and then AsyncContext.dispatch?

Broadly the same behaviour as in a synchronous servlet if the app calls
sendError() and then continues writing to the response.

>     > What if it calls sendError and then times out?

The listener gets called which calls dispatch()/complete() and
processing continues as if the servlet made that call.

>     > I think that sendError followed by AsyncContext.dispatch should be
>     > handled with an ISE, just like trying to use a normal dispatcher would
>     > if sendError had been called.

An async dispatch seems closer to RequestDispatcher.include than
.forward but I see where you are coming from. The app gets an ISE but
does the container handler the original error or the ISE? I'd lean
towards the original error.

>     > I'm not sure at all what to do on the timeout.   If there is no
>     > onTimeout listener, then we'd do a ERROR dispatch for the timeout, so
>     > perhaps we do the same:
>     >
>     >   * we call any onTimeout listeners, if they call dispatch, then it is
>     >     ISE. If they call complete then we have our complete call and
>     >     proceed as above.
>     >   * If there are no onTimeout listeners or if they don't call
>     complete,
>     >     then we just do the ERROR dispatch for the sendError rather
>     than for
>     >     the timeout?

I think so.

I am concerned that there are a LOT of moving parts here. Anything we
can do in the next iteration of the spec to clarify the expected
behaviour would be welcome.

Mark