From b38c81cb449a822ab45e6caa210d070b91a59836 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 19 Mar 2018 14:18:50 +0100 Subject: [PATCH] lib: improve error handling This improves the error handling for a couple cases where the received value would not have been handled so far or where the name is wrong etc. PR-URL: https://github.com/nodejs/node/pull/19445 Reviewed-By: James M Snell Reviewed-By: Matteo Collina Reviewed-By: Joyee Cheung Reviewed-By: Anna Henningsen --- lib/_http_client.js | 5 +++-- lib/_http_outgoing.js | 2 +- lib/_tls_wrap.js | 3 ++- lib/buffer.js | 5 ++++- lib/internal/http2/compat.js | 7 +++++-- test/parallel/test-buffer-concat.js | 18 ++++++++++++++++++ ...test-http-client-reject-unexpected-agent.js | 4 ++-- .../test-http2-compat-serverrequest-headers.js | 11 ++++++----- test/parallel/test-tls-basic-validations.js | 4 ++-- 9 files changed, 43 insertions(+), 16 deletions(-) diff --git a/lib/_http_client.js b/lib/_http_client.js index 03c69a9306b626..5ab6d0bdba74b0 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -87,8 +87,9 @@ function ClientRequest(options, cb) { // Explicitly pass through this statement as agent will not be used // when createConnection is provided. } else if (typeof agent.addRequest !== 'function') { - throw new ERR_INVALID_ARG_TYPE('Agent option', - ['Agent-like Object', 'undefined', 'false']); + throw new ERR_INVALID_ARG_TYPE('options.agent', + ['Agent-like Object', 'undefined', 'false'], + agent); } this.agent = agent; diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 2af2fddb62b8ee..adc4481347f71a 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -755,7 +755,7 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) { var uncork; if (chunk) { if (typeof chunk !== 'string' && !(chunk instanceof Buffer)) { - throw new ERR_INVALID_ARG_TYPE('first argument', ['string', 'Buffer']); + throw new ERR_INVALID_ARG_TYPE('chunk', ['string', 'Buffer'], chunk); } if (!this._header) { if (typeof chunk === 'string') diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 41dd001bd6a28b..686e109ca30b8a 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -908,7 +908,8 @@ function Server(options, listener) { this[kSNICallback] = options.SNICallback; if (typeof this[kHandshakeTimeout] !== 'number') { - throw new ERR_INVALID_ARG_TYPE('timeout', 'number'); + throw new ERR_INVALID_ARG_TYPE( + 'options.handshakeTimeout', 'number', options.handshakeTimeout); } if (this.sessionTimeout) { diff --git a/lib/buffer.js b/lib/buffer.js index a13ada73ce6f5a..45c1282f2d9503 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -461,7 +461,10 @@ Buffer.concat = function concat(list, length) { for (i = 0; i < list.length; i++) { var buf = list[i]; if (!isUint8Array(buf)) { - throw new ERR_INVALID_ARG_TYPE('list', ['Array', 'Buffer', 'Uint8Array']); + // TODO(BridgeAR): This should not be of type ERR_INVALID_ARG_TYPE. + // Instead, find the proper error code for this. + throw new ERR_INVALID_ARG_TYPE( + `list[${i}]`, ['Array', 'Buffer', 'Uint8Array'], list[i]); } _copy(buf, buffer, pos); pos += buf.length; diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 78fedcb2ce5f7e..1a855f5bf04dc0 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -13,6 +13,7 @@ const { ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED, ERR_HTTP2_STATUS_INVALID, ERR_INVALID_ARG_TYPE, + ERR_INVALID_ARG_VALUE, ERR_INVALID_CALLBACK, ERR_INVALID_HTTP_TOKEN } = require('internal/errors').codes; @@ -312,8 +313,10 @@ class Http2ServerRequest extends Readable { } set method(method) { - if (typeof method !== 'string' || method.trim() === '') - throw new ERR_INVALID_ARG_TYPE('method', 'string'); + if (typeof method !== 'string') + throw new ERR_INVALID_ARG_TYPE('method', 'string', method); + if (method.trim() === '') + throw new ERR_INVALID_ARG_VALUE('method', method); this[kHeaders][HTTP2_HEADER_METHOD] = method; } diff --git a/test/parallel/test-buffer-concat.js b/test/parallel/test-buffer-concat.js index 27a485ec9d20ce..07c2189d97db43 100644 --- a/test/parallel/test-buffer-concat.js +++ b/test/parallel/test-buffer-concat.js @@ -54,6 +54,24 @@ assert.strictEqual(flatLongLen.toString(), check); }); }); +[[42], ['hello', Buffer.from('world')]].forEach((value) => { + assert.throws(() => { + Buffer.concat(value); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "list[0]" argument must be one of type Array, Buffer, ' + + `or Uint8Array. Received type ${typeof value[0]}` + }); +}); + +assert.throws(() => { + Buffer.concat([Buffer.from('hello'), 3]); +}, { + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "list[1]" argument must be one of type Array, Buffer, ' + + 'or Uint8Array. Received type number' +}); + // eslint-disable-next-line node-core/crypto-check const random10 = common.hasCrypto ? require('crypto').randomBytes(10) : diff --git a/test/parallel/test-http-client-reject-unexpected-agent.js b/test/parallel/test-http-client-reject-unexpected-agent.js index 50e5d2b57a7b96..27cb77951cf5fd 100644 --- a/test/parallel/test-http-client-reject-unexpected-agent.js +++ b/test/parallel/test-http-client-reject-unexpected-agent.js @@ -52,8 +52,8 @@ server.listen(0, baseOptions.host, common.mustCall(function() { { code: 'ERR_INVALID_ARG_TYPE', type: TypeError, - message: 'The "Agent option" argument must be one of type ' + - 'Agent-like Object, undefined, or false' + message: 'The "options.agent" property must be one of type Agent-like' + + ` Object, undefined, or false. Received type ${typeof agent}` } ); }); diff --git a/test/parallel/test-http2-compat-serverrequest-headers.js b/test/parallel/test-http2-compat-serverrequest-headers.js index 71120fa594273f..bd46b719000240 100644 --- a/test/parallel/test-http2-compat-serverrequest-headers.js +++ b/test/parallel/test-http2-compat-serverrequest-headers.js @@ -45,12 +45,12 @@ server.listen(0, common.mustCall(function() { // change the request method request.method = 'POST'; assert.strictEqual(request.method, 'POST'); - common.expectsError( + assert.throws( () => request.method = ' ', { - code: 'ERR_INVALID_ARG_TYPE', - type: TypeError, - message: 'The "method" argument must be of type string' + code: 'ERR_INVALID_ARG_VALUE', + name: 'TypeError [ERR_INVALID_ARG_VALUE]', + message: "The argument 'method' is invalid. Received ' '" } ); assert.throws( @@ -58,7 +58,8 @@ server.listen(0, common.mustCall(function() { { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError [ERR_INVALID_ARG_TYPE]', - message: 'The "method" argument must be of type string' + message: 'The "method" argument must be of type string. ' + + 'Received type boolean' } ); diff --git a/test/parallel/test-tls-basic-validations.js b/test/parallel/test-tls-basic-validations.js index a2f7c8d4617a27..74d56546ebb469 100644 --- a/test/parallel/test-tls-basic-validations.js +++ b/test/parallel/test-tls-basic-validations.js @@ -26,8 +26,8 @@ common.expectsError(() => tls.createServer({ handshakeTimeout: 'abcd' }), { code: 'ERR_INVALID_ARG_TYPE', type: TypeError, - message: 'The "timeout" argument must ' + - 'be of type number' + message: 'The "options.handshakeTimeout" property must ' + + 'be of type number. Received type string' } );