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

errors, buffer: Migrate buffer errors to use internal/errors #13976

Merged
merged 1 commit into from
Jul 12, 2017
Merged

errors, buffer: Migrate buffer errors to use internal/errors #13976

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
22 changes: 22 additions & 0 deletions doc/api/errors.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,11 @@ Used as special type of error that can be triggered whenever Node.js detects an
exceptional logic violation that should never occur. These are raised typically
by the `assert` module.

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

Used when attempting to perform an operation outside the bounds of a `Buffer`.

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

Expand Down Expand Up @@ -625,6 +630,11 @@ to a Node.js API.

Used when an Array is not of the expected length or in a valid range.

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

Used when performing a swap on a `Buffer` but it's size is not compatible with the operation.

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

Expand Down Expand Up @@ -781,6 +791,13 @@ would be possible by calling a callback more then once.
Used when an attempt is made to use crypto features while Node.js is not
compiled with OpenSSL crypto support.

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

Used when a Node.js API is called in an unsupported manner.

For example: `Buffer.write(string, encoding, offset[, length])`

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

Expand Down Expand Up @@ -844,6 +861,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_UNKNOWN_ENCODING"></a>
### ERR_UNKNOWN_ENCODING

Used when an invalid or unknown encoding option is passed to an API.

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

Expand Down
79 changes: 40 additions & 39 deletions lib/buffer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@ Object.defineProperty(exports, 'constants', {

exports.kStringMaxLength = binding.kStringMaxLength;

const kFromErrorMsg = 'First argument must be a string, Buffer, ' +
'ArrayBuffer, Array, or array-like object.';

Buffer.poolSize = 8 * 1024;
var poolSize, poolOffset, allocPool;

Expand Down Expand Up @@ -146,9 +143,8 @@ function Buffer(arg, encodingOrOffset, length) {
// Common case.
if (typeof arg === 'number') {
if (typeof encodingOrOffset === 'string') {
throw new Error(
'If encoding is specified then the first argument must be a string'
);
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'string',
'string', arg);
}
return Buffer.alloc(arg);
}
Expand Down Expand Up @@ -177,10 +173,12 @@ Buffer.from = function(value, encodingOrOffset, length) {
return fromArrayBuffer(value, encodingOrOffset, length);

if (value == null)
throw new TypeError(kFromErrorMsg);
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'first argument',
['string', 'buffer', 'arrayBuffer', 'array', 'array-like object'], value);

if (typeof value === 'number')
throw new TypeError('"value" argument must not be a number');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'value', 'not number',
Copy link
Member

Choose a reason for hiding this comment

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

What does the string end up being here ? I think this may be the first case were we have the 'not' cases versus listing out what it should be.

Copy link
Contributor Author

@starkwang starkwang Jul 7, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I extended the invalidArgType(name, expected, actual) in internal/errors.js to make it adapt to the 'not' cases. See here.

The case here will end up being:

TypeError [ERR_INVALID_ARG_TYPE]: The "value" argument must not be of type number. Received type number

The tests for it is also changed.

value);

const valueOf = value.valueOf && value.valueOf();
if (valueOf != null && valueOf !== value)
Expand All @@ -196,7 +194,8 @@ Buffer.from = function(value, encodingOrOffset, length) {
length);
}

throw new TypeError(kFromErrorMsg);
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'first argument',
['string', 'buffer', 'arrayBuffer', 'array', 'array-like object']);
};

