Commit graph

4 commits

Author SHA1 Message Date
Justin Ridgewell
3906374740
Try to fix flakey socket hang up failures in stream cancel tests (#53318)
### What?

I think the flakiness from the new stream cancel tests is due to a uncaught error thrown from the priming. If it is during the priming, then we need to also explicitly check if we restart the test and reset the `streamable`.

### Why?

Flakey tests suck.

### How?

- Adds a `on('error', reject)` to catch the socket error and associate it with the test
- Explicitly checks for the `?write=` param to see if we're priming or getting results
2023-07-28 22:59:24 +00:00
Justin Ridgewell
31d2b720d9
Reimplement stream cancellation (#52281)
### What?

This reimplements our stream cancellation code for a few more cases:
1. Adds support in all stream-returning APIs
2. Fixes cancellation detection in node 16
3. Implements out-of-band detection, so can cancel in the middle of a
read

It also (finally) adds tests for all the cases I'm aware of.

### Why?

To allow disconnecting from an AI service when a client disconnects. $$$

### How?

1. Reuses a single pipe function in all paths to push data from the
dev's `ReadableStream` into our `ServerResponse`
2. Uses `ServerResponse` to detect disconnect, instead of the
`IncomingMessage` (request)
    - The `close` event fire once all incoming body data is read
- The request `abort` event will not fire after the incoming body data
has been fully read
3. Using `on('close')` on the writable destination allows us to detect
close
- Checking for `res.destroyed` in the body of the loop meant we had to
wait for the `await stream.read()` to complete before we could possibly
cancel the stream

- - -

#52157 (and #51594) had an issue with Node 16, because I was using
`res.closed` to detect when the server response was closed by the client
disconnecting. But, `closed` wasn't
[added](https://github.com/nodejs/node/pull/45672) until
[v18.13.0](https://nodejs.org/en/blog/release/v18.13.0#:~:text=%5Bcbd710bbf4%5D%20%2D%20http%3A%20make%20OutgoingMessage%20more%20streamlike%20(Robert%20Nagy)%20%2345672).
This fixes it by using `res.destroyed`.

Reverts #52277
Relands #52157
Fixes #52809

---------
2023-07-26 12:57:34 -07:00
JJ Kasper
5046ee13aa
Revert "Fix stream cancellation in RenderResult.pipe() and sendResponse()" (#52277)
The test here seems to be failing after merge 

x-ref:
https://github.com/vercel/next.js/actions/runs/5467319221/jobs/9953529745
x-ref:
https://github.com/vercel/next.js/actions/runs/5467152519/jobs/9953132439

Reverts vercel/next.js#52157
2023-07-05 11:12:39 -07:00
Justin Ridgewell
ff1f75a873
Fix stream cancellation in RenderResult.pipe() and sendResponse() (#52157)
### What?

I've found 2 more spots that didn't properly cancel the streaming response when the client disconnects. This fixes `RenderResult.pipe()` (called during dynamic render results) and `sendResponse()` (used during Route Handlers using `nodejs` runtime).

It also (finally) adds tests for all the cases I'm aware of.

### Why?

To allow disconnecting from an AI service when a client disconnects. $$$

### How?

Just checks for `response.closed`, which will be closed when the client's connection disconnects.
2023-07-05 17:15:48 +00:00