From 55c361921912aad2cb30d3f1049cf9ed27a41e47 Mon Sep 17 00:00:00 2001 From: Anton Paras Date: Sat, 23 Dec 2017 01:53:17 -0800 Subject: [PATCH 1/3] lib: migrate _http_outgoing.js's remaining errors A couple of lib/_http_outgoing.js's errors were still in the "old style": `throw new Error()`. This commit migrates those 2 old style errors to the "new style": internal/errors.js's error-system. In the future, changes to these errors' messages won't break semver-major status. With the old style, changes to these errors' messages broke semver-major status. It was inconvenient. Refs: https://github.com/nodejs/node/issues/17709 --- lib/_http_outgoing.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 07cf84866fd831..324373f3851658 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -629,7 +629,7 @@ OutgoingMessage.prototype.write = function write(chunk, encoding, callback) { function write_(msg, chunk, encoding, callback, fromEnd) { if (msg.finished) { - var err = new Error('write after end'); + const err = new errors.Error('ERR_STREAM_WRITE_AFTER_END'); nextTick(msg.socket && msg.socket[async_id_symbol], writeAfterEndNT.bind(msg), err, @@ -880,7 +880,7 @@ OutgoingMessage.prototype.flush = internalUtil.deprecate(function() { OutgoingMessage.prototype.pipe = function pipe() { // OutgoingMessage should be write-only. Piping from it is disabled. - this.emit('error', new Error('Cannot pipe, not readable')); + this.emit('error', new errors.Error('ERR_STREAM_CANNOT_PIPE')); }; module.exports = { From 6ba656c049f8568441c6c422db8b0e40b4242ca9 Mon Sep 17 00:00:00 2001 From: Anton Paras Date: Sat, 23 Dec 2017 02:39:24 -0800 Subject: [PATCH 2/3] test: adapt _http_outgoing.js tests to new errors Two of _http_outgoing.js's errors were migrated to the internal/errors.js system. This commit adapts _http_outgoing.js's tests accordingly. Refs: https://github.com/nodejs/node/issues/17709 --- test/parallel/test-http-res-write-after-end.js | 5 ++++- test/parallel/test-http-server-write-after-end.js | 6 ++++-- test/parallel/test-outgoing-message-pipe.js | 7 ++++--- .../test-pipe-outgoing-message-data-emitted-after-ended.js | 6 ++++-- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/test/parallel/test-http-res-write-after-end.js b/test/parallel/test-http-res-write-after-end.js index 32c6cf2e082668..90c3535896ce93 100644 --- a/test/parallel/test-http-res-write-after-end.js +++ b/test/parallel/test-http-res-write-after-end.js @@ -26,7 +26,10 @@ const http = require('http'); const server = http.Server(common.mustCall(function(req, res) { res.on('error', common.mustCall(function onResError(err) { - assert.strictEqual(err.message, 'write after end'); + common.expectsError({ + code: 'ERR_STREAM_WRITE_AFTER_END', + type: Error + })(err); })); res.write('This should write.'); diff --git a/test/parallel/test-http-server-write-after-end.js b/test/parallel/test-http-server-write-after-end.js index 045cdcf4b75afb..a405844cfc9659 100644 --- a/test/parallel/test-http-server-write-after-end.js +++ b/test/parallel/test-http-server-write-after-end.js @@ -2,7 +2,6 @@ const common = require('../common'); const http = require('http'); -const assert = require('assert'); // Fix for https://github.com/nodejs/node/issues/14368 @@ -10,7 +9,10 @@ const server = http.createServer(handle); function handle(req, res) { res.on('error', common.mustCall((err) => { - assert.strictEqual(err.message, 'write after end'); + common.expectsError({ + code: 'ERR_STREAM_WRITE_AFTER_END', + type: Error + })(err); server.close(); })); diff --git a/test/parallel/test-outgoing-message-pipe.js b/test/parallel/test-outgoing-message-pipe.js index 2030d8f43b09be..3486f458ccc135 100644 --- a/test/parallel/test-outgoing-message-pipe.js +++ b/test/parallel/test-outgoing-message-pipe.js @@ -8,8 +8,9 @@ const OutgoingMessage = require('_http_outgoing').OutgoingMessage; const outgoingMessage = new OutgoingMessage(); assert.throws( common.mustCall(() => { outgoingMessage.pipe(outgoingMessage); }), - (err) => { - return ((err instanceof Error) && /Cannot pipe, not readable/.test(err)); - }, + common.expectsError({ + code: 'ERR_STREAM_CANNOT_PIPE', + type: Error + }), 'OutgoingMessage.pipe should throw an error' ); diff --git a/test/parallel/test-pipe-outgoing-message-data-emitted-after-ended.js b/test/parallel/test-pipe-outgoing-message-data-emitted-after-ended.js index 0bd823908a6107..7c920480e46d29 100644 --- a/test/parallel/test-pipe-outgoing-message-data-emitted-after-ended.js +++ b/test/parallel/test-pipe-outgoing-message-data-emitted-after-ended.js @@ -1,7 +1,6 @@ 'use strict'; const common = require('../common'); const http = require('http'); -const assert = require('assert'); const stream = require('stream'); // Verify that when piping a stream to an `OutgoingMessage` (or a type that @@ -18,7 +17,10 @@ const server = http.createServer(common.mustCall(function(req, res) { res.end(); myStream.emit('data', 'some data'); res.on('error', common.mustCall(function(err) { - assert.strictEqual(err.message, 'write after end'); + common.expectsError({ + code: 'ERR_STREAM_WRITE_AFTER_END', + type: Error + })(err); })); process.nextTick(common.mustCall(() => server.close())); From 1ce419786b84bac4e439131a613d227ca843567d Mon Sep 17 00:00:00 2001 From: Anton Paras Date: Sat, 23 Dec 2017 16:43:11 -0800 Subject: [PATCH 3/3] test: simplify _http_outgoing.js's error tests _http_outgoing.js's tests perform error-checks. They ensure that appropriate errors are thrown in the appropriate circumstances. A few error-checks were bloated. They used unnecessary intermediate functions. E.g. there were unnecessary combinations of `common.mustCall()` & `common.expectsError()` and `common.expectsError()` & `assert.throws()`. This commit removes those unnecessary intermediate functions. This simplifies the given error-checks. --- test/parallel/test-http-res-write-after-end.js | 8 +++----- test/parallel/test-outgoing-message-pipe.js | 10 ++++------ ...t-pipe-outgoing-message-data-emitted-after-ended.js | 8 +++----- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/test/parallel/test-http-res-write-after-end.js b/test/parallel/test-http-res-write-after-end.js index 90c3535896ce93..1d34cf6dcb4841 100644 --- a/test/parallel/test-http-res-write-after-end.js +++ b/test/parallel/test-http-res-write-after-end.js @@ -25,11 +25,9 @@ const assert = require('assert'); const http = require('http'); const server = http.Server(common.mustCall(function(req, res) { - res.on('error', common.mustCall(function onResError(err) { - common.expectsError({ - code: 'ERR_STREAM_WRITE_AFTER_END', - type: Error - })(err); + res.on('error', common.expectsError({ + code: 'ERR_STREAM_WRITE_AFTER_END', + type: Error })); res.write('This should write.'); diff --git a/test/parallel/test-outgoing-message-pipe.js b/test/parallel/test-outgoing-message-pipe.js index 3486f458ccc135..7cfe3687ecc8a7 100644 --- a/test/parallel/test-outgoing-message-pipe.js +++ b/test/parallel/test-outgoing-message-pipe.js @@ -1,16 +1,14 @@ 'use strict'; -const assert = require('assert'); const common = require('../common'); const OutgoingMessage = require('_http_outgoing').OutgoingMessage; // Verify that an error is thrown upon a call to `OutgoingMessage.pipe`. const outgoingMessage = new OutgoingMessage(); -assert.throws( - common.mustCall(() => { outgoingMessage.pipe(outgoingMessage); }), - common.expectsError({ +common.expectsError( + () => { outgoingMessage.pipe(outgoingMessage); }, + { code: 'ERR_STREAM_CANNOT_PIPE', type: Error - }), - 'OutgoingMessage.pipe should throw an error' + } ); diff --git a/test/parallel/test-pipe-outgoing-message-data-emitted-after-ended.js b/test/parallel/test-pipe-outgoing-message-data-emitted-after-ended.js index 7c920480e46d29..c6191a906b32d6 100644 --- a/test/parallel/test-pipe-outgoing-message-data-emitted-after-ended.js +++ b/test/parallel/test-pipe-outgoing-message-data-emitted-after-ended.js @@ -16,11 +16,9 @@ const server = http.createServer(common.mustCall(function(req, res) { process.nextTick(common.mustCall(() => { res.end(); myStream.emit('data', 'some data'); - res.on('error', common.mustCall(function(err) { - common.expectsError({ - code: 'ERR_STREAM_WRITE_AFTER_END', - type: Error - })(err); + res.on('error', common.expectsError({ + code: 'ERR_STREAM_WRITE_AFTER_END', + type: Error })); process.nextTick(common.mustCall(() => server.close()));