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.

Sign up for GitHub

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

Jump to bottom

_http_outgoing: migrate to use internal/errors #14735

Closed
wants to merge 1 commit into from
Closed

_http_outgoing: migrate to use internal/errors #14735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions doc/api/errors.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,11 @@ Used when an attempt is made to open an IPC communication channel with a
synchronous forked Node.js process. See the documentation for the
[`child_process`](child_process.html) module for more information.

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

Used when a method is required but not implemented.

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

Expand Down
6 changes: 3 additions & 3 deletions lib/_http_client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ function ClientRequest(options, cb) {

if (methodIsString && method) {
if (!common._checkIsHttpToken(method)) {
throw new errors.TypeError('ERR_INVALID_HTTP_TOKEN', 'Method');
throw new errors.TypeError('ERR_INVALID_HTTP_TOKEN', 'Method', method);
}
method = this.method = method.toUpperCase();
} else {
Expand Down Expand Up @@ -211,7 +211,7 @@ function ClientRequest(options, cb) {
options.headers);
} else if (this.getHeader('expect')) {
if (this._header) {
throw new errors.Error('ERR_HTTP_HEADERS_SENT');
throw new errors.Error('ERR_HTTP_HEADERS_SENT', 'render');
}

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

ClientRequest.prototype._implicitHeader = function _implicitHeader() {
if (this._header) {
throw new errors.Error('ERR_HTTP_HEADERS_SENT');
throw new errors.Error('ERR_HTTP_HEADERS_SENT', 'render');
}
this._storeHeader(this.method + ' ' + this.path + ' HTTP/1.1\r\n',
this[outHeadersKey]);
Expand Down
40 changes: 21 additions & 19 deletions lib/_http_outgoing.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ Object.defineProperty(OutgoingMessage.prototype, '_headerNames', {

OutgoingMessage.prototype._renderHeaders = function _renderHeaders() {
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', 'render');
}

var headersMap = this[outHeadersKey];
Expand Down Expand Up @@ -433,14 +433,14 @@ function _storeHeader(firstLine, headers) {
function storeHeader(self, state, key, value, validate) {
if (validate) {
if (typeof key !== 'string' || !key || !checkIsHttpToken(key)) {
throw new TypeError(
'Header name must be a valid HTTP Token ["' + key + '"]');
throw new errors.TypeError(
'ERR_INVALID_HTTP_TOKEN', 'Header name', key);
}
if (value === undefined) {
throw new Error('Header "%s" value must not be undefined', key);
throw new errors.TypeError('ERR_MISSING_ARGS', `header "${key}"`);
} else if (checkInvalidHeaderChar(value)) {
debug('Header "%s" contains invalid characters', key);
throw new TypeError('The header content contains invalid characters');
throw new errors.TypeError('ERR_INVALID_CHAR', 'header content', key);
}
}
state.header += key + ': ' + escapeHeaderValue(value) + CRLF;
Expand Down Expand Up @@ -491,14 +491,14 @@ function matchHeader(self, state, field, value) {

function validateHeader(msg, name, value) {
if (typeof name !== 'string' || !name || !checkIsHttpToken(name))
throw new TypeError(`Header name must be a valid HTTP Token ["${name}"]`);
throw new errors.TypeError('ERR_INVALID_HTTP_TOKEN', 'Header name', name);
if (value === undefined)
throw new Error('"value" required in setHeader("' + name + '", value)');
throw new errors.TypeError('ERR_MISSING_ARGS', 'value');
if (msg._header)
throw new Error('Can\'t set headers after they are sent.');
throw new errors.Error('ERR_HTTP_HEADERS_SENT', 'set');
if (checkInvalidHeaderChar(value)) {
debug('Header "%s" contains invalid characters', name);
throw new TypeError('The header content contains invalid characters');
throw new errors.TypeError('ERR_INVALID_CHAR', 'header content', name);
}
}
OutgoingMessage.prototype.setHeader = function setHeader(name, value) {
Expand Down Expand Up @@ -529,7 +529,7 @@ OutgoingMessage.prototype.setHeader = function setHeader(name, value) {

OutgoingMessage.prototype.getHeader = function getHeader(name) {
if (typeof name !== 'string') {
throw new TypeError('"name" argument must be a string');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'name', 'string');
}

if (!this[outHeadersKey]) return;
Expand Down Expand Up @@ -565,7 +565,7 @@ OutgoingMessage.prototype.getHeaders = function getHeaders() {

OutgoingMessage.prototype.hasHeader = function hasHeader(name) {
if (typeof name !== 'string') {
throw new TypeError('"name" argument must be a string');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'name', 'string');
}

return !!(this[outHeadersKey] && this[outHeadersKey][name.toLowerCase()]);
Expand All @@ -574,11 +574,11 @@ OutgoingMessage.prototype.hasHeader = function hasHeader(name) {

OutgoingMessage.prototype.removeHeader = function removeHeader(name) {
if (typeof name !== 'string') {
throw new TypeError('"name" argument must be a string');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'name', 'string');
}

if (this._header) {
throw new Error('Can\'t remove headers after they are sent');
throw new errors.Error('ERR_HTTP_HEADERS_SENT', 'remove');
}

var key = name.toLowerCase();
Expand Down Expand Up @@ -609,7 +609,7 @@ OutgoingMessage.prototype.removeHeader = function removeHeader(name) {


OutgoingMessage.prototype._implicitHeader = function _implicitHeader() {
throw new Error('_implicitHeader() method is not implemented');
throw new errors.Error('ERR_METHOD_NOT_IMPLEMENTED', '_implicitHeader()');
};

Object.defineProperty(OutgoingMessage.prototype, 'headersSent', {
Expand Down Expand Up @@ -646,7 +646,8 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
}

if (!fromEnd && typeof chunk !== 'string' && !(chunk instanceof Buffer)) {
throw new TypeError('First argument must be a string or Buffer');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'first argument',
['string', 'buffer']);
}


Expand Down Expand Up @@ -712,12 +713,12 @@ OutgoingMessage.prototype.addTrailers = function addTrailers(headers) {
value = headers[key];
}
if (typeof field !== 'string' || !field || !checkIsHttpToken(field)) {
throw new TypeError(
'Trailer name must be a valid HTTP Token ["' + field + '"]');
throw new errors.TypeError('ERR_INVALID_HTTP_TOKEN', 'Trailer name',
field);
}
if (checkInvalidHeaderChar(value)) {
debug('Trailer "%s" contains invalid characters', field);
throw new TypeError('The trailer content contains invalid characters');
throw new errors.TypeError('ERR_INVALID_CHAR', 'trailer content', field);
}
this._trailer += field + ': ' + escapeHeaderValue(value) + CRLF;
}
Expand All @@ -743,7 +744,8 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) {
var uncork;
if (chunk) {
if (typeof chunk !== 'string' && !(chunk instanceof Buffer)) {
throw new TypeError('First argument must be a string or Buffer');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'first argument',
['string', 'buffer']);
}
if (!this._header) {
if (typeof chunk === 'string')
Expand Down
2 changes: 1 addition & 1 deletion lib/_http_server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ function writeHead(statusCode, reason, obj) {
}
}
if (k === undefined && this._header) {
throw new errors.Error('ERR_HTTP_HEADERS_SENT');
throw new errors.Error('ERR_HTTP_HEADERS_SENT', 'render');
}
// only progressive api is used
headers = this[outHeadersKey];
Expand Down
15 changes: 12 additions & 3 deletions lib/internal/errors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ E('ERR_ENCODING_NOT_SUPPORTED',
E('ERR_ENCODING_INVALID_ENCODED_DATA',
(enc) => `The encoded data was not valid for encoding ${enc}`);
E('ERR_HTTP_HEADERS_SENT',
'Cannot render headers after they are sent to the client');
'Cannot %s headers after they are sent to the client');
E('ERR_HTTP_INVALID_STATUS_CODE', 'Invalid status code: %s');
E('ERR_HTTP_TRAILER_INVALID',
'Trailers are invalid with this transfer encoding');
Expand Down Expand Up @@ -190,7 +190,7 @@ E('ERR_INVALID_ARRAY_LENGTH',
});
E('ERR_INVALID_BUFFER_SIZE', 'Buffer size must be a multiple of %s');
E('ERR_INVALID_CALLBACK', 'Callback must be a function');
E('ERR_INVALID_CHAR', 'Invalid character in %s');
E('ERR_INVALID_CHAR', invalidChar);
E('ERR_INVALID_CURSOR_POS',
'Cannot set cursor row without setting its column');
E('ERR_INVALID_DOMAIN_NAME', 'Unable to determine the domain name');
Expand All @@ -199,7 +199,7 @@ 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_HTTP_TOKEN', '%s must be a valid HTTP token ["%s"]');
E('ERR_INVALID_IP_ADDRESS', 'Invalid IP address: %s');
E('ERR_INVALID_OPT_VALUE',
(name, value) => {
Expand All @@ -222,6 +222,7 @@ E('ERR_IPC_CHANNEL_CLOSED', 'Channel closed');
E('ERR_IPC_DISCONNECTED', 'IPC channel is already disconnected');
E('ERR_IPC_ONE_PIPE', 'Child process can have only one IPC pipe');
E('ERR_IPC_SYNC_FORK', 'IPC cannot be used with synchronous forks');
E('ERR_METHOD_NOT_IMPLEMENTED', 'The %s method is not implemented');
E('ERR_MISSING_ARGS', missingArgs);
E('ERR_MULTIPLE_CALLBACK', 'Callback called multiple times');
E('ERR_NAPI_CONS_FUNCTION', 'Constructor must be a function');
Expand Down Expand Up @@ -342,3 +343,11 @@ function bufferOutOfBounds(name, isWriting) {
return `"${name}" is outside of buffer bounds`;
}
}

function invalidChar(name, field) {
let msg = `Invalid character in ${name}`;
if (field) {
msg += ` ["${field}"]`;
}
return msg;
}
73 changes: 49 additions & 24 deletions test/parallel/test-http-mutable-headers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,38 @@ const s = http.createServer(common.mustCall((req, res) => {
assert.deepStrictEqual(res.hasHeader('Connection'), false);
assert.deepStrictEqual(res.getHeader('Connection'), undefined);

assert.throws(() => {
res.setHeader();
}, /^TypeError: Header name must be a valid HTTP Token \["undefined"\]$/);
assert.throws(() => {
res.setHeader('someHeader');
}, /^Error: "value" required in setHeader\("someHeader", value\)$/);
assert.throws(() => {
res.getHeader();
}, /^TypeError: "name" argument must be a string$/);
assert.throws(() => {
res.removeHeader();
}, /^TypeError: "name" argument must be a string$/);
common.expectsError(
() => res.setHeader(),
{
code: 'ERR_INVALID_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.

@starkwang how much work will it to add the given value (in addition to the token name)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added some more information about the given value in the error message.

type: TypeError,
message: 'Header name must be a valid HTTP token ["undefined"]'
}
);
common.expectsError(
() => res.setHeader('someHeader'),
{
code: 'ERR_MISSING_ARGS',
type: TypeError,
message: 'The "value" argument must be specified'
}
);
common.expectsError(
() => res.getHeader(),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "name" argument must be of type string'
}
);
common.expectsError(
() => res.removeHeader(),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "name" argument must be of type string'
}
);

const arrayValues = [1, 2, 3];
res.setHeader('x-test-header', 'testing');
Expand Down Expand Up @@ -89,18 +109,23 @@ const s = http.createServer(common.mustCall((req, res) => {
assert.strictEqual(res.hasHeader('x-test-header2'), true);
assert.strictEqual(res.hasHeader('X-TEST-HEADER2'), true);
assert.strictEqual(res.hasHeader('X-Test-Header2'), true);
assert.throws(() => {
res.hasHeader();
}, /^TypeError: "name" argument must be a string$/);
assert.throws(() => {
res.hasHeader(null);
}, /^TypeError: "name" argument must be a string$/);
assert.throws(() => {
res.hasHeader(true);
}, /^TypeError: "name" argument must be a string$/);
assert.throws(() => {
res.hasHeader({ toString: () => 'X-TEST-HEADER2' });
}, /^TypeError: "name" argument must be a string$/);
[
undefined,
null,
true,
{},
{ toString: () => 'X-TEST-HEADER2' },
() => { }
].forEach((val) => {
common.expectsError(
() => res.hasHeader(val),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "name" argument must be of type string'
}
);
});

res.removeHeader('x-test-header2');

Expand Down
Loading