From ff6535a4331d611bf0d205ee23d88e100dce92b2 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sat, 11 Apr 2020 16:06:43 -0400 Subject: [PATCH 1/3] http: return this from IncomingMessage#destroy() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit updates IncomingMessage#destroy() to return `this` for consistency with other readable streams. PR-URL: https://github.com/nodejs/node/pull/32789 Fixes: https://github.com/nodejs/node/issues/32772 Reviewed-By: Robert Nagy Reviewed-By: Anna Henningsen Reviewed-By: Michaël Zasso Reviewed-By: Ruben Bridgewater --- doc/api/http.md | 6 ++++++ lib/_http_incoming.js | 1 + test/parallel/test-http-incoming-message-destroy.js | 10 ++++++++++ 3 files changed, 17 insertions(+) create mode 100644 test/parallel/test-http-incoming-message-destroy.js diff --git a/doc/api/http.md b/doc/api/http.md index 1ac10261b332a3..364978855eee3f 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -1847,9 +1847,15 @@ const req = http.request({ ### `message.destroy([error])` * `error` {Error} +* Returns: {this} Calls `destroy()` on the socket that received the `IncomingMessage`. If `error` is provided, an `'error'` event is emitted on the socket and `error` is passed diff --git a/lib/_http_incoming.js b/lib/_http_incoming.js index 60081279a05e42..bccd79f8c94c32 100644 --- a/lib/_http_incoming.js +++ b/lib/_http_incoming.js @@ -123,6 +123,7 @@ IncomingMessage.prototype.destroy = function destroy(error) { this.destroyed = true; if (this.socket) this.socket.destroy(error); + return this; }; diff --git a/test/parallel/test-http-incoming-message-destroy.js b/test/parallel/test-http-incoming-message-destroy.js new file mode 100644 index 00000000000000..4241ec8e7d85ef --- /dev/null +++ b/test/parallel/test-http-incoming-message-destroy.js @@ -0,0 +1,10 @@ +'use strict'; + +// Test that http.IncomingMessage,prototype.destroy() returns `this`. +require('../common'); + +const assert = require('assert'); +const http = require('http'); +const incomingMessage = new http.IncomingMessage(); + +assert.strictEqual(incomingMessage.destroy(), incomingMessage); From cad76da198ff59e16d3439cbdb5793750c01f382 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Fri, 8 May 2020 21:25:55 -0400 Subject: [PATCH 2/3] http: return this from ClientRequest#destroy() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit updates ClientRequest#destroy() to return `this` for consistency with other writable streams. PR-URL: https://github.com/nodejs/node/pull/32789 Reviewed-By: Robert Nagy Reviewed-By: Anna Henningsen Reviewed-By: Michaël Zasso Reviewed-By: Ruben Bridgewater --- doc/api/http.md | 5 +++++ lib/_http_client.js | 4 +++- test/parallel/test-client-request-destroy.js | 13 +++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-client-request-destroy.js diff --git a/doc/api/http.md b/doc/api/http.md index 364978855eee3f..a6f73ee94f9cfd 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -631,6 +631,11 @@ is finished. ### `request.destroy([error])` * `error` {Error} Optional, an error to emit with `'error'` event. diff --git a/lib/_http_client.js b/lib/_http_client.js index 22b695e6064809..7b00417e8a253a 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -349,7 +349,7 @@ ClientRequest.prototype.abort = function abort() { ClientRequest.prototype.destroy = function destroy(err) { if (this.destroyed) { - return; + return this; } this.destroyed = true; @@ -365,6 +365,8 @@ ClientRequest.prototype.destroy = function destroy(err) { } else if (err) { this[kError] = err; } + + return this; }; function _destroy(req, socket, err) { diff --git a/test/parallel/test-client-request-destroy.js b/test/parallel/test-client-request-destroy.js new file mode 100644 index 00000000000000..2f3efcf81204b3 --- /dev/null +++ b/test/parallel/test-client-request-destroy.js @@ -0,0 +1,13 @@ +'use strict'; + +// Test that http.ClientRequest,prototype.destroy() returns `this`. +require('../common'); + +const assert = require('assert'); +const http = require('http'); +const clientRequest = new http.ClientRequest({ createConnection: () => {} }); + +assert.strictEqual(clientRequest.destroyed, false); +assert.strictEqual(clientRequest.destroy(), clientRequest); +assert.strictEqual(clientRequest.destroyed, true); +assert.strictEqual(clientRequest.destroy(), clientRequest); From 94e5b5c77dade0d8f7358c66144b75c369679cab Mon Sep 17 00:00:00 2001 From: cjihrig Date: Fri, 8 May 2020 21:44:13 -0400 Subject: [PATCH 3/3] http: return this from OutgoingMessage#destroy() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit updates OutgoingMessage#destroy() to return `this` for consistency with other writable streams. PR-URL: https://github.com/nodejs/node/pull/32789 Reviewed-By: Robert Nagy Reviewed-By: Anna Henningsen Reviewed-By: Michaël Zasso Reviewed-By: Ruben Bridgewater --- lib/_http_outgoing.js | 4 +++- test/parallel/test-outgoing-message-destroy.js | 13 +++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-outgoing-message-destroy.js diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 33520610bc0eb2..2e639535ee4a9e 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -283,7 +283,7 @@ OutgoingMessage.prototype.setTimeout = function setTimeout(msecs, callback) { // it, since something else is destroying this connection anyway. OutgoingMessage.prototype.destroy = function destroy(error) { if (this.destroyed) { - return; + return this; } this.destroyed = true; @@ -294,6 +294,8 @@ OutgoingMessage.prototype.destroy = function destroy(error) { socket.destroy(error); }); } + + return this; }; diff --git a/test/parallel/test-outgoing-message-destroy.js b/test/parallel/test-outgoing-message-destroy.js new file mode 100644 index 00000000000000..0ee7b5f40ef9fa --- /dev/null +++ b/test/parallel/test-outgoing-message-destroy.js @@ -0,0 +1,13 @@ +'use strict'; + +// Test that http.OutgoingMessage,prototype.destroy() returns `this`. +require('../common'); + +const assert = require('assert'); +const http = require('http'); +const outgoingMessage = new http.OutgoingMessage(); + +assert.strictEqual(outgoingMessage.destroyed, false); +assert.strictEqual(outgoingMessage.destroy(), outgoingMessage); +assert.strictEqual(outgoingMessage.destroyed, true); +assert.strictEqual(outgoingMessage.destroy(), outgoingMessage);