Skip to content

Commit

Permalink
http_client, errors: migrate to internal/errors
Browse files Browse the repository at this point in the history
PR-URL: #14423
Refs: #11273
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
  • Loading branch information
starkwang authored and tniessen committed Aug 1, 2017
1 parent 8db3997 commit bdfbce9
Show file tree
Hide file tree
Showing 10 changed files with 121 additions and 32 deletions.
22 changes: 22 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,7 @@ Used when `Console` is instantiated without `stdout` stream or when `stdout` or
Used when the native call from `process.cpuUsage` cannot be processed properly.

<a id="ERR_DNS_SET_SERVERS_FAILED"></a>
### ERR_DNS_SET_SERVERS_FAILED

Used when `c-ares` failed to set the DNS server.

Expand Down Expand Up @@ -661,6 +662,11 @@ Used when invalid characters are detected in headers.
The `'ERR_INVALID_CURSOR_POS'` is thrown specifically when a cursor on a given
stream is attempted to move to a specified row without a specified column.

<a id="ERR_INVALID_DOMAIN_NAME"></a>
### ERR_INVALID_DOMAIN_NAME

Used when `hostname` can not be parsed from a provided URL.

<a id="ERR_INVALID_FD"></a>
### ERR_INVALID_FD

Expand Down Expand Up @@ -689,7 +695,13 @@ Used when an attempt is made to send an unsupported "handle" over an IPC
communication channel to a child process. See [`child.send()`] and
[`process.send()`] for more information.

<a id="ERR_INVALID_HTTP_TOKEN"></a>
### ERR_INVALID_HTTP_TOKEN

Used when `options.method` received an invalid HTTP token.

<a id="ERR_INVALID_IP_ADDRESS"></a>
### ERR_INVALID_IP_ADDRESS

Used when an IP address is not valid.

Expand All @@ -704,6 +716,11 @@ passed in an options object.

Used when an invalid or unknown file encoding is passed.

<a id="ERR_INVALID_PROTOCOL"></a>
### ERR_INVALID_PROTOCOL

Used when an invalid `options.protocol` is passed.

<a id="ERR_INVALID_REPL_EVAL_CONFIG"></a>
### ERR_INVALID_REPL_EVAL_CONFIG

Expand Down Expand Up @@ -879,6 +896,11 @@ Used to identify a specific kind of internal Node.js error that should not
typically be triggered by user code. Instances of this error point to an
internal bug within the Node.js binary itself.

<a id="ERR_UNESCAPED_CHARACTERS"></a>
### ERR_UNESCAPED_CHARACTERS

Used when a string that contains unescaped characters was received.

<a id="ERR_UNKNOWN_ENCODING"></a>
### ERR_UNKNOWN_ENCODING

Expand Down
27 changes: 13 additions & 14 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const Buffer = require('buffer').Buffer;
const { urlToOptions, searchParamsSymbol } = require('internal/url');
const outHeadersKey = require('internal/http').outHeadersKey;
const nextTick = require('internal/process/next_tick').nextTick;
const errors = require('internal/errors');

// The actual list of disallowed characters in regexp form is more like:
// /[^A-Za-z0-9\-._~!$&'()*+,;=/:@]/
Expand Down Expand Up @@ -68,8 +69,8 @@ function isInvalidPath(s) {

function validateHost(host, name) {
if (host != null && typeof host !== 'string') {
throw new TypeError(
`"options.${name}" must either be a string, undefined or null`);
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', `options.${name}`,
['string', 'undefined', 'null'], host);
}
return host;
}
Expand All @@ -80,7 +81,7 @@ function ClientRequest(options, cb) {
if (typeof options === 'string') {
options = url.parse(options);
if (!options.hostname) {
throw new Error('Unable to determine the domain name');
throw new errors.Error('ERR_INVALID_DOMAIN_NAME');
}
} else if (options && options[searchParamsSymbol] &&
options[searchParamsSymbol][searchParamsSymbol]) {
Expand All @@ -101,9 +102,8 @@ 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 TypeError(
'Agent option must be an Agent-like object, undefined, or false.'
);
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'Agent option',
['Agent-like object', 'undefined', 'false']);
}
this.agent = agent;

Expand All @@ -122,12 +122,11 @@ function ClientRequest(options, cb) {
invalidPath = /[\u0000-\u0020]/.test(path);
}
if (invalidPath)
throw new TypeError('Request path contains unescaped characters');
throw new errors.TypeError('ERR_UNESCAPED_CHARACTERS', 'Request path');
}

