Skip to content

Commit

Permalink
buffer: improve error messages
Browse files Browse the repository at this point in the history
Some errors in buffer module losed some arguments or received
wrong arguments when they were created. This PR added these
losing arguments and fixed the wrong arguments.

PR-URL: nodejs/node#14975
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
starkwang authored and addaleax committed Aug 25, 2017
1 parent 1b33e4a commit dc16c54
Show file tree
Hide file tree
Showing 9 changed files with 117 additions and 100 deletions.
18 changes: 12 additions & 6 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,8 @@ Buffer.from = function from(value, encodingOrOffset, length) {
throw new errors.TypeError(
'ERR_INVALID_ARG_TYPE',
'first argument',
['string', 'buffer', 'arrayBuffer', 'array', 'array-like object']
['string', 'buffer', 'arrayBuffer', 'array', 'array-like object'],
value
);
};

Expand Down Expand Up @@ -507,7 +508,8 @@ function byteLength(string, encoding) {
}

throw new errors.TypeError(
'ERR_INVALID_ARG_TYPE', 'string', ['string', 'buffer', 'arrayBuffer']
'ERR_INVALID_ARG_TYPE', 'string',
['string', 'buffer', 'arrayBuffer'], string
);
}

Expand Down Expand Up @@ -668,7 +670,8 @@ Buffer.prototype.toString = function toString(encoding, start, end) {
Buffer.prototype.equals = function equals(b) {
if (!isUint8Array(b)) {
throw new errors.TypeError(
'ERR_INVALID_ARG_TYPE', 'otherBuffer', ['buffer', 'uint8Array']
'ERR_INVALID_ARG_TYPE', 'otherBuffer',
['buffer', 'uint8Array'], b
);
}
if (this === b)
Expand Down Expand Up @@ -696,7 +699,8 @@ Buffer.prototype.compare = function compare(target,
thisEnd) {
if (!isUint8Array(target)) {
throw new errors.TypeError(
'ERR_INVALID_ARG_TYPE', 'target', ['buffer', 'uint8Array']
'ERR_INVALID_ARG_TYPE', 'target',
['buffer', 'uint8Array'], target
);
}
if (arguments.length === 1)
Expand Down Expand Up @@ -778,7 +782,8 @@ function bidirectionalIndexOf(buffer, val, byteOffset, encoding, dir) {
}

throw new errors.TypeError(
'ERR_INVALID_ARG_TYPE', 'val', ['string', 'buffer', 'uint8Array']
'ERR_INVALID_ARG_TYPE', 'value',
['string', 'buffer', 'uint8Array'], val
);
}

Expand Down Expand Up @@ -847,7 +852,8 @@ Buffer.prototype.fill = function fill(val, start, end, encoding) {
}

if (encoding !== undefined && typeof encoding !== 'string') {
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'encoding', 'string');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'encoding',
'string', encoding);
}
var normalizedEncoding = normalizeEncoding(encoding);
if (normalizedEncoding === undefined) {
Expand Down
27 changes: 16 additions & 11 deletions test/parallel/test-buffer-bytelength.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,22 @@ const assert = require('assert');
const SlowBuffer = require('buffer').SlowBuffer;
const vm = require('vm');

// coerce values to string
const errMsg = common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "string" argument must be one of type string, ' +
'buffer, or arrayBuffer'
}, 4);
assert.throws(() => { Buffer.byteLength(32, 'latin1'); }, errMsg);
assert.throws(() => { Buffer.byteLength(NaN, 'utf8'); }, errMsg);
assert.throws(() => { Buffer.byteLength({}, 'latin1'); }, errMsg);
assert.throws(() => { Buffer.byteLength(); }, errMsg);
[
[32, 'latin1'],
[NaN, 'utf8'],
[{}, 'latin1'],
[]
].forEach((args) => {
common.expectsError(
() => Buffer.byteLength(...args),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "string" argument must be one of type string, ' +
`buffer, or arrayBuffer. Received type ${typeof args[0]}`
}
);
});

assert.strictEqual(Buffer.byteLength('', undefined, true), -1);

Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-buffer-compare-offset.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,6 @@ assert.throws(() => a.compare(b, -Infinity, Infinity), oor);
assert.throws(() => a.compare(), common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "target" argument must be one of type buffer or uint8Array'
message: 'The "target" argument must be one of ' +
'type buffer or uint8Array. Received type undefined'
}));
3 changes: 2 additions & 1 deletion test/parallel/test-buffer-compare.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,6 @@ assert.throws(() => Buffer.compare('abc', Buffer.alloc(1)), errMsg);
assert.throws(() => Buffer.alloc(1).compare('abc'), common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "target" argument must be one of type buffer or uint8Array'
message: 'The "target" argument must be one of ' +
'type buffer or uint8Array. Received type string'
}));
16 changes: 9 additions & 7 deletions test/parallel/test-buffer-equals.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ assert.ok(!d.equals(e));
assert.ok(d.equals(d));
assert.ok(d.equals(new Uint8Array([0x61, 0x62, 0x63, 0x64, 0x65])));

assert.throws(() => Buffer.alloc(1).equals('abc'),
common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "otherBuffer" argument must be one of type ' +
'buffer or uint8Array'
}));
common.expectsError(
() => Buffer.alloc(1).equals('abc'),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "otherBuffer" argument must be one of type ' +
'buffer or uint8Array. Received type string'
}
);
66 changes: 35 additions & 31 deletions test/parallel/test-buffer-fill.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,45 +192,49 @@ deepStrictEqualValues(genBuffer(4, [hexBufFill, 1, -1]), [0, 0, 0, 0]);


