Skip to content

Commit fa312f6

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 78a2b68 commit fa312f6

3 files changed

Lines changed: 184 additions & 16 deletions

File tree

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

Lines changed: 84 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,21 @@
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 or closed.
14+
*/
15+
function isResponseDestroyedOrClosed(destination: ServerResponse | Http2ServerResponse): boolean {
16+
return (
17+
Boolean(destination.destroyed) ||
18+
Boolean(destination.closed) ||
19+
Boolean(destination.writableEnded) ||
20+
('stream' in destination &&
21+
(!destination.stream ||
22+
Boolean(destination.stream.destroyed) ||
23+
Boolean(destination.stream.closed)))
24+
);
25+
}
26+
1227
/**
1328
* Streams a web-standard `Response` into a Node.js `ServerResponse`
1429
* or `Http2ServerResponse`.
@@ -24,6 +39,10 @@ export async function writeResponseToNodeResponse(
2439
source: Response,
2540
destination: ServerResponse | Http2ServerResponse,
2641
): Promise<void> {
42+
if (isResponseDestroyedOrClosed(destination)) {
43+
return;
44+
}
45+
2746
const { status, headers, body } = source;
2847
destination.statusCode = status;
2948

@@ -48,40 +67,89 @@ export async function writeResponseToNodeResponse(
4867
}
4968

5069
if (!body) {
51-
destination.end();
70+
if (!isResponseDestroyedOrClosed(destination)) {
71+
destination.end();
72+
}
5273

5374
return;
5475
}
5576

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-
});
77+
let isClosed = isResponseDestroyedOrClosed(destination);
78+
const isDestroyedOrClosed = () => isClosed || isResponseDestroyedOrClosed(destination);
79+
80+
let readerCancelled = false;
81+
const reader = body.getReader();
82+
const cancelReader = (error?: unknown) => {
83+
if (readerCancelled) {
84+
return;
85+
}
86+
readerCancelled = true;
87+
isClosed = true;
88+
reader.cancel(error).catch((err) => {
89+
// eslint-disable-next-line no-console
90+
console.error(
91+
`An error occurred while writing the response body for: ${destination.req.url}.`,
92+
err,
93+
);
6794
});
95+
};
96+
97+
destination.once('close', cancelReader);
98+
destination.once('error', cancelReader);
6899

100+
try {
69101
// eslint-disable-next-line no-constant-condition
70102
while (true) {
103+
if (isDestroyedOrClosed()) {
104+
cancelReader();
105+
break;
106+
}
107+
71108
const { done, value } = await reader.read();
72-
if (done) {
73-
destination.end();
109+
if (done || isDestroyedOrClosed()) {
110+
if (!isDestroyedOrClosed()) {
111+
destination.end();
112+
} else {
113+
cancelReader();
114+
}
74115
break;
75116
}
76117

77118
const canContinue = (destination as ServerResponse).write(value);
78119
if (canContinue === false) {
79120
// Explicitly check for `false`, as AWS may return `undefined` even though this is not valid.
80121
// See: https://github.com/CodeGenieApp/serverless-express/issues/683
81-
await new Promise<void>((resolve) => destination.once('drain', resolve));
122+
await new Promise<void>((resolve) => {
123+
if (isDestroyedOrClosed()) {
124+
resolve();
125+
return;
126+
}
127+
128+
const onDrain = () => {
129+
destination.off('close', onClose);
130+
destination.off('error', onClose);
131+
resolve();
132+
};
133+
134+
const onClose = () => {
135+
destination.off('drain', onDrain);
136+
destination.off('error', onClose);
137+
cancelReader();
138+
resolve();
139+
};
140+
141+
destination.once('drain', onDrain);
142+
destination.once('close', onClose);
143+
destination.once('error', onClose);
144+
});
82145
}
83146
}
84147
} catch {
85-
destination.end('Internal server error.');
148+
if (!isDestroyedOrClosed()) {
149+
destination.end('Internal server error.');
150+
}
151+
} finally {
152+
destination.off('close', cancelReader);
153+
destination.off('error', cancelReader);
86154
}
87155
}

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,4 +107,54 @@ 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 resolveWritePromise: () => void;
113+
const writePromise = new Promise<void>((resolve) => {
114+
resolveWritePromise = resolve;
115+
});
116+
117+
let readerCancelled = false;
118+
const largeChunk = 'x'.repeat(1024 * 1024 * 4); // 4MB to ensure backpressure
119+
const stream = new ReadableStream({
120+
start(controller) {
121+
controller.enqueue(largeChunk);
122+
controller.enqueue(largeChunk);
123+
},
124+
cancel() {
125+
readerCancelled = true;
126+
},
127+
});
128+
129+
server.once('request', (req, nodeResponse) => {
130+
writeResponseToNodeResponse(new Response(stream), nodeResponse).finally(() => {
131+
resolveWritePromise();
132+
});
133+
});
134+
135+
await new Promise<void>((resolve) => {
136+
const { port } = server.address() as AddressInfo;
137+
const clientRequest = requestCb(
138+
{
139+
host: 'localhost',
140+
port,
141+
},
142+
(response) => {
143+
response.once('data', () => {
144+
clientRequest.destroy();
145+
resolve();
146+
});
147+
},
148+
);
149+
150+
clientRequest.on('error', () => {
151+
// Expected when destroying the socket
152+
});
153+
154+
clientRequest.end();
155+
});
156+
157+
await writePromise;
158+
expect(readerCancelled).toBeTrue();
159+
});
110160
});

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,4 +116,54 @@ 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 resolveWritePromise: () => void;
122+
const writePromise = new Promise<void>((resolve) => {
123+
resolveWritePromise = resolve;
124+
});
125+
126+
let readerCancelled = false;
127+
const largeChunk = 'x'.repeat(1024 * 1024); // 1MB to exceed HTTP/2 flow control window
128+
const stream = new ReadableStream({
129+
start(controller) {
130+
controller.enqueue(largeChunk);
131+
controller.enqueue(largeChunk);
132+
},
133+
cancel() {
134+
readerCancelled = true;
135+
},
136+
});
137+
138+
server.once('request', (req, nodeResponse) => {
139+
writeResponseToNodeResponse(new Response(stream), nodeResponse).finally(() => {
140+
resolveWritePromise();
141+
});
142+
});
143+
144+
await new Promise<void>((resolve) => {
145+
const { port } = server.address() as AddressInfo;
146+
const client = connect(`http://localhost:${port}`);
147+
const req = client.request({
148+
':path': '/',
149+
});
150+
151+
req.once('response', () => {
152+
req.once('data', () => {
153+
req.destroy();
154+
client.destroy();
155+
resolve();
156+
});
157+
});
158+
159+
req.on('error', () => {
160+
// Expected when destroying/closing the stream
161+
});
162+
163+
req.end();
164+
});
165+
166+
await writePromise;
167+
expect(readerCancelled).toBeTrue();
168+
});
119169
});

0 commit comments

Comments
 (0)