fix: Add automatic response cleanup via AbortSignal

[sommeeeer]

To make request.signal.onabort work in route handlers. Currently only works for cloudflare-node, node and express-dev. It will close the OpenNextNodeResponse we pass into NextServer's request handler when the original response in the wrapper is closed. This will ensure that the AbortSignal associated with request.signal is properly triggered when a client disconnects during a streaming response.

I did try to get it to work with awsLambda.streamifyResponse. It looks like we get no event emitted on responseStream when a client disconnects. I even tried with a barebone Lambda like this:

exports.handler = awslambda.streamifyResponse(
    async (event, responseStream, context) => {
        responseStream.setContentType('text/event-stream')
        responseStream.on("close", ()=> {
          // this never gets emitted at any point of time
          console.log("responseStream closed");
        });
        for (let i = 0; i < 20; i++) {
            await sleep(1000);
            responseStream.write(`event: ping\n`);
            responseStream.write(`data: ${i}\n\n`);
        }
        responseStream.end();
    }
);

[changeset-bot]

🦋 Changeset detected

Latest commit: 7d0cb12

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@opennextjs/aws	Patch
app-pages-router	Patch
app-router	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pkg-pr-new]

Open in StackBlitz

pnpm add https://pkg.pr.new/@opennextjs/aws@952

commit: 7d0cb12

[conico974]

This and the other one in cloudflare for the same thing makes me think that we should use a single interface in aws.

What make the most sense IMO is probably to add an AbortSignal in the StreamCreator. When it aborts, we just destroy the stream.
It's basically what you did, but using AbortSignal instead of our own implementation

[sommeeeer]

It's basically what you did, but using AbortSignal instead of our own implementation

Thanks for the review Nico! This was an excellent idea.

I am now using an AbortSignal instead of a callback. However, to get this to work in cloudflare I had to add the initial request.signal you retrieve from fetch to the ALS store. The request.signal you retrieve in the handler here will not be === to the one you retrieve in your workers fetch. It doesn't trigger any event when the client disconnects/aborts either. Im not sure why thats the case, but im gonna look into it later. For now this PR would require us to put the signal in opennextjs-cloudflare here

Update: Instead we pass the original AbortSignal you retrieve in cloudflare to the wrapper.

[sommeeeer]

Thanks for the review Kheuzy!

[conico974]

LGTM Thanks!!!