// Check exceptions
assert.throws(
() => buf1.fill(0, -1),
common.expectsError({ code: 'ERR_INDEX_OUT_OF_RANGE' }));
assert.throws(
() => buf1.fill(0, 0, buf1.length + 1),
common.expectsError({ code: 'ERR_INDEX_OUT_OF_RANGE' }));
assert.throws(
() => buf1.fill('', -1),
common.expectsError({ code: 'ERR_INDEX_OUT_OF_RANGE' }));
assert.throws(
() => buf1.fill('', 0, buf1.length + 1),
common.expectsError({ code: 'ERR_INDEX_OUT_OF_RANGE' }));
assert.throws(
[
[0, -1],
[0, 0, buf1.length + 1],
['', -1],
['', 0, buf1.length + 1]
].forEach((args) => {
common.expectsError(
() => buf1.fill(...args),
{ code: 'ERR_INDEX_OUT_OF_RANGE' }
);
});

common.expectsError(
() => buf1.fill('a', 0, buf1.length, 'node rocks!'),
common.expectsError({
{
code: 'ERR_UNKNOWN_ENCODING',
type: TypeError,
message: 'Unknown encoding: node rocks!'
}));
assert.throws(
() => buf1.fill('a', 0, 0, NaN),
common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
message: 'The "encoding" argument must be of type string'
}));
assert.throws(
() => buf1.fill('a', 0, 0, null),
common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
message: 'The "encoding" argument must be of type string'
}));
assert.throws(
}
);

[
['a', 0, 0, NaN],
['a', 0, 0, null]
].forEach((args) => {
common.expectsError(
() => buf1.fill(...args),
{
code: 'ERR_INVALID_ARG_TYPE',
message: 'The "encoding" argument must be of type ' +
`string. Received type ${args[3] === null ? 'null' : typeof args[3]}`
}
);
});

common.expectsError(
() => buf1.fill('a', 0, 0, 'foo'),
common.expectsError({
{
code: 'ERR_UNKNOWN_ENCODING',
type: TypeError,
message: 'Unknown encoding: foo'
}));

}
);

function genBuffer(size, args) {
const b = Buffer.allocUnsafe(size);
Expand Down
17 changes: 9 additions & 8 deletions test/parallel/test-buffer-from.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,19 @@ deepStrictEqual(
);

[
{},
new Boolean(true),
{ valueOf() { return null; } },
{ valueOf() { return undefined; } },
{ valueOf: null },
Object.create(null)
].forEach((input) => {
[{}, 'object'],
[new Boolean(true), 'boolean'],
[{ valueOf() { return null; } }, 'object'],
[{ valueOf() { return undefined; } }, 'object'],
[{ valueOf: null }, 'object'],
[Object.create(null), 'object']
].forEach(([input, actualType]) => {
const err = common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The first argument must be one of type string, buffer, ' +
'arrayBuffer, array, or array-like object'
'arrayBuffer, array, or array-like object. Received ' +
`type ${actualType}`
});
throws(() => Buffer.from(input), err);
});
Expand Down
34 changes: 17 additions & 17 deletions test/parallel/test-buffer-includes.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ assert(twoByteString.includes(
assert(!twoByteString.includes('\u03a3', -2, 'ucs2'));

const mixedByteStringUcs2 =
Buffer.from('\u039a\u0391abc\u03a3\u03a3\u0395', 'ucs2');
Buffer.from('\u039a\u0391abc\u03a3\u03a3\u0395', 'ucs2');
assert(mixedByteStringUcs2.includes('bc', 0, 'ucs2'));
assert(mixedByteStringUcs2.includes('\u03a3', 0, 'ucs2'));
assert(!mixedByteStringUcs2.includes('\u0396', 0, 'ucs2'));
Expand Down Expand Up @@ -261,7 +261,7 @@ for (let lengthIndex = 0; lengthIndex < lengths.length; lengthIndex++) {
const length = lengths[lengthIndex];

const patternBufferUcs2 =
allCharsBufferUcs2.slice(index, index + length);
allCharsBufferUcs2.slice(index, index + length);
assert.ok(
allCharsBufferUcs2.includes(patternBufferUcs2, 0, 'ucs2'));

Expand All @@ -271,21 +271,21 @@ for (let lengthIndex = 0; lengthIndex < lengths.length; lengthIndex++) {
}
}

const expectedError = common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "val" argument must be one of type ' +
'string, buffer, or uint8Array'
}, 3);
assert.throws(() => {
b.includes(() => {});
}, expectedError);
assert.throws(() => {
b.includes({});
}, expectedError);
assert.throws(() => {
b.includes([]);
}, expectedError);
[
() => { },
{},
[]
].forEach((val) => {
common.expectsError(
() => b.includes(val),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "value" argument must be one of type string, ' +
`buffer, or uint8Array. Received type ${typeof val}`
}
);
});

// test truncation of Number arguments to uint8
{
Expand Down
33 changes: 15 additions & 18 deletions test/parallel/test-buffer-indexof.js
Original file line number Diff line number Diff line change
Expand Up @@ -344,24 +344,21 @@ assert.strictEqual(Buffer.from('aaaaa').indexOf('b', 'ucs2'), -1);
}
}

const argumentExpected = common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "val" argument must be one of type ' +
'string, buffer, or uint8Array'
}, 3);

assert.throws(() => {
b.indexOf(() => { });
}, argumentExpected);

assert.throws(() => {
b.indexOf({});
}, argumentExpected);

assert.throws(() => {
b.indexOf([]);
}, argumentExpected);
[
() => {},
{},
[]
].forEach((val) => {
common.expectsError(
() => b.indexOf(val),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "value" argument must be one of type string, ' +
`buffer, or uint8Array. Received type ${typeof val}`
}
);
});

// Test weird offset arguments.
// The following offsets coerce to NaN or 0, searching the whole Buffer
Expand Down

0 comments on commit dc16c54

Please sign in to comment.