From 0419699639d5c5208eddf9489fd86e5887c828a6 Mon Sep 17 00:00:00 2001 From: kysnm Date: Wed, 20 Dec 2017 08:07:41 +0900 Subject: [PATCH 1/7] net: migrate errors to internal/errors --- doc/api/errors.md | 6 ++++++ lib/internal/errors.js | 1 + lib/net.js | 4 ++-- test/parallel/test-http-unix-socket.js | 2 +- test/parallel/test-net-socket-destroy-send.js | 4 ++-- test/parallel/test-net-socket-write-after-close.js | 3 +-- 6 files changed, 13 insertions(+), 7 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index 918cfa343136af..19851de607c78c 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1355,6 +1355,12 @@ The [`server.listen()`][] method was called while a `net.Server` was already listening. This applies to all instances of `net.Server`, including HTTP, HTTPS, and HTTP/2 Server instances. + +### ERR_SERVER_NOT_RUNNING + +The [`server.close()`][] method was called when a `net.Server` was not running. This applies to all instances of `net.Server`, including HTTP, HTTPS, +and HTTP/2 Server instances. + ### ERR_SOCKET_ALREADY_BOUND diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 7276bb6344a5bd..72db142c7a0e81 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -447,6 +447,7 @@ E('ERR_PARSE_HISTORY_DATA', 'Could not parse history data in %s'); E('ERR_REQUIRE_ESM', 'Must use import to load ES Module: %s'); E('ERR_SERVER_ALREADY_LISTEN', 'Listen method has been called more than once without closing.'); +E('ERR_SERVER_NOT_RUNNING', 'Server is not running.'); E('ERR_SOCKET_ALREADY_BOUND', 'Socket is already bound'); E('ERR_SOCKET_BAD_BUFFER_SIZE', 'Buffer size must be a positive integer'); E('ERR_SOCKET_BAD_PORT', 'Port should be > 0 and < 65536. Received %s.'); diff --git a/lib/net.js b/lib/net.js index 245415d87c63a1..bfafbcc21915ba 100644 --- a/lib/net.js +++ b/lib/net.js @@ -724,7 +724,7 @@ Socket.prototype._writeGeneric = function(writev, data, encoding, cb) { this._unrefTimer(); if (!this._handle) { - this.destroy(new Error('This socket is closed'), cb); + this.destroy(new errors.Error('ERR_SOCKET_CLOSED'), cb); return false; } @@ -1624,7 +1624,7 @@ Server.prototype.close = function(cb) { if (typeof cb === 'function') { if (!this._handle) { this.once('close', function close() { - cb(new Error('Not running')); + cb(new errors.Error('ERR_SERVER_NOT_RUNNING')); }); } else { this.once('close', cb); diff --git a/test/parallel/test-http-unix-socket.js b/test/parallel/test-http-unix-socket.js index 6d5897cacbecc2..44dd70fe3276dd 100644 --- a/test/parallel/test-http-unix-socket.js +++ b/test/parallel/test-http-unix-socket.js @@ -59,7 +59,7 @@ server.listen(common.PIPE, common.mustCall(function() { server.close(common.mustCall(function(error) { assert.strictEqual(error, undefined); server.close(common.mustCall(function(error) { - assert.strictEqual(error && error.message, 'Not running'); + assert.strictEqual(error && error.code, 'ERR_SERVER_NOT_RUNNING'); })); })); })); diff --git a/test/parallel/test-net-socket-destroy-send.js b/test/parallel/test-net-socket-destroy-send.js index 6dc4b9566a3900..2ba52a32beadcf 100644 --- a/test/parallel/test-net-socket-destroy-send.js +++ b/test/parallel/test-net-socket-destroy-send.js @@ -13,10 +13,10 @@ server.listen(0, common.mustCall(function() { // Test destroy returns this, even on multiple calls when it short-circuits. assert.strictEqual(conn, conn.destroy().destroy()); conn.on('error', common.mustCall(function(err) { - assert.strictEqual(err.message, 'This socket is closed'); + assert.strictEqual(err.code, 'ERR_SOCKET_CLOSED'); })); conn.write(Buffer.from('kaboom'), common.mustCall(function(err) { - assert.strictEqual(err.message, 'This socket is closed'); + assert.strictEqual(err.code, 'ERR_SOCKET_CLOSED'); })); server.close(); })); diff --git a/test/parallel/test-net-socket-write-after-close.js b/test/parallel/test-net-socket-write-after-close.js index d3a3d937d338ab..316188c52b5da4 100644 --- a/test/parallel/test-net-socket-write-after-close.js +++ b/test/parallel/test-net-socket-write-after-close.js @@ -28,8 +28,7 @@ const net = require('net'); const client = net.connect({ port }, common.mustCall(() => { client.on('error', common.mustCall((err) => { server.close(); - assert.strictEqual(err.constructor, Error); - assert.strictEqual(err.message, 'This socket is closed'); + assert.strictEqual(err.code, 'ERR_SOCKET_CLOSED'); })); client._handle.close(); client._handle = null; From 614792bf4f7c7dc5f5fbbbc7cfbe1c28b5c19707 Mon Sep 17 00:00:00 2001 From: kysnm Date: Wed, 20 Dec 2017 11:11:17 +0900 Subject: [PATCH 2/7] Wrap at 80 chars --- doc/api/errors.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index 19851de607c78c..dfffe263979ca2 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1358,7 +1358,8 @@ and HTTP/2 Server instances. ### ERR_SERVER_NOT_RUNNING -The [`server.close()`][] method was called when a `net.Server` was not running. This applies to all instances of `net.Server`, including HTTP, HTTPS, +The [`server.close()`][] method was called when a `net.Server` was not running. +This applies to all instances of `net.Server`, including HTTP, HTTPS, and HTTP/2 Server instances. From 9f60c2cb9c2ed0a03139660fd7bb0ffd48db76a0 Mon Sep 17 00:00:00 2001 From: kysnm Date: Wed, 20 Dec 2017 11:14:32 +0900 Subject: [PATCH 3/7] `Server` should be enclosed to backticks --- doc/api/errors.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index dfffe263979ca2..794fc946b02563 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1353,14 +1353,14 @@ An attempt was made to `require()` an [ES6 module][]. The [`server.listen()`][] method was called while a `net.Server` was already listening. This applies to all instances of `net.Server`, including HTTP, HTTPS, -and HTTP/2 Server instances. +and HTTP/2 `Server` instances. ### ERR_SERVER_NOT_RUNNING The [`server.close()`][] method was called when a `net.Server` was not running. This applies to all instances of `net.Server`, including HTTP, HTTPS, -and HTTP/2 Server instances. +and HTTP/2 `Server` instances. ### ERR_SOCKET_ALREADY_BOUND From 4f79622f497efe5d588eb33fdf661e177b471981 Mon Sep 17 00:00:00 2001 From: kysnm Date: Wed, 20 Dec 2017 11:19:39 +0900 Subject: [PATCH 4/7] Just an adjustment --- doc/api/errors.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index 794fc946b02563..1e091b46a1d8d3 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1358,8 +1358,8 @@ and HTTP/2 `Server` instances. ### ERR_SERVER_NOT_RUNNING -The [`server.close()`][] method was called when a `net.Server` was not running. -This applies to all instances of `net.Server`, including HTTP, HTTPS, +The [`server.close()`][] method was called when a `net.Server` was not +running. This applies to all instances of `net.Server`, including HTTP, HTTPS, and HTTP/2 `Server` instances. From 37711dd9e60c14dcd6748cc0417899929bbcc4b4 Mon Sep 17 00:00:00 2001 From: kysnm Date: Wed, 20 Dec 2017 22:45:57 +0900 Subject: [PATCH 5/7] Use common.expectsError instead of assert.strictEqual --- test/parallel/test-http-unix-socket.js | 6 +++++- test/parallel/test-net-socket-destroy-send.js | 12 ++++++++++-- test/parallel/test-net-socket-write-after-close.js | 6 +++++- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-http-unix-socket.js b/test/parallel/test-http-unix-socket.js index 44dd70fe3276dd..f166456163ca53 100644 --- a/test/parallel/test-http-unix-socket.js +++ b/test/parallel/test-http-unix-socket.js @@ -59,7 +59,11 @@ server.listen(common.PIPE, common.mustCall(function() { server.close(common.mustCall(function(error) { assert.strictEqual(error, undefined); server.close(common.mustCall(function(error) { - assert.strictEqual(error && error.code, 'ERR_SERVER_NOT_RUNNING'); + common.expectsError({ + code: 'ERR_SERVER_NOT_RUNNING', + message: 'Server is not running.', + type: Error + })(error); })); })); })); diff --git a/test/parallel/test-net-socket-destroy-send.js b/test/parallel/test-net-socket-destroy-send.js index 2ba52a32beadcf..62a8b28f0a8066 100644 --- a/test/parallel/test-net-socket-destroy-send.js +++ b/test/parallel/test-net-socket-destroy-send.js @@ -13,10 +13,18 @@ server.listen(0, common.mustCall(function() { // Test destroy returns this, even on multiple calls when it short-circuits. assert.strictEqual(conn, conn.destroy().destroy()); conn.on('error', common.mustCall(function(err) { - assert.strictEqual(err.code, 'ERR_SOCKET_CLOSED'); + common.expectsError({ + code: 'ERR_SOCKET_CLOSED', + message: 'Socket is closed', + type: Error + })(err); })); conn.write(Buffer.from('kaboom'), common.mustCall(function(err) { - assert.strictEqual(err.code, 'ERR_SOCKET_CLOSED'); + common.expectsError({ + code: 'ERR_SOCKET_CLOSED', + message: 'Socket is closed', + type: Error + })(err); })); server.close(); })); diff --git a/test/parallel/test-net-socket-write-after-close.js b/test/parallel/test-net-socket-write-after-close.js index 316188c52b5da4..de1cb0e3ed6a53 100644 --- a/test/parallel/test-net-socket-write-after-close.js +++ b/test/parallel/test-net-socket-write-after-close.js @@ -28,7 +28,11 @@ const net = require('net'); const client = net.connect({ port }, common.mustCall(() => { client.on('error', common.mustCall((err) => { server.close(); - assert.strictEqual(err.code, 'ERR_SOCKET_CLOSED'); + common.expectsError({ + code: 'ERR_SOCKET_CLOSED', + message: 'Socket is closed', + type: Error + })(err); })); client._handle.close(); client._handle = null; From 7832c2fb39f19ef0a550f7d4c7a2ed33e13fa472 Mon Sep 17 00:00:00 2001 From: kysnm Date: Thu, 21 Dec 2017 00:13:50 +0900 Subject: [PATCH 6/7] Remove useless common.mustCall --- test/parallel/test-net-socket-destroy-send.js | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/test/parallel/test-net-socket-destroy-send.js b/test/parallel/test-net-socket-destroy-send.js index 62a8b28f0a8066..a602b89253887d 100644 --- a/test/parallel/test-net-socket-destroy-send.js +++ b/test/parallel/test-net-socket-destroy-send.js @@ -12,19 +12,16 @@ server.listen(0, common.mustCall(function() { conn.on('connect', common.mustCall(function() { // Test destroy returns this, even on multiple calls when it short-circuits. assert.strictEqual(conn, conn.destroy().destroy()); - conn.on('error', common.mustCall(function(err) { - common.expectsError({ - code: 'ERR_SOCKET_CLOSED', - message: 'Socket is closed', - type: Error - })(err); + conn.on('error', common.expectsError({ + code: 'ERR_SOCKET_CLOSED', + message: 'Socket is closed', + type: Error })); - conn.write(Buffer.from('kaboom'), common.mustCall(function(err) { - common.expectsError({ - code: 'ERR_SOCKET_CLOSED', - message: 'Socket is closed', - type: Error - })(err); + + conn.write(Buffer.from('kaboom'), common.expectsError({ + code: 'ERR_SOCKET_CLOSED', + message: 'Socket is closed', + type: Error })); server.close(); })); From 3cf5f8f95e398c29fb28433dc4404533406fa820 Mon Sep 17 00:00:00 2001 From: kysnm Date: Thu, 21 Dec 2017 16:38:45 +0900 Subject: [PATCH 7/7] Remove useless common.mustCall again --- test/parallel/test-http-unix-socket.js | 10 ++++------ test/parallel/test-net-socket-write-after-close.js | 14 +++++++------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/test/parallel/test-http-unix-socket.js b/test/parallel/test-http-unix-socket.js index f166456163ca53..7a17b9bc9ca50a 100644 --- a/test/parallel/test-http-unix-socket.js +++ b/test/parallel/test-http-unix-socket.js @@ -58,12 +58,10 @@ server.listen(common.PIPE, common.mustCall(function() { assert.strictEqual(res.body, 'hello world\n'); server.close(common.mustCall(function(error) { assert.strictEqual(error, undefined); - server.close(common.mustCall(function(error) { - common.expectsError({ - code: 'ERR_SERVER_NOT_RUNNING', - message: 'Server is not running.', - type: Error - })(error); + server.close(common.expectsError({ + code: 'ERR_SERVER_NOT_RUNNING', + message: 'Server is not running.', + type: Error })); })); })); diff --git a/test/parallel/test-net-socket-write-after-close.js b/test/parallel/test-net-socket-write-after-close.js index de1cb0e3ed6a53..e3593a3a09a146 100644 --- a/test/parallel/test-net-socket-write-after-close.js +++ b/test/parallel/test-net-socket-write-after-close.js @@ -26,14 +26,14 @@ const net = require('net'); server.listen(common.mustCall(() => { const port = server.address().port; const client = net.connect({ port }, common.mustCall(() => { - client.on('error', common.mustCall((err) => { - server.close(); - common.expectsError({ - code: 'ERR_SOCKET_CLOSED', - message: 'Socket is closed', - type: Error - })(err); + client.on('error', common.expectsError({ + code: 'ERR_SOCKET_CLOSED', + message: 'Socket is closed', + type: Error })); + + server.close(); + client._handle.close(); client._handle = null; client.write('foo');