-
-
Notifications
You must be signed in to change notification settings - Fork 732
Description
Bug Description
We're hitting assert(!this.completed) in Request.onData (request.js:265) in production with HTTP/2
enabled. The process crashes because onData() is called after onComplete() has already set this.completed = true.
Looking at client-h2.js, the data handler (line 705) calls request.onData(chunk) without
checking request.completed or request.aborted. The end handler (line 711) removes data
listeners before calling onComplete, but the [trailers handler (line 765)](https://github.com/nodejs/undici/blob/v7.21.0/lib/dispatcher/client-h2.js#L765-
L770) calls request.onComplete(trailers) without removing data listeners first.
Reproducible By
I couldn't reproduce it on localhost, but the code below triggers the assertion error if any data arrives after onComplete has been called.
const Request = require("undici/lib/core/request");
const request = new Request("http://localhost", {
path: "/", method: "GET", headers: {},
}, {
onConnect() {}, onHeaders() { return true; },
onData() { return true; }, onComplete() {}, onError() {},
});
request.onConnect(() => {});
request.onHeaders(200, [], () => {}, "OK");
request.onData(Buffer.from("data"));
request.onComplete({});
// This simulates what happens when a late H2 DATA frame
// arrives after END_STREAM has been processed
request.onData(Buffer.from("late"));
AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:
assert(!this.completed)
at Request.onData (/node_modules/undici/lib/core/request.js:265:5)
Our config:
new Agent({
keepAliveTimeout: 20_000,
keepAliveMaxTimeout: 60_000,
bodyTimeout: 10_000,
headersTimeout: 10_000,
connections: 500,
pipelining: 100,
allowH2: true,
maxConcurrentStreams: 100,
}).compose(interceptors.responseError())
GET requests against remote HTTPS servers. The crash happened at ~6 req/s.
Expected Behavior
Late data events after stream completion should not crash the process.
Logs & Screenshots
AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:
assert(!this.completed)
at Request.onData (/opt/sye/node_modules/undici/lib/core/request.js:265:5)
at ClientHttp2Stream.<anonymous> (/opt/sye/node_modules/undici/lib/dispatcher/client-h2.js:706:17)
at ClientHttp2Stream.emit (node:events:513:28)
at addChunk (node:internal/streams/readable:324:12)
at readableAddChunk (node:internal/streams/readable:297:9)
at Readable.push (node:internal/streams/readable:234:10)
at Http2Stream.onStreamRead (node:internal/stream_base_commons:190:23)
Environment
- undici 7.21.0
- Node.js v20.19.0
- Linux (production)