if (protocol !== expectedProtocol) {
throw new Error('Protocol "' + protocol + '" not supported. ' +
'Expected "' + expectedProtocol + '"');
throw new errors.Error('ERR_INVALID_PROTOCOL', protocol, expectedProtocol);
}

var defaultPort = options.defaultPort ||
Expand All @@ -145,12 +144,13 @@ function ClientRequest(options, cb) {
var method = options.method;
var methodIsString = (typeof method === 'string');
if (method != null && !methodIsString) {
throw new TypeError('Method must be a string');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'method',
'string', method);
}

if (methodIsString && method) {
if (!common._checkIsHttpToken(method)) {
throw new TypeError('Method must be a valid HTTP token');
throw new errors.TypeError('ERR_INVALID_HTTP_TOKEN', 'Method');
}
method = this.method = method.toUpperCase();
} else {
Expand Down Expand Up @@ -211,8 +211,7 @@ function ClientRequest(options, cb) {
options.headers);
} else if (this.getHeader('expect')) {
if (this._header) {
throw new Error('Can\'t render headers after they are sent to the ' +
'client');
throw new errors.Error('ERR_HTTP_HEADERS_SENT');
}

this._storeHeader(this.method + ' ' + this.path + ' HTTP/1.1\r\n',
Expand Down Expand Up @@ -303,7 +302,7 @@ ClientRequest.prototype._finish = function _finish() {

ClientRequest.prototype._implicitHeader = function _implicitHeader() {
if (this._header) {
throw new Error('Can\'t render headers after they are sent to the client');
throw new errors.Error('ERR_HTTP_HEADERS_SENT');
}
this._storeHeader(this.method + ' ' + this.path + ' HTTP/1.1\r\n',
this[outHeadersKey]);
Expand Down
6 changes: 6 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,18 +130,22 @@ E('ERR_INVALID_CALLBACK', 'Callback must be a function');
E('ERR_INVALID_CHAR', 'Invalid character in %s');
E('ERR_INVALID_CURSOR_POS',
'Cannot set cursor row without setting its column');
E('ERR_INVALID_DOMAIN_NAME', 'Unable to determine the domain name');
E('ERR_INVALID_FD', '"fd" must be a positive integer: %s');
E('ERR_INVALID_FILE_URL_HOST',
'File URL host must be "localhost" or empty on %s');
E('ERR_INVALID_FILE_URL_PATH', 'File URL path %s');
E('ERR_INVALID_HANDLE_TYPE', 'This handle type cannot be sent');
E('ERR_INVALID_HTTP_TOKEN', (name) => `${name} must be a valid HTTP token`);
E('ERR_INVALID_IP_ADDRESS', 'Invalid IP address: %s');
E('ERR_INVALID_OPT_VALUE',
(name, value) => {
return `The value "${String(value)}" is invalid for option "${name}"`;
});
E('ERR_INVALID_OPT_VALUE_ENCODING',
(value) => `The value "${String(value)}" is invalid for option "encoding"`);
E('ERR_INVALID_PROTOCOL', (protocol, expectedProtocol) =>
`Protocol "${protocol}" not supported. Expected "${expectedProtocol}"`);
E('ERR_INVALID_REPL_EVAL_CONFIG',
'Cannot specify both "breakEvalOnSigint" and "eval" for REPL');
E('ERR_INVALID_SYNC_FORK_INPUT',
Expand Down Expand Up @@ -185,6 +189,8 @@ E('ERR_TRANSFORM_ALREADY_TRANSFORMING',
'Calling transform done when still transforming');
E('ERR_TRANSFORM_WITH_LENGTH_0',
'Calling transform done when writableState.length != 0');
E('ERR_UNESCAPED_CHARACTERS',
(name) => `${name} contains unescaped characters`);
E('ERR_UNKNOWN_ENCODING', 'Unknown encoding: %s');
E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s');
E('ERR_UNKNOWN_STDIN_TYPE', 'Unknown stdin file type');
Expand Down
7 changes: 6 additions & 1 deletion test/parallel/test-http-client-check-http-token.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@ server.listen(0, common.mustCall(() => {
expectedFails.forEach((method) => {
assert.throws(() => {
http.request({ method, path: '/' }, common.mustNotCall());
}, /^TypeError: Method must be a string$/);
}, common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "method" argument must be of type string. ' +
`Received type ${typeof method}`
}));
});

expectedSuccesses.forEach((method) => {
Expand Down
8 changes: 6 additions & 2 deletions test/parallel/test-http-client-reject-unexpected-agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,12 @@ server.listen(0, baseOptions.host, common.mustCall(function() {
failingAgentOptions.forEach((agent) => {
assert.throws(
() => createRequest(agent),
/^TypeError: Agent option must be an Agent-like object/,
`Expected typeof agent: ${typeof agent} to be rejected`
common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "Agent option" argument must be one of type ' +
'Agent-like object, undefined, or false'
})
);
});

Expand Down
10 changes: 8 additions & 2 deletions test/parallel/test-http-client-unescaped-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,14 @@ const common = require('../common');
const assert = require('assert');
const http = require('http');

const errMessage = /contains unescaped characters/;
for (let i = 0; i <= 32; i += 1) {
const path = `bad${String.fromCharCode(i)}path`;
assert.throws(() => http.get({ path }, common.mustNotCall()), errMessage);
assert.throws(
() => http.get({ path }, common.mustNotCall()),
common.expectsError({
code: 'ERR_UNESCAPED_CHARACTERS',
type: TypeError,
message: 'Request path contains unescaped characters'
})
);
}
30 changes: 22 additions & 8 deletions test/parallel/test-http-hostname-typechecking.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,35 @@
'use strict';
require('../common');

const common = require('../common');
const assert = require('assert');
const http = require('http');

// All of these values should cause http.request() to throw synchronously
// when passed as the value of either options.hostname or options.host
const vals = [{}, [], NaN, Infinity, -Infinity, true, false, 1, 0, new Date()];

const errHostname =
/^TypeError: "options\.hostname" must either be a string, undefined or null$/;
const errHost =
/^TypeError: "options\.host" must either be a string, undefined or null$/;

vals.forEach((v) => {
assert.throws(() => http.request({ hostname: v }), errHostname);
assert.throws(() => http.request({ host: v }), errHost);
assert.throws(
() => http.request({ hostname: v }),
common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "options.hostname" property must be one of ' +
'type string, undefined, or null. ' +
`Received type ${typeof v}`
})
);

assert.throws(
() => http.request({ host: v }),
common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "options.host" property must be one of ' +
'type string, undefined, or null. ' +
`Received type ${typeof v}`
})
);
});

// These values are OK and should not throw synchronously.
Expand Down
9 changes: 7 additions & 2 deletions test/parallel/test-http-invalid-path-chars.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
'use strict';
require('../common');

const common = require('../common');
const assert = require('assert');
const http = require('http');

const expectedError = /^TypeError: Request path contains unescaped characters$/;
const expectedError = common.expectsError({
code: 'ERR_UNESCAPED_CHARACTERS',
type: TypeError,
message: 'Request path contains unescaped characters'
}, 1320);
const theExperimentallyDeterminedNumber = 39;

function fail(path) {
Expand Down
9 changes: 6 additions & 3 deletions test/parallel/test-http-request-invalid-method-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ const assert = require('assert');
const http = require('http');

assert.throws(
() => { http.request({ method: '\0' }); },
common.expectsError({ type: TypeError,
message: 'Method must be a valid HTTP token' })
() => http.request({ method: '\0' }),
common.expectsError({
code: 'ERR_INVALID_HTTP_TOKEN',
type: TypeError,
message: 'Method must be a valid HTTP token'
})
);
25 changes: 25 additions & 0 deletions test/parallel/test-internal-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,3 +235,28 @@ assert.throws(
assert.strictEqual(
errors.message('ERR_TLS_CERT_ALTNAME_INVALID', ['altname']),
'Hostname/IP does not match certificate\'s altnames: altname');

assert.strictEqual(
errors.message('ERR_INVALID_PROTOCOL', ['bad protocol', 'http']),
'Protocol "bad protocol" not supported. Expected "http"'
);

assert.strictEqual(
errors.message('ERR_HTTP_HEADERS_SENT'),
'Cannot render headers after they are sent to the client'
);

assert.strictEqual(
errors.message('ERR_INVALID_DOMAIN_NAME'),
'Unable to determine the domain name'
);

assert.strictEqual(
errors.message('ERR_INVALID_HTTP_TOKEN', ['Method']),
'Method must be a valid HTTP token'
);

assert.strictEqual(
errors.message('ERR_UNESCAPED_CHARACTERS', ['Request path']),
'Request path contains unescaped characters'
);

0 comments on commit bdfbce9

Please sign in to comment.