From d6271e204b65c6332c0461d90b36944e2bd4e12d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 8 Jun 2023 20:32:26 +0200 Subject: [PATCH] chore(ext/node): revert changes to ClientRequest.onSocket (#19426) Partially reverts https://github.com/denoland/deno/pull/19340 because it causes hangs in some situations. --- cli/tests/unit_node/http_test.ts | 8 ++---- ext/node/polyfills/http.ts | 45 +++++++++++++++++++++++++------- 2 files changed, 37 insertions(+), 16 deletions(-) diff --git a/cli/tests/unit_node/http_test.ts b/cli/tests/unit_node/http_test.ts index 6b02282743..8f87b1fd27 100644 --- a/cli/tests/unit_node/http_test.ts +++ b/cli/tests/unit_node/http_test.ts @@ -195,14 +195,11 @@ Deno.test("[node/http] request default protocol", async () => { // @ts-ignore IncomingMessageForClient // deno-lint-ignore no-explicit-any let clientRes: any; - // deno-lint-ignore no-explicit-any - let clientReq: any; server.listen(() => { - clientReq = http.request( + const req = http.request( // deno-lint-ignore no-explicit-any { host: "localhost", port: (server.address() as any).port }, (res) => { - assert(res.socket instanceof EventEmitter); assertEquals(res.complete, false); res.on("data", () => {}); res.on("end", () => { @@ -213,14 +210,13 @@ Deno.test("[node/http] request default protocol", async () => { promise2.resolve(); }, ); - clientReq.end(); + req.end(); }); server.on("close", () => { promise.resolve(); }); await promise; await promise2; - assert(clientReq.socket instanceof EventEmitter); assertEquals(clientRes!.complete, true); }); diff --git a/ext/node/polyfills/http.ts b/ext/node/polyfills/http.ts index 250d34e7cb..697de64149 100644 --- a/ext/node/polyfills/http.ts +++ b/ext/node/polyfills/http.ts @@ -267,9 +267,6 @@ const kError = Symbol("kError"); const kUniqueHeaders = Symbol("kUniqueHeaders"); -class FakeSocket extends EventEmitter { -} - /** ClientRequest represents the http(s) request from the client */ class ClientRequest extends OutgoingMessage { defaultProtocol = "http:"; @@ -544,7 +541,6 @@ class ClientRequest extends OutgoingMessage { this.onSocket(createConnection(optsWithoutSignal)); } }*/ - this.onSocket(new FakeSocket()); const url = this._createUrlStrFromOptions(); @@ -574,12 +570,41 @@ class ClientRequest extends OutgoingMessage { return undefined; } - // TODO(bartlomieju): handle error - onSocket(socket, _err) { - nextTick(() => { - this.socket = socket; - this.emit("socket", socket); - }); + onSocket(socket, err) { + if (this.destroyed || err) { + this.destroyed = true; + + // deno-lint-ignore no-inner-declarations + function _destroy(req, err) { + if (!req.aborted && !err) { + err = connResetException("socket hang up"); + } + if (err) { + req.emit("error", err); + } + req._closed = true; + req.emit("close"); + } + + if (socket) { + if (!err && this.agent && !socket.destroyed) { + socket.emit("free"); + } else { + finished(socket.destroy(err || this[kError]), (er) => { + if (er?.code === "ERR_STREAM_PREMATURE_CLOSE") { + er = null; + } + _destroy(this, er || err); + }); + return; + } + } + + _destroy(this, err || this[kError]); + } else { + //tickOnSocket(this, socket); + //this._flush(); + } } // deno-lint-ignore no-explicit-any