Feature proposal: Signal client disconnect using Request.signal

[brettwillis]

As I understand it, when a client disconnects, the context/"thread" for a request is aborted as is. And if ctx.waitUntil() was called, then those promises are allowed some time to resolve.

When you are running async callbacks using ctx.waitUntil(promise), there is no easy way to know that the client has disconnected.

Conceptually, if you responded with a ReadableStream, then controller.enqueue(chunk) should throw or writer.write(chunk) of the writable side should reject on the next write. This would require keeping the context alive with ctx.waitUntil(writable.closed) or something like that, otherwise the context would just die immediately anyway. However I've found that these don't seem to reliably throw/reject when the client disconnects. Perhaps that is a separate issue.

Much of what I'm describing above is from experience/testing or examining workerd rather than documentation, so feel free to correct me.

Proposal

The Request.signal API is an existing standard API which reflects the AbortSignal passed when constructing the request. However the signal property of the incoming request currently does nothing.

I propose that the Workers runtime utilise this existing API by "aborting" the signal of the incoming request when a client disconnects. Any scripts that are interested in reacting when the client has disconnected can utilise this signal.

If an abort listener has been registered using request.signal.addEventListener('abort', ...) then the runtime will call those handler(s) when the client disconnects, before aborting the context. If the handler(s) synchronously call ctx.waitUntil(promise) then that will keep the context alive for a further period of time.

Credit to uri2 here for this inspiration of this idea, as other proposals I had entertained were new non-standard APIs such as ctx.addEventListener('end', ...) etc.

I'm happy to contribute if this proposal makes it through.

Use cases

The use-cases I require this for are scenarios when the Worker is generating a long-running response stream on the fly, for example Server-sent events.

When the client disconnects from the stream, it is useful to react to perform logging, or do cleanup such as signalling user presence in a database.

Currently the request will either die immediately, or if ctx.waitUntil(writable.closed) was called then it will continue running unknowingly until it gets terminated anyway 30s or so later.

[kentonv]

FWIW, I agree this is the right API.

(But I don't currently know when we'll be able to implement it.)

[brettwillis]

(But I don't currently know when we'll be able to implement it.)

I'd be open to contributing this. But I wouldn't want to get started until the proposal has been through the appropriate reviews etc? Let me know.

[ricardopolo]

👍 to this feature, cloudflare should support abort requests

in the meantime, anyone have any workaround in mind?

[kentonv]

@mikenomitch @irvinebroque thoughts?

[irvinebroque]

We want to support Request.signal, and agree that registering the event listener on the incoming request makes sense. Matter of when not if in my mind (particularly now that the cache property of Request has some support in Workers cloudflare/cloudflare-docs#17926)

Defer to others on if we think this is a viable thing for someone externally to contribute but in principle really appreciate that you're interested in contributing @brettwillis.

@jasnell FYI re: standards

[jasnell]

To be clear, we already support request.signal on the client side of a fetch, just not on the server side. We've had folks request the server-side for a while but it needs to come with the caveat that we likely cannot guarantee that it will fire in every case. Detecting that the connection has been aborted reliably can be a bit tricky but definitely doable.

One key question to determine is under what conditions would the request.signal be triggered on the server-side? Only when the runtime detects that the client has disconnected? Should it be called under other circumstances such as if the runtime cancels the request due to an error?

And just to make sure we're on the same page, this is what we're talking about, correct?

export default {
  fetch(request) {
    request.signal.addEventListener('abort', () => {
      // If this is called, it means the incoming request was canceled and we should stop trying to do stuff for it.
    });
  }
}

[brettwillis]

And just to make sure we're on the same page, this is what we're talking about, correct?

Yes correct. I'd add that the work in that callback may be async, and therefore involve ctx.waitUntil():

export default {
  fetch(request, env, ctx) {
    request.signal.addEventListener('abort', () => {
      // Do some logging or async work to signal other parts of the system
      const promise = ...;
      ctx.waitUntil(promise);
    });

    // Response may be a long-running readable stream
    const readable = ...;
    return readable;
  }
}

One key question to determine is under what conditions would the request.signal be triggered on the server-side? Only when the runtime detects that the client has disconnected? Should it be called under other circumstances such as if the runtime cancels the request due to an error?

Good point, needs some careful consideration. Here's a train of thought from my side:

Is it correct to say that there are currently two ways for a request to end: (1) end "naturally" by emptying the event loop, or (2) "aborted" by the runtime by cancelling any pending callbacks in the event loop.

We could then say that request.signal is aborted whenever the runtime itself aborts a request as in (2) without it ending naturally? Worded in another way: "abort callbacks are guaranteed to be called when the runtime [knowingly] aborts a request before it naturally ends".

While this sounds clean, it may break down a little when reasoning about (3) and (4) below...

Conditions I can think of where the runtime currently aborts a request (cancelling event loop callbacks) are:

1. Platform detects a client disconnect (we see this as "Request cancelled" error in the logs). I think you are saying that the platform does not always reliably detect this, but when it does detect this the runtime does abort the request.
2. Internal reasons like server maintenance.
3. Timeout limits like callbacks in ctx.waitUntil() that take too long to resolve.
4. Resource limits like CPU or memory.

In the cases of (3) and (4), the script has already reached it's limits and therefore you might not want to give it more life with the abort callbacks?

There's also another case: I think "dangling" timers and async callbacks are cancelled from the event loop if they were not wrapped in ctx.waitUntil() once the response has been sent or an unhandled exception is thrown. In such cases the script has already yielded naturally under it's own control flow. It is fully capable of detecting it's own natural end or catching the exception, so I'd it does not need a signal from the runtime. We want the abort callback for when the runtime aborts the script outside of the script's control.