From 514d447f6d5248781fcc20a3afbdcbc32cd59100 Mon Sep 17 00:00:00 2001 From: Kai Tamkun Date: Tue, 11 Mar 2025 14:57:32 -0700 Subject: [PATCH 1/2] Initialize ServerResponse.finished to false --- src/js/node/http.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/js/node/http.ts b/src/js/node/http.ts index b3a6a6b16af321..92493eaed4ecb4 100644 --- a/src/js/node/http.ts +++ b/src/js/node/http.ts @@ -39,7 +39,7 @@ const runSymbol = Symbol("run"); const deferredSymbol = Symbol("deferred"); const eofInProgress = Symbol("eofInProgress"); const fakeSocketSymbol = Symbol("fakeSocket"); -const finishedSymbol = "finished"; +const finishedSymbol = Symbol("finished"); const firstWriteSymbol = Symbol("firstWrite"); const headersSymbol = Symbol("headers"); const isTlsSymbol = Symbol("is_tls"); @@ -1850,6 +1850,7 @@ function ServerResponse(req, options) { this._sent100 = false; this[headerStateSymbol] = NodeHTTPHeaderState.none; this[kPendingCallbacks] = []; + this[finishedSymbol] = false; // this is matching node's behaviour // https://github.com/nodejs/node/blob/cf8c6994e0f764af02da4fa70bc5962142181bf3/lib/_http_server.js#L192 @@ -1886,6 +1887,13 @@ const ServerResponsePrototype = { this[headerStateSymbol] = value ? NodeHTTPHeaderState.sent : NodeHTTPHeaderState.none; }, + get finished() { + return this[finishedSymbol]; + }, + set finished(value) { + this[finishedSymbol] = value; + }, + // This end method is actually on the OutgoingMessage prototype in Node.js // But we don't want it for the fetch() response version. end(chunk, encoding, callback) { @@ -1936,7 +1944,7 @@ const ServerResponsePrototype = { req._dump(); } this.detachSocket(socket); - this[finishedSymbol] = this.finished = true; + this.finished = true; this.emit("prefinish"); this._callPendingCallbacks(); From 8ccdc4bc6428c3d650f223f62307ca738ef7a745 Mon Sep 17 00:00:00 2001 From: Kai Tamkun Date: Tue, 11 Mar 2025 19:51:01 -0700 Subject: [PATCH 2/2] Remove finishedSymbol and de-getterify the `finished` property --- src/js/node/http.ts | 50 +++++++++++++++++++-------------------------- 1 file changed, 21 insertions(+), 29 deletions(-) diff --git a/src/js/node/http.ts b/src/js/node/http.ts index 92493eaed4ecb4..a106e3b664ea19 100644 --- a/src/js/node/http.ts +++ b/src/js/node/http.ts @@ -39,7 +39,6 @@ const runSymbol = Symbol("run"); const deferredSymbol = Symbol("deferred"); const eofInProgress = Symbol("eofInProgress"); const fakeSocketSymbol = Symbol("fakeSocket"); -const finishedSymbol = Symbol("finished"); const firstWriteSymbol = Symbol("firstWrite"); const headersSymbol = Symbol("headers"); const isTlsSymbol = Symbol("is_tls"); @@ -677,14 +676,14 @@ Object.defineProperty(Server, "name", { value: "Server" }); function onRequestEvent(event) { const [server, http_res, req] = this.socket[kInternalSocketData]; - if (!http_res[finishedSymbol]) { + if (!http_res.finished) { switch (event) { case NodeHTTPResponseAbortEvent.timeout: this.emit("timeout"); server.emit("timeout", req.socket); break; case NodeHTTPResponseAbortEvent.abort: - http_res[finishedSymbol] = true; + http_res.finished = true; this.destroy(); break; } @@ -1542,7 +1541,7 @@ function OutgoingMessage(options) { Stream.$call(this, options); this.sendDate = true; - this[finishedSymbol] = false; + this.finished = false; this[headerStateSymbol] = NodeHTTPHeaderState.none; this[kAbortController] = null; @@ -1714,15 +1713,15 @@ const OutgoingMessagePrototype = { }, get writableNeedDrain() { - return !this.destroyed && !this[finishedSymbol] && this[kBodyChunks] && this[kBodyChunks].length > 0; + return !this.destroyed && !this.finished && this[kBodyChunks] && this[kBodyChunks].length > 0; }, get writableEnded() { - return this[finishedSymbol]; + return this.finished; }, get writableFinished() { - return this[finishedSymbol] && !!(this[kEmitState] & (1 << ClientRequestEmitState.finish)); + return this.finished && !!(this[kEmitState] & (1 << ClientRequestEmitState.finish)); }, _send(data, encoding, callback, byteLength) { @@ -1850,7 +1849,7 @@ function ServerResponse(req, options) { this._sent100 = false; this[headerStateSymbol] = NodeHTTPHeaderState.none; this[kPendingCallbacks] = []; - this[finishedSymbol] = false; + this.finished = false; // this is matching node's behaviour // https://github.com/nodejs/node/blob/cf8c6994e0f764af02da4fa70bc5962142181bf3/lib/_http_server.js#L192 @@ -1887,13 +1886,6 @@ const ServerResponsePrototype = { this[headerStateSymbol] = value ? NodeHTTPHeaderState.sent : NodeHTTPHeaderState.none; }, - get finished() { - return this[finishedSymbol]; - }, - set finished(value) { - this[finishedSymbol] = value; - }, - // This end method is actually on the OutgoingMessage prototype in Node.js // But we don't want it for the fetch() response version. end(chunk, encoding, callback) { @@ -2074,11 +2066,11 @@ const ServerResponsePrototype = { }, get writableNeedDrain() { - return !this.destroyed && !this[finishedSymbol] && (this[kHandle]?.bufferedAmount ?? 1) !== 0; + return !this.destroyed && !this.finished && (this[kHandle]?.bufferedAmount ?? 1) !== 0; }, get writableFinished() { - return !!(this[finishedSymbol] && (!this[kHandle] || this[kHandle].finished)); + return !!(this.finished && (!this[kHandle] || this[kHandle].finished)); }, get writableLength() { @@ -2227,7 +2219,7 @@ function ensureReadableStreamController(run) { for (let run of this[runSymbol]) { run(controller); } - if (!this[finishedSymbol]) { + if (!this.finished) { const { promise, resolve } = $newPromiseCapability(GlobalPromise); this[deferredSymbol] = resolve; return promise; @@ -2280,7 +2272,7 @@ function ServerResponse_finalDeprecated(chunk, encoding, callback) { chunk = Buffer.from(chunk, encoding); } const req = this.req; - const shouldEmitClose = req && req.emit && !this[finishedSymbol]; + const shouldEmitClose = req && req.emit && !this.finished; if (!this.headersSent) { let data = this[firstWriteSymbol]; if (chunk) { @@ -2300,7 +2292,7 @@ function ServerResponse_finalDeprecated(chunk, encoding, callback) { } this[firstWriteSymbol] = undefined; - this[finishedSymbol] = true; + this.finished = true; this.headersSent = true; // https://github.com/oven-sh/bun/issues/3458 drainHeadersIfObservable.$call(this); this[kDeprecatedReplySymbol]( @@ -2318,7 +2310,7 @@ function ServerResponse_finalDeprecated(chunk, encoding, callback) { return; } - this[finishedSymbol] = true; + this.finished = true; ensureReadableStreamController.$call(this, controller => { if (chunk && encoding) { chunk = Buffer.from(chunk, encoding); @@ -2464,13 +2456,13 @@ function ClientRequest(input, options, cb) { } if (chunk) { - if (this[finishedSymbol]) { + if (this.finished) { emitErrorNextTickIfErrorListenerNT(this, $ERR_STREAM_WRITE_AFTER_END("Cannot write after end"), callback); return this; } write_(chunk, encoding, null); - } else if (this[finishedSymbol]) { + } else if (this.finished) { if (callback) { if (!this.writableFinished) { this.on("finish", callback); @@ -2484,7 +2476,7 @@ function ClientRequest(input, options, cb) { this.once("finish", callback); } - if (!this[finishedSymbol]) { + if (!this.finished) { send(); resolveNextChunk?.(true); } @@ -2503,7 +2495,7 @@ function ClientRequest(input, options, cb) { res._dump(); } - this[finishedSymbol] = true; + this.finished = true; // If request is destroyed we abort the current response this[kAbortController]?.abort?.(); @@ -2614,7 +2606,7 @@ function ClientRequest(input, options, cb) { self.emit("drain"); } - while (!self[finishedSymbol]) { + while (!self.finished) { yield await new Promise(resolve => { resolveNextChunk = end => { resolveNextChunk = undefined; @@ -2725,7 +2717,7 @@ function ClientRequest(input, options, cb) { let handleResponse = () => {}; const send = () => { - this[finishedSymbol] = true; + this.finished = true; const controller = new AbortController(); this[kAbortController] = controller; controller.signal.addEventListener("abort", onAbort, { once: true }); @@ -2978,7 +2970,7 @@ function ClientRequest(input, options, cb) { // this.useChunkedEncodingByDefault = true; // } - this[finishedSymbol] = false; + this.finished = false; this[kRes] = null; this[kUpgradeOrConnect] = false; this[kParser] = null; @@ -3148,7 +3140,7 @@ const ClientRequestPrototype = { }, get writable() { - return !this[finishedSymbol]; + return !this.finished; }, };