Skip to content
This repository has been archived by the owner on Aug 31, 2018. It is now read-only.

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 @addaleax
starkwang authored and addaleax committed Aug 28, 2017
1 parent 784bab7 commit 18cabcc
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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 18cabcc

Please sign in to comment.