Object.setPrototypeOf(Buffer, Uint8Array);
Expand All @@ -208,12 +207,11 @@ function assertSize(size) {
let err = null;

if (typeof size !== 'number') {
err = new TypeError('"size" argument must be a number');
err = new errors.TypeError('ERR_INVALID_ARG_TYPE', 'size', 'number', size);
} else if (size < 0) {
err = new RangeError('"size" argument must not be negative');
err = new errors.RangeError('ERR_INVALID_OPT_VALUE', 'size', size);
} else if (size > binding.kMaxLength) {
err = new RangeError('"size" argument must not be larger ' +
'than ' + binding.kMaxLength);
err = new errors.RangeError('ERR_INVALID_OPT_VALUE', 'size', size);
}

if (err) {
Expand Down Expand Up @@ -300,7 +298,7 @@ function fromString(string, encoding) {
} else {
length = byteLength(string, encoding, true);
if (length === -1)
throw new TypeError('"encoding" must be a valid string encoding');
throw new errors.TypeError('ERR_UNKNOWN_ENCODING', encoding);
if (string.length === 0)
return new FastBuffer();
}
Expand Down Expand Up @@ -343,7 +341,7 @@ function fromArrayBuffer(obj, byteOffset, length) {
const maxLength = obj.byteLength - byteOffset;

if (maxLength < 0)
throw new RangeError("'offset' is out of bounds");
throw new errors.RangeError('ERR_BUFFER_OUT_OF_BOUNDS', 'offset');

if (length === undefined) {
length = maxLength;
Expand All @@ -355,7 +353,7 @@ function fromArrayBuffer(obj, byteOffset, length) {
length = 0;
} else if (length > 0) {
if (length > maxLength)
throw new RangeError("'length' is out of bounds");
throw new errors.RangeError('ERR_BUFFER_OUT_OF_BOUNDS', 'length');
} else {
length = 0;
}
Expand Down Expand Up @@ -399,7 +397,8 @@ Buffer.isBuffer = function isBuffer(b) {

Buffer.compare = function compare(a, b) {
if (!isUint8Array(a) || !isUint8Array(b)) {
throw new TypeError('Arguments must be Buffers or Uint8Arrays');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', ['buf1', 'buf2'],
['buffer', 'uint8Array']);
}

if (a === b) {
Expand All @@ -416,13 +415,13 @@ Buffer.isEncoding = function(encoding) {
};
Buffer[internalUtil.kIsEncodingSymbol] = Buffer.isEncoding;

const kConcatErrMsg = '"list" argument must be an Array ' +
'of Buffer or Uint8Array instances';
const kConcatErr = new errors.TypeError('ERR_INVALID_ARG_TYPE', 'list',
['array', 'buffer', 'uint8Array']);

Buffer.concat = function(list, length) {
var i;
if (!Array.isArray(list))
throw new TypeError(kConcatErrMsg);
throw kConcatErr;

if (list.length === 0)
return new FastBuffer();
Expand All @@ -440,7 +439,7 @@ Buffer.concat = function(list, length) {
for (i = 0; i < list.length; i++) {
var buf = list[i];
if (!isUint8Array(buf))
throw new TypeError(kConcatErrMsg);
throw kConcatErr;
binding.copy(buf, buffer, pos);
pos += buf.length;
}
Expand Down Expand Up @@ -475,7 +474,8 @@ function byteLength(string, encoding) {
return string.byteLength;
}

throw new TypeError('"string" must be a string, Buffer, or ArrayBuffer');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'string',
['string', 'buffer', 'arrayBuffer']);
}

const len = string.length;
Expand Down Expand Up @@ -591,7 +591,7 @@ function stringSlice(buf, encoding, start, end) {
return buf.ucs2Slice(start, end);
break;
}
throw new TypeError('Unknown encoding: ' + encoding);
throw new errors.TypeError('ERR_UNKNOWN_ENCODING', encoding);
}


Expand Down Expand Up @@ -633,8 +633,8 @@ Buffer.prototype.toString = function(encoding, start, end) {

Buffer.prototype.equals = function equals(b) {
if (!isUint8Array(b))
throw new TypeError('Argument must be a Buffer or Uint8Array');

throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'otherBuffer',
['buffer', 'uint8Array']);
if (this === b)
return true;

Expand All @@ -659,7 +659,8 @@ Buffer.prototype.compare = function compare(target,
thisStart,
thisEnd) {
if (!isUint8Array(target))
throw new TypeError('Argument must be a Buffer or Uint8Array');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'target',
['buffer', 'uint8Array']);
if (arguments.length === 1)
return compare_(this, target);

Expand Down Expand Up @@ -738,8 +739,8 @@ function bidirectionalIndexOf(buffer, val, byteOffset, encoding, dir) {
return binding.indexOfNumber(buffer, val, byteOffset, dir);
}

throw new TypeError('"val" argument must be string, number, Buffer ' +
'or Uint8Array');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'val',
['string', 'buffer', 'uint8Array']);
}


Expand All @@ -765,7 +766,7 @@ function slowIndexOf(buffer, val, byteOffset, encoding, dir) {

default:
if (loweredCase) {
throw new TypeError('Unknown encoding: ' + encoding);
throw new errors.TypeError('ERR_UNKNOWN_ENCODING', encoding);
}

encoding = ('' + encoding).toLowerCase();
Expand Down Expand Up @@ -807,11 +808,11 @@ Buffer.prototype.fill = function fill(val, start, end, encoding) {
}

if (encoding !== undefined && typeof encoding !== 'string') {
throw new TypeError('encoding must be a string');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'encoding', 'string');
}
var normalizedEncoding = internalUtil.normalizeEncoding(encoding);
if (normalizedEncoding === undefined) {
throw new TypeError('Unknown encoding: ' + encoding);
throw new errors.TypeError('ERR_UNKNOWN_ENCODING', encoding);
}

if (val.length === 0) {
Expand Down Expand Up @@ -872,13 +873,13 @@ Buffer.prototype.write = function(string, offset, length, encoding) {
length = remaining;

if (string.length > 0 && (length < 0 || offset < 0))
throw new RangeError('Attempt to write outside buffer bounds');
throw new errors.RangeError('ERR_BUFFER_OUT_OF_BOUNDS', 'length', true);
} else {
// if someone is still calling the obsolete form of write(), tell them.
// we don't want eg buf.write("foo", "utf8", 10) to silently turn into
// buf.write("foo", "utf8"), so we can't ignore extra args
throw new Error('Buffer.write(string, encoding, offset[, length]) ' +
'is no longer supported');
throw new errors.Error('ERR_NO_LONGER_SUPPORTED',
'Buffer.write(string, encoding, offset[, length])');
}

if (!encoding) return this.utf8Write(string, offset, length);
Expand Down Expand Up @@ -925,7 +926,7 @@ Buffer.prototype.write = function(string, offset, length, encoding) {
return this.hexWrite(string, offset, length);
break;
}
throw new TypeError('Unknown encoding: ' + encoding);
throw new errors.TypeError('ERR_UNKNOWN_ENCODING', encoding);
};


Expand Down Expand Up @@ -1176,7 +1177,7 @@ Buffer.prototype.readDoubleBE = function readDoubleBE(offset, noAssert) {

function checkInt(buffer, value, offset, ext, max, min) {
if (value > max || value < min)
throw new TypeError('"value" argument is out of bounds');
throw new errors.RangeError('ERR_INVALID_OPT_VALUE', 'value', value);
if (offset + ext > buffer.length)
throw new errors.RangeError('ERR_INDEX_OUT_OF_RANGE');
}
Expand Down Expand Up @@ -1448,7 +1449,7 @@ Buffer.prototype.swap16 = function swap16() {
// dropping down to the native code is faster.
const len = this.length;
if (len % 2 !== 0)
throw new RangeError('Buffer size must be a multiple of 16-bits');
throw new errors.RangeError('ERR_INVALID_BUFFER_SIZE', '16-bits');
if (len < 128) {
for (var i = 0; i < len; i += 2)
swap(this, i, i + 1);
Expand All @@ -1464,7 +1465,7 @@ Buffer.prototype.swap32 = function swap32() {
// dropping down to the native code is faster.
const len = this.length;
if (len % 4 !== 0)
throw new RangeError('Buffer size must be a multiple of 32-bits');
throw new errors.RangeError('ERR_INVALID_BUFFER_SIZE', '32-bits');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ERR_INVALID_BUFFER_SIZE is added into error code for the error here

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this code is for a little bit more specific condition, think about ERR_INVALID_BUFFER_ALIGNMENT, but this is just a suggestion. ERR_INVALID_BUFFER_SIZE might be useful for more situations in the future, so it's Ok too.
@starkwang it's your PR, so you should decide 👍

if (len < 192) {
for (var i = 0; i < len; i += 4) {
swap(this, i, i + 3);
Expand All @@ -1482,7 +1483,7 @@ Buffer.prototype.swap64 = function swap64() {
// dropping down to the native code is faster.
const len = this.length;
if (len % 8 !== 0)
throw new RangeError('Buffer size must be a multiple of 64-bits');
throw new errors.RangeError('ERR_INVALID_BUFFER_SIZE', '64-bits');
if (len < 192) {
for (var i = 0; i < len; i += 8) {
swap(this, i, i + 7);
Expand Down
37 changes: 35 additions & 2 deletions lib/internal/errors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,11 @@ module.exports = exports = {
// Note: Please try to keep these in alphabetical order
E('ERR_ARG_NOT_ITERABLE', '%s must be iterable');
E('ERR_ASSERTION', '%s');
E('ERR_BUFFER_OUT_OF_BOUNDS', bufferOutOfBounds);
E('ERR_CONSOLE_WRITABLE_STREAM',
'Console expects a writable stream instance for %s');
E('ERR_CPU_USAGE', 'Unable to obtain cpu usage %s');
E('ERR_NO_LONGER_SUPPORTED', '%s is no longer supported');
E('ERR_FALSY_VALUE_REJECTION', 'Promise was rejected with falsy value');
E('ERR_HTTP_HEADERS_SENT',
'Cannot render headers after they are sent to the client');
Expand All @@ -127,6 +129,7 @@ E('ERR_INVALID_ARRAY_LENGTH',
return `The "${name}" array must have a length of ${
length}. Received length ${actual}`;
});
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_CURSOR_POS',
Expand Down Expand Up @@ -173,6 +176,7 @@ 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_UNKNOWN_ENCODING', 'Unknown encoding: %s');
E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s');
E('ERR_UNKNOWN_STDIN_TYPE', 'Unknown stdin file type');
E('ERR_UNKNOWN_STREAM_TYPE', 'Unknown stream file type');
Expand All @@ -183,8 +187,29 @@ E('ERR_V8BREAKITERATOR', 'Full ICU data not installed. ' +
function invalidArgType(name, expected, actual) {
const assert = lazyAssert();
assert(name, 'name is required');
const type = name.includes('.') ? 'property' : 'argument';
var msg = `The "${name}" ${type} must be ${oneOf(expected, 'type')}`;

// determiner: 'must be' or 'must not be'
let determiner;
if (expected.includes('not ')) {
determiner = 'must not be';
expected = expected.split('not ')[1];
} else {
determiner = 'must be';
}

let msg;
if (Array.isArray(name)) {
var names = name.map((val) => `"${val}"`).join(', ');
msg = `The ${names} arguments ${determiner} ${oneOf(expected, 'type')}`;
} else if (name.includes(' argument')) {
// for the case like 'first argument'
msg = `The ${name} ${determiner} ${oneOf(expected, 'type')}`;
} else {
const type = name.includes('.') ? 'property' : 'argument';
msg = `The "${name}" ${type} ${determiner} ${oneOf(expected, 'type')}`;
}

// if actual value received, output it
if (arguments.length >= 3) {
msg += `. Received type ${actual !== null ? typeof actual : 'null'}`;
}
Expand Down Expand Up @@ -231,3 +256,11 @@ function oneOf(expected, thing) {
return `of ${thing} ${String(expected)}`;
}
}

function bufferOutOfBounds(name, isWriting) {
if (isWriting) {
return 'Attempt to write outside buffer bounds';
} else {
return `"${name}" is outside of buffer bounds`;
}
}
Loading