Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http_client, errors: migrate to use internal/errors #14423

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that I missing some titles for errors.md in #14212 . So I added the missing titles in this PR


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`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI you could just use %s place holders, but IMHO this way is more readable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But: consistency 😉

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is none anyway...
I'll open a PR to change them all.

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'
);