Api route req on close event not triggered when request is cancelled

[jbojcic1]

Verify canary release

• I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: linux
      Arch: x64
      Version: #22 SMP Tue Jan 10 18:39:00 UTC 2023
    Binaries:
      Node: 18.14.2
      npm: 9.5.0
      Yarn: 1.22.19
      pnpm: N/A
    Relevant Packages:
      next: 13.4.10-canary.8
      eslint-config-next: 13.4.8
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.0.4
    Next.js Config:
      output: N/A

Which area(s) of Next.js are affected? (leave empty if unsure)

Middleware / Edge (API routes, runtime)

Link to the code that reproduces this issue or a replay of the bug

https://codesandbox.io/p/sandbox/blissful-driscoll-y78mrd

To Reproduce

1. open the link from above
2. click on "Trigger API call" button
3. click on "Cancel API call" button and note in browser network tab that the request which was previously pending is now cancelled
4. note how req.on("close" event handler didn't run and API route work is still being done

Describe the Bug

Request close event is not firing in the API route when the caller cancels the request

Expected Behavior

Request close event should be fired so that any necessary cleanup can be done

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

[masterkain]

perhaps related #52281

[jbojcic1]

I also just noticed that req.destroyed is set to true immediately in the API route handler (see the log I just added to the same demo). Isn't that supposed to be true only if the request was forcefully closed (e.g. cancelled by the client)?

[jridgewell]

I also just noticed that req.destroyed is set to true immediately in the API route handler (see the log I just added to the same demo). Isn't that supposed to be true only if the request was forcefully closed (e.g. cancelled by the client)?

request is destroyed when the stream's data is fully read into the program, which happens automatically because Next provides you with a req.body. If the stream data isn't read, then it's also destroyed when the client disconnects.

response is only destroyed when the client disconnects, so it's the one you should use for the disconnect signal.

So, this is working as intended. req.on('close', cb) will never invoke the callback, because the request is already closed. Try using res.on('close') instead, and possibly listening to 'end' event as well if you want to distinguish the client closing the stream from you ending the response with res.end().

[jbojcic1]

request is destroyed when the stream's data is fully read into the program, which happens automatically because Next provides you with a req.body. If the stream data isn't read, then it's also destroyed when the client disconnects.

@jridgewell I see. Thx for the explanation. However, not even res.on('close') is emitting. I've updated my example to demonstrate. You can see in the network tab that hitting cancel API call button cancels the request but server logs don't show that response close event is emitted

[jbojcic1]

Also, if I disable bodyParser, the request destroyed and closed flags are false but still when the request is then canceled by the client, the request closed event is not emitted + destroyed and closed flags stay false. An example of this can be seen here

[jridgewell]

Strange, I'm not sure why the close event isn't firing.

[jridgewell]

Ok, now I see what's happening. We're using an async iterator with a break condition:

next.js/packages/next/src/server/next-server.ts

Lines 1537 to 1538 in 87763d7

	for await (const chunk of invokeRes) {
	if (originalResponse.closed) break

The loop's body only runs after data has been received (you write something in your API response), where we can check if the client has disconnected and break. The break is the thing that forwards the client disconnect to the API response. But you're not writing any data, so we're stuck waiting for something to be written, and we can never reach the break.

I'll update so that we listen for the close signal outside of the loop as well.

[jridgewell]

Confirmed, by writing to the response it will eventually get the close signal (and a few modifications to the client code, because you're immediately setRunning(false) in the makeApiCall function). I'll get this fixed in #52281

[jbojcic1]

Confirmed

not sure if I am doing something wrong, but in the linked sandbox I still don't see res closed printed after I cancel the request

[tangye1234]

When invoking the response piping in the Node server, special attention should be paid to the issue of backpressure.

I highly recommend using the node Readable.from(iterable) method to obtain the readable stream, and then using the node stream.pipe method to pipe the readable stream to the outgoing stream.

If this occurs on the edge, simply employ the approach utilized in the edge-runtime. It also takes into account the issue of back pressure by actively monitoring the drain event from the node res.

This will also provide support for user abortion since the pipe function listens for the underlying response close event and will handle any error issues appropriately.

@jridgewell