From 8fc08752e5a89f18a4e10cfdff59357ba7e08635 Mon Sep 17 00:00:00 2001 From: Meghan Denny Date: Tue, 5 Mar 2024 23:01:32 -0800 Subject: [PATCH] node:http: emit 'socket' event after calling http.request() (#9270) * node:http: emit 'socket' event after calling http.request() * add reference links for why this is how it is * add guards to not waste time * add a regression test * use test harness port number --- src/js/node/http.ts | 15 +++++++++++++-- test/js/node/http/node-http.test.ts | 23 ++++++++++++++++++++--- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/js/node/http.ts b/src/js/node/http.ts index 505ed155bf..d1f19bed61 100644 --- a/src/js/node/http.ts +++ b/src/js/node/http.ts @@ -1657,7 +1657,7 @@ class ClientRequest extends OutgoingMessage { this.#host = host; this.#protocol = protocol; - var timeout = options.timeout; + const timeout = options.timeout; if (timeout !== undefined && timeout !== 0) { this.setTimeout(timeout, undefined); } @@ -1716,8 +1716,19 @@ class ClientRequest extends OutgoingMessage { // this[kUniqueHeaders] = parseUniqueHeadersOption(options.uniqueHeaders); - var { signal: _signal, ...optsWithoutSignal } = options; + const { signal: _signal, ...optsWithoutSignal } = options; this.#options = optsWithoutSignal; + + // needs to run on the next tick so that consumer has time to register the event handler + // Ref: https://github.com/nodejs/node/blob/f63e8b7fa7a4b5e041ddec67307609ec8837154f/lib/_http_client.js#L353 + // Ref: https://github.com/nodejs/node/blob/f63e8b7fa7a4b5e041ddec67307609ec8837154f/lib/_http_client.js#L865 + process.nextTick(() => { + // this will be FakeSocket since we use fetch() atm under the hood. + // Ref: https://github.com/nodejs/node/blob/f63e8b7fa7a4b5e041ddec67307609ec8837154f/lib/_http_client.js#L803-L839 + if (this.destroyed) return; + if (this.listenerCount("socket") === 0) return; + this.emit("socket", this.socket); + }); } setSocketKeepAlive(enable = true, initialDelay = 0) { diff --git a/test/js/node/http/node-http.test.ts b/test/js/node/http/node-http.test.ts index bffdecad85..f4199cd206 100644 --- a/test/js/node/http/node-http.test.ts +++ b/test/js/node/http/node-http.test.ts @@ -175,8 +175,8 @@ describe("node:http", () => { describe("request", () => { function runTest(done: Function, callback: (server: Server, port: number, done: (err?: Error) => void) => void) { - var timer; - var server = createServer((req, res) => { + let timer; + const server = createServer((req, res) => { if (req.headers.__proto__ !== {}.__proto__) { throw new Error("Headers should inherit from Object.prototype"); } @@ -663,6 +663,7 @@ describe("node:http", () => { req.end(); }); }); + it("reassign writeHead method, issue#3585", done => { runTest(done, (server, serverPort, done) => { const req = request(`http://localhost:${serverPort}/customWriteHead`, res => { @@ -673,6 +674,7 @@ describe("node:http", () => { req.end(); }); }); + it("uploading file by 'formdata/multipart', issue#3116", done => { runTest(done, (server, serverPort, done) => { const boundary = "----FormBoundary" + Date.now(); @@ -716,6 +718,7 @@ describe("node:http", () => { req.end(); }); }); + it("request via http proxy, issue#4295", done => { const proxyServer = createServer(function (req, res) { let option = url.parse(req.url); @@ -792,10 +795,24 @@ describe("node:http", () => { req.end(); }); }); + + it("should emit a socket event when connecting", async done => { + runTest(done, async (server, serverPort, done) => { + const req = request(`http://localhost:${serverPort}`, {}); + await new Promise((resolve, reject) => { + req.on("error", reject); + req.on("socket", function onRequestSocket(socket) { + req.destroy(); + done(); + resolve(); + }); + }); + }); + }); }); describe("signal", () => { - it.skip("should abort and close the server", done => { + it("should abort and close the server", done => { const server = createServer((req, res) => { res.writeHead(200, { "Content-Type": "text/plain" }); res.end("Hello World");