From 5a1745e1eafef618f7de36b7fd008582e6ba9905 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Fri, 2 Dec 2022 10:40:44 +0100 Subject: [PATCH] http: make `OutgoingMessage` more streamlike MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement missing getters error & closed. Add support for proper "writable" check through `isWritable` helper. We cannot fix the `OutgoingMessage.writable` property as that would break the ecosystem. PR-URL: https://github.com/nodejs/node/pull/45672 Reviewed-By: Matteo Collina Reviewed-By: Gerhard Stöbich Reviewed-By: Minwoo Jung Reviewed-By: Antoine du Hamel --- lib/_http_outgoing.js | 19 +++++ ...est-http-client-incomingmessage-destroy.js | 9 ++- test/parallel/test-http-outgoing-destroyed.js | 71 +++++++++++++------ 3 files changed, 78 insertions(+), 21 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index a5b3863abaeebb..8c80eabaec9e74 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -88,6 +88,7 @@ const kCorked = Symbol('corked'); const kUniqueHeaders = Symbol('kUniqueHeaders'); const kBytesWritten = Symbol('kBytesWritten'); const kEndCalled = Symbol('kEndCalled'); +const kErrored = Symbol('errored'); const nop = () => {}; @@ -147,10 +148,26 @@ function OutgoingMessage() { this._keepAliveTimeout = 0; this._onPendingData = nop; + + this[kErrored] = null; } ObjectSetPrototypeOf(OutgoingMessage.prototype, Stream.prototype); ObjectSetPrototypeOf(OutgoingMessage, Stream); +ObjectDefineProperty(OutgoingMessage.prototype, 'errored', { + __proto__: null, + get() { + return this[kErrored]; + }, +}); + +ObjectDefineProperty(OutgoingMessage.prototype, 'closed', { + __proto__: null, + get() { + return this._closed; + }, +}); + ObjectDefineProperty(OutgoingMessage.prototype, 'writableFinished', { __proto__: null, get() { @@ -320,6 +337,8 @@ OutgoingMessage.prototype.destroy = function destroy(error) { } this.destroyed = true; + this[kErrored] = error; + if (this.socket) { this.socket.destroy(error); } else { diff --git a/test/parallel/test-http-client-incomingmessage-destroy.js b/test/parallel/test-http-client-incomingmessage-destroy.js index a0823d37786365..64b95dc2ff273f 100644 --- a/test/parallel/test-http-client-incomingmessage-destroy.js +++ b/test/parallel/test-http-client-incomingmessage-destroy.js @@ -20,6 +20,13 @@ server.listen(0, () => { get({ port: server.address().port }, common.mustCall((res) => { - res.destroy(new Error('Destroy test')); + const err = new Error('Destroy test'); + assert.strictEqual(res.errored, null); + res.destroy(err); + assert.strictEqual(res.closed, false); + assert.strictEqual(res.errored, err); + res.on('close', () => { + assert.strictEqual(res.closed, true); + }); })); }); diff --git a/test/parallel/test-http-outgoing-destroyed.js b/test/parallel/test-http-outgoing-destroyed.js index e0199a1cd5ef23..2dd3d76fde7d40 100644 --- a/test/parallel/test-http-outgoing-destroyed.js +++ b/test/parallel/test-http-outgoing-destroyed.js @@ -1,24 +1,55 @@ 'use strict'; const common = require('../common'); const http = require('http'); +const assert = require('assert'); -const server = http.createServer(common.mustCall((req, res) => { - req.pipe(res); - res.on('error', common.mustNotCall()); - res.on('close', common.mustCall(() => { - res.end('asd'); - process.nextTick(() => { - server.close(); - }); - })); -})).listen(0, () => { - http - .request({ - port: server.address().port, - method: 'PUT' - }) - .on('response', (res) => { - res.destroy(); - }) - .write('asd'); -}); +{ + const server = http.createServer(common.mustCall((req, res) => { + assert.strictEqual(res.closed, false); + req.pipe(res); + res.on('error', common.mustNotCall()); + res.on('close', common.mustCall(() => { + assert.strictEqual(res.closed, true); + res.end('asd'); + process.nextTick(() => { + server.close(); + }); + })); + })).listen(0, () => { + http + .request({ + port: server.address().port, + method: 'PUT' + }) + .on('response', (res) => { + res.destroy(); + }) + .write('asd'); + }); +} + +{ + const server = http.createServer(common.mustCall((req, res) => { + assert.strictEqual(res.closed, false); + req.pipe(res); + res.on('error', common.mustNotCall()); + res.on('close', common.mustCall(() => { + assert.strictEqual(res.closed, true); + process.nextTick(() => { + server.close(); + }); + })); + const err = new Error('Destroy test'); + res.destroy(err); + assert.strictEqual(res.errored, err); + })).listen(0, () => { + http + .request({ + port: server.address().port, + method: 'PUT' + }) + .on('error', common.mustCall()) + .write('asd'); + }); + +}