Skip to content

Commit 2cd9689

Browse files
committed
fix(@angular/ssr): settle writeResponseToNodeResponse when client disconnects
When a client disconnects while a response is backpressured or streaming, the Node response (`ServerResponse` or `Http2ServerResponse`) is closed or destroyed without emitting a `drain` event. Previously, this caused `writeResponseToNodeResponse()` to park indefinitely waiting for `drain` and never settle. This change monitors whether the Node response is closed or destroyed and removes event listeners, cancels the reader, and resolves the returned Promise when a client disconnect occurs. Closes #33719 TAG=agy CONV=12c9fd92-1bdb-4e84-8c35-a43b5b69e75d
1 parent 0ae5690 commit 2cd9689

3 files changed

Lines changed: 176 additions & 14 deletions

File tree

packages/angular/ssr/node/src/response.ts

Lines changed: 88 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,24 @@
99
import type { ServerResponse } from 'node:http';
1010
import type { Http2ServerResponse } from 'node:http2';
1111

12+
/**
13+
* Checks whether a Node.js `ServerResponse` or `Http2ServerResponse` is destroyed, closed, or ended.
14+
*
15+
* @param destination - The HTTP/1.1 or HTTP/2 server response to check.
16+
* @returns `true` if the response or its underlying stream is destroyed, closed, or ended; otherwise `false`.
17+
*/
18+
function isResponseDestroyedOrClosed(destination: ServerResponse | Http2ServerResponse): boolean {
19+
return (
20+
Boolean(destination.destroyed) ||
21+
Boolean(destination.closed) ||
22+
Boolean(destination.writableEnded) ||
23+
('stream' in destination &&
24+
(!destination.stream ||
25+
Boolean(destination.stream.destroyed) ||
26+
Boolean(destination.stream.closed)))
27+
);
28+
}
29+
1230
/**
1331
* Streams a web-standard `Response` into a Node.js `ServerResponse`
1432
* or `Http2ServerResponse`.
@@ -24,6 +42,10 @@ export async function writeResponseToNodeResponse(
2442
source: Response,
2543
destination: ServerResponse | Http2ServerResponse,
2644
): Promise<void> {
45+
if (isResponseDestroyedOrClosed(destination)) {
46+
return;
47+
}
48+
2749
const { status, headers, body } = source;
2850
destination.statusCode = status;
2951

@@ -48,27 +70,50 @@ export async function writeResponseToNodeResponse(
4870
}
4971

5072
if (!body) {
51-
destination.end();
73+
if (!isResponseDestroyedOrClosed(destination)) {
74+
destination.end();
75+
}
5276

5377
return;
5478
}
5579

56-
try {
57-
const reader = body.getReader();
58-
59-
destination.on('close', () => {
60-
reader.cancel().catch((error) => {
61-
// eslint-disable-next-line no-console
62-
console.error(
63-
`An error occurred while writing the response body for: ${destination.req.url}.`,
64-
error,
65-
);
66-
});
80+
let isClosed = isResponseDestroyedOrClosed(destination);
81+
const isDestroyedOrClosed = () => isClosed || isResponseDestroyedOrClosed(destination);
82+
83+
let readerCancelled = false;
84+
const reader = body.getReader();
85+
const cancelReader = (error?: unknown) => {
86+
if (readerCancelled) {
87+
return;
88+
}
89+
readerCancelled = true;
90+
isClosed = true;
91+
reader.cancel(error).catch((err) => {
92+
// eslint-disable-next-line no-console
93+
console.error(
94+
`An error occurred while writing the response body for: ${destination.req.url}.`,
95+
err,
96+
);
6797
});
98+
};
99+
100+
destination.once('close', cancelReader);
101+
destination.once('error', cancelReader);
68102

103+
try {
69104
// eslint-disable-next-line no-constant-condition
70105
while (true) {
106+
if (isDestroyedOrClosed()) {
107+
cancelReader();
108+
break;
109+
}
110+
71111
const { done, value } = await reader.read();
112+
if (isDestroyedOrClosed()) {
113+
cancelReader();
114+
break;
115+
}
116+
72117
if (done) {
73118
destination.end();
74119
break;
@@ -78,10 +123,39 @@ export async function writeResponseToNodeResponse(
78123
if (canContinue === false) {
79124
// Explicitly check for `false`, as AWS may return `undefined` even though this is not valid.
80125
// See: https://github.com/CodeGenieApp/serverless-express/issues/683
81-
await new Promise<void>((resolve) => destination.once('drain', resolve));
126+
await new Promise<void>((resolve) => {
127+
if (isDestroyedOrClosed()) {
128+
resolve();
129+
130+
return;
131+
}
132+
133+
const onDrain = () => {
134+
destination.off('close', onClose);
135+
destination.off('error', onClose);
136+
resolve();
137+
};
138+
139+
const onClose = () => {
140+
destination.off('drain', onDrain);
141+
destination.off('close', onClose);
142+
destination.off('error', onClose);
143+
cancelReader();
144+
resolve();
145+
};
146+
147+
destination.once('drain', onDrain);
148+
destination.once('close', onClose);
149+
destination.once('error', onClose);
150+
});
82151
}
83152
}
84153
} catch {
85-
destination.end('Internal server error.');
154+
if (!isDestroyedOrClosed()) {
155+
destination.end('Internal server error.');
156+
}
157+
} finally {
158+
destination.off('close', cancelReader);
159+
destination.off('error', cancelReader);
86160
}
87161
}

packages/angular/ssr/node/test/response_http1_spec.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,4 +107,48 @@ describe('writeResponseToNodeResponse (HTTP/1.1)', () => {
107107

108108
expect(response.headers['set-cookie']).toEqual(cookieValue);
109109
});
110+
111+
it('should resolve and cancel reader when client disconnects while response is backpressured', async () => {
112+
let writePromise!: Promise<void>;
113+
let readerCancelled = false;
114+
const largeChunk = 'x'.repeat(1024 * 1024 * 4); // 4MB to ensure backpressure
115+
const stream = new ReadableStream({
116+
start(controller) {
117+
controller.enqueue(largeChunk);
118+
controller.enqueue(largeChunk);
119+
},
120+
cancel() {
121+
readerCancelled = true;
122+
},
123+
});
124+
125+
server.once('request', (_, nodeResponse) => {
126+
writePromise = writeResponseToNodeResponse(new Response(stream), nodeResponse);
127+
});
128+
129+
await new Promise<void>((resolve) => {
130+
const { port } = server.address() as AddressInfo;
131+
const clientRequest = requestCb(
132+
{
133+
host: 'localhost',
134+
port,
135+
},
136+
(response) => {
137+
response.once('data', () => {
138+
clientRequest.destroy();
139+
resolve();
140+
});
141+
},
142+
);
143+
144+
clientRequest.on('error', () => {
145+
// Expected when destroying the socket
146+
});
147+
148+
clientRequest.end();
149+
});
150+
151+
await writePromise;
152+
expect(readerCancelled).toBeTrue();
153+
});
110154
});

packages/angular/ssr/node/test/response_http2_spec.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,4 +116,48 @@ describe('writeResponseToNodeResponse (HTTP/2)', () => {
116116

117117
expect(resHeaders['set-cookie']).toEqual(cookieValue);
118118
});
119+
120+
it('should resolve and cancel reader when client disconnects while response is backpressured', async () => {
121+
let writePromise!: Promise<void>;
122+
let readerCancelled = false;
123+
const largeChunk = 'x'.repeat(1024 * 1024); // 1MB to exceed HTTP/2 flow control window
124+
const stream = new ReadableStream({
125+
start(controller) {
126+
controller.enqueue(largeChunk);
127+
controller.enqueue(largeChunk);
128+
},
129+
cancel() {
130+
readerCancelled = true;
131+
},
132+
});
133+
134+
server.once('request', (_, nodeResponse) => {
135+
writePromise = writeResponseToNodeResponse(new Response(stream), nodeResponse);
136+
});
137+
138+
await new Promise<void>((resolve) => {
139+
const { port } = server.address() as AddressInfo;
140+
const client = connect(`http://localhost:${port}`);
141+
const req = client.request({
142+
':path': '/',
143+
});
144+
145+
req.once('response', () => {
146+
req.once('data', () => {
147+
req.destroy();
148+
client.destroy();
149+
resolve();
150+
});
151+
});
152+
153+
req.on('error', () => {
154+
// Expected when destroying/closing the stream
155+
});
156+
157+
req.end();
158+
});
159+
160+
await writePromise;
161+
expect(readerCancelled).toBeTrue();
162+
});
119163
});

0 commit comments

Comments
 (0)