errors: convert 'fs'

covert lib/fs.js over to using lib/internal/errors.js
i have not addressed the cases that use errnoException(),
for reasons described in GH-12926

- throw the ERR_INVALID_CALLBACK error
  when the the callback is invalid
- replace the ['object', 'string'] with
  ['string', 'object'] in the error constructor call,
  to better match the previous err msg
  in the getOptions() function
- add error ERR_VALUE_OUT_OF_RANGE in lib/internal/errors.js,
  this error is thrown when a numeric value is out of range
- document the ERR_VALUE_OUT_OF_RANGE err in errors.md
- correct the expected args, in the error thrown in the function
  fs._toUnixTimestamp() to ['Date', 'time in seconds'] (lib/fs.js)
- update the listener error type in the fs.watchFile() function,
  from Error to TypeError (lib/fs.js)
- update errors from ERR_INVALID_OPT_VALUE to ERR_INVALID_ARG_TYPE
  in the functions fs.ReadStream() and fs.WriteStream(),
  for the cases of range errors use the new error:
  ERR_VALUE_OUT_OF_RANGE (lib/fs.js)

PR-URL: https://github.com/nodejs/node/pull/15043
Refs: https://github.com/nodejs/node/issues/11273
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
This commit is contained in:
matzavinos 2017-08-26 07:46:47 -04:00 committed by Refael Ackermann
parent a517466aa7
commit 219932a9f7
No known key found for this signature in database
GPG Key ID: CD704BD80FDDDB64
17 changed files with 244 additions and 114 deletions

View File

@ -1136,6 +1136,11 @@ Used when an attempt is made to launch a Node.js process with an unknown
by errors in user code, although it is not impossible. Occurrences of this error by errors in user code, although it is not impossible. Occurrences of this error
are most likely an indication of a bug within Node.js itself. are most likely an indication of a bug within Node.js itself.
<a id="ERR_VALUE_OUT_OF_RANGE"></a>
### ERR_VALUE_OUT_OF_RANGE
Used when a number value is out of range.
<a id="ERR_V8BREAKITERATOR"></a> <a id="ERR_V8BREAKITERATOR"></a>
### ERR_V8BREAKITERATOR ### ERR_V8BREAKITERATOR

View File

@ -33,6 +33,7 @@ const { isUint8Array, createPromise, promiseResolve } = process.binding('util');
const binding = process.binding('fs'); const binding = process.binding('fs');
const fs = exports; const fs = exports;
const Buffer = require('buffer').Buffer; const Buffer = require('buffer').Buffer;
const errors = require('internal/errors');
const Stream = require('stream').Stream; const Stream = require('stream').Stream;
const EventEmitter = require('events'); const EventEmitter = require('events');
const FSReqWrap = binding.FSReqWrap; const FSReqWrap = binding.FSReqWrap;
@ -72,8 +73,10 @@ function getOptions(options, defaultOptions) {
defaultOptions.encoding = options; defaultOptions.encoding = options;
options = defaultOptions; options = defaultOptions;
} else if (typeof options !== 'object') { } else if (typeof options !== 'object') {
throw new TypeError('"options" must be a string or an object, got ' + throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
typeof options + ' instead.'); 'options',
['string', 'object'],
options);
} }
if (options.encoding !== 'buffer') if (options.encoding !== 'buffer')
@ -128,7 +131,7 @@ function makeCallback(cb) {
} }
if (typeof cb !== 'function') { if (typeof cb !== 'function') {
throw new TypeError('"callback" argument must be a function'); throw new errors.TypeError('ERR_INVALID_CALLBACK');
} }
return function() { return function() {
@ -145,7 +148,7 @@ function makeStatsCallback(cb) {
} }
if (typeof cb !== 'function') { if (typeof cb !== 'function') {
throw new TypeError('"callback" argument must be a function'); throw new errors.TypeError('ERR_INVALID_CALLBACK');
} }
return function(err) { return function(err) {
@ -156,8 +159,11 @@ function makeStatsCallback(cb) {
function nullCheck(path, callback) { function nullCheck(path, callback) {
if (('' + path).indexOf('\u0000') !== -1) { if (('' + path).indexOf('\u0000') !== -1) {
var er = new Error('Path must be a string without null bytes'); const er = new errors.Error('ERR_INVALID_ARG_TYPE',
er.code = 'ENOENT'; 'path',
'string without null bytes',
path);
if (typeof callback !== 'function') if (typeof callback !== 'function')
throw er; throw er;
process.nextTick(callback, er); process.nextTick(callback, er);
@ -274,7 +280,7 @@ fs.access = function(path, mode, callback) {
callback = mode; callback = mode;
mode = fs.F_OK; mode = fs.F_OK;
} else if (typeof callback !== 'function') { } else if (typeof callback !== 'function') {
throw new TypeError('"callback" argument must be a function'); throw new errors.TypeError('ERR_INVALID_CALLBACK');
} }
if (handleError((path = getPathFromURL(path)), callback)) if (handleError((path = getPathFromURL(path)), callback))
@ -1193,7 +1199,10 @@ function toUnixTimestamp(time) {
// convert to 123.456 UNIX timestamp // convert to 123.456 UNIX timestamp
return time.getTime() / 1000; return time.getTime() / 1000;
} }
throw new Error('Cannot parse time: ' + time); throw new errors.Error('ERR_INVALID_ARG_TYPE',
'time',
['Date', 'time in seconds'],
time);
} }
// exported for unit tests, not for public consumption // exported for unit tests, not for public consumption
@ -1495,7 +1504,10 @@ fs.watchFile = function(filename, options, listener) {
} }
if (typeof listener !== 'function') { if (typeof listener !== 'function') {
throw new Error('"watchFile()" requires a listener function'); throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
'listener',
'function',
listener);
} }
stat = statWatchers.get(filename); stat = statWatchers.get(filename);
@ -1842,8 +1854,12 @@ fs.realpath = function realpath(p, options, callback) {
fs.mkdtemp = function(prefix, options, callback) { fs.mkdtemp = function(prefix, options, callback) {
callback = makeCallback(typeof options === 'function' ? options : callback); callback = makeCallback(typeof options === 'function' ? options : callback);
options = getOptions(options, {}); options = getOptions(options, {});
if (!prefix || typeof prefix !== 'string') if (!prefix || typeof prefix !== 'string') {
throw new TypeError('filename prefix is required'); throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
'prefix',
'string',
prefix);
}
if (!nullCheck(prefix, callback)) { if (!nullCheck(prefix, callback)) {
return; return;
} }
@ -1856,8 +1872,12 @@ fs.mkdtemp = function(prefix, options, callback) {
fs.mkdtempSync = function(prefix, options) { fs.mkdtempSync = function(prefix, options) {
if (!prefix || typeof prefix !== 'string') if (!prefix || typeof prefix !== 'string') {
throw new TypeError('filename prefix is required'); throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
'prefix',
'string',
prefix);
}
options = getOptions(options, {}); options = getOptions(options, {});
nullCheck(prefix); nullCheck(prefix);
return binding.mkdtemp(prefix + 'XXXXXX', options.encoding); return binding.mkdtemp(prefix + 'XXXXXX', options.encoding);
@ -1903,16 +1923,26 @@ function ReadStream(path, options) {
if (this.start !== undefined) { if (this.start !== undefined) {
if (typeof this.start !== 'number') { if (typeof this.start !== 'number') {
throw new TypeError('"start" option must be a Number'); throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
'start',
'number',
this.start);
} }
if (this.end === undefined) { if (this.end === undefined) {
this.end = Infinity; this.end = Infinity;
} else if (typeof this.end !== 'number') { } else if (typeof this.end !== 'number') {
throw new TypeError('"end" option must be a Number'); throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
'end',
'number',
this.end);
} }
if (this.start > this.end) { if (this.start > this.end) {
throw new Error('"start" option must be <= "end" option'); const errVal = `{start: ${this.start}, end: ${this.end}}`;
throw new errors.RangeError('ERR_VALUE_OUT_OF_RANGE',
'start',
'<= "end"',
errVal);
} }
this.pos = this.start; this.pos = this.start;
@ -2069,10 +2099,17 @@ function WriteStream(path, options) {
if (this.start !== undefined) { if (this.start !== undefined) {
if (typeof this.start !== 'number') { if (typeof this.start !== 'number') {
throw new TypeError('"start" option must be a Number'); throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
'start',
'number',
this.start);
} }
if (this.start < 0) { if (this.start < 0) {
throw new Error('"start" must be >= zero'); const errVal = `{start: ${this.start}}`;
throw new errors.RangeError('ERR_VALUE_OUT_OF_RANGE',
'start',
'>= 0',
errVal);
} }
this.pos = this.start; this.pos = this.start;

View File

@ -264,6 +264,9 @@ E('ERR_UNKNOWN_ENCODING', 'Unknown encoding: %s');
E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s'); E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s');
E('ERR_UNKNOWN_STDIN_TYPE', 'Unknown stdin file type'); E('ERR_UNKNOWN_STDIN_TYPE', 'Unknown stdin file type');
E('ERR_UNKNOWN_STREAM_TYPE', 'Unknown stream file type'); E('ERR_UNKNOWN_STREAM_TYPE', 'Unknown stream file type');
E('ERR_VALUE_OUT_OF_RANGE', (start, end, value) => {
return `The value of "${start}" must be ${end}. Received "${value}"`;
});
E('ERR_V8BREAKITERATOR', 'Full ICU data not installed. ' + E('ERR_V8BREAKITERATOR', 'Full ICU data not installed. ' +
'See https://github.com/nodejs/node/wiki/Intl'); 'See https://github.com/nodejs/node/wiki/Intl');
E('ERR_VALID_PERFORMANCE_ENTRY_TYPE', E('ERR_VALID_PERFORMANCE_ENTRY_TYPE',

View File

@ -176,12 +176,15 @@ function run_test_3() {
const run_test_4 = common.mustCall(function() { const run_test_4 = common.mustCall(function() {
// Error: start must be >= zero // Error: start must be >= zero
assert.throws( const block = () => {
function() {
fs.createWriteStream(filepath, { start: -5, flags: 'r+' }); fs.createWriteStream(filepath, { start: -5, flags: 'r+' });
}, };
/"start" must be/ const err = {
); code: 'ERR_VALUE_OUT_OF_RANGE',
message: 'The value of "start" must be >= 0. Received "{start: -5}"',
type: RangeError
};
common.expectsError(block, err);
}); });
run_test_1(); run_test_1();

View File

@ -85,13 +85,23 @@ assert.throws(() => {
fs.access(100, fs.F_OK, common.mustNotCall()); fs.access(100, fs.F_OK, common.mustNotCall());
}, /^TypeError: path must be a string or Buffer$/); }, /^TypeError: path must be a string or Buffer$/);
assert.throws(() => { common.expectsError(
() => {
fs.access(__filename, fs.F_OK); fs.access(__filename, fs.F_OK);
}, /^TypeError: "callback" argument must be a function$/); },
{
code: 'ERR_INVALID_CALLBACK',
type: TypeError
});
assert.throws(() => { common.expectsError(
() => {
fs.access(__filename, fs.F_OK, {}); fs.access(__filename, fs.F_OK, {});
}, /^TypeError: "callback" argument must be a function$/); },
{
code: 'ERR_INVALID_CALLBACK',
type: TypeError
});
assert.doesNotThrow(() => { assert.doesNotThrow(() => {
fs.accessSync(__filename); fs.accessSync(__filename);

View File

@ -2,7 +2,6 @@
const common = require('../common'); const common = require('../common');
const assert = require('assert'); const assert = require('assert');
const fs = require('fs'); const fs = require('fs');
const cbTypeError = /^TypeError: "callback" argument must be a function$/;
const callbackThrowValues = [null, true, false, 0, 1, 'foo', /foo/, [], {}]; const callbackThrowValues = [null, true, false, 0, 1, 'foo', /foo/, [], {}];
const { sep } = require('path'); const { sep } = require('path');
@ -24,7 +23,10 @@ assert.doesNotThrow(testMakeCallback());
function invalidCallbackThrowsTests() { function invalidCallbackThrowsTests() {
callbackThrowValues.forEach((value) => { callbackThrowValues.forEach((value) => {
assert.throws(testMakeCallback(value), cbTypeError); common.expectsError(testMakeCallback(value), {
code: 'ERR_INVALID_CALLBACK',
type: TypeError
});
}); });
} }

View File

@ -2,7 +2,6 @@
const common = require('../common'); const common = require('../common');
const assert = require('assert'); const assert = require('assert');
const fs = require('fs'); const fs = require('fs');
const cbTypeError = /^TypeError: "callback" argument must be a function$/;
const callbackThrowValues = [null, true, false, 0, 1, 'foo', /foo/, [], {}]; const callbackThrowValues = [null, true, false, 0, 1, 'foo', /foo/, [], {}];
const warn = 'Calling an asynchronous function without callback is deprecated.'; const warn = 'Calling an asynchronous function without callback is deprecated.';
@ -23,7 +22,10 @@ assert.doesNotThrow(testMakeStatsCallback());
function invalidCallbackThrowsTests() { function invalidCallbackThrowsTests() {
callbackThrowValues.forEach((value) => { callbackThrowValues.forEach((value) => {
assert.throws(testMakeStatsCallback(value), cbTypeError); common.expectsError(testMakeStatsCallback(value), {
code: 'ERR_INVALID_CALLBACK',
type: TypeError
});
}); });
} }

View File

@ -1,22 +1,29 @@
'use strict'; 'use strict';
const common = require('../common'); const common = require('../common');
const assert = require('assert');
const fs = require('fs'); const fs = require('fs');
const expectedError = /^TypeError: filename prefix is required$/;
const prefixValues = [undefined, null, 0, true, false, 1, '']; const prefixValues = [undefined, null, 0, true, false, 1, ''];
function fail(value) { function fail(value) {
assert.throws( common.expectsError(
() => fs.mkdtempSync(value, {}), () => {
expectedError fs.mkdtempSync(value, {});
); },
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError
});
} }
function failAsync(value) { function failAsync(value) {
assert.throws( common.expectsError(
() => fs.mkdtemp(value, common.mustNotCall()), expectedError () => {
); fs.mkdtemp(value, common.mustNotCall());
},
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError
});
} }
prefixValues.forEach((prefixValue) => { prefixValues.forEach((prefixValue) => {

View File

@ -13,20 +13,32 @@ fs.writeFileSync(tempFile, 'abc\ndef');
const sanity = 'def'; const sanity = 'def';
const saneEmitter = fs.createReadStream(tempFile, { start: 4, end: 6 }); const saneEmitter = fs.createReadStream(tempFile, { start: 4, end: 6 });
assert.throws(function() { common.expectsError(
() => {
fs.createReadStream(tempFile, { start: '4', end: 6 }); fs.createReadStream(tempFile, { start: '4', end: 6 });
}, /^TypeError: "start" option must be a Number$/, },
"start as string didn't throw an error for createReadStream"); {
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError
});
assert.throws(function() { common.expectsError(
() => {
fs.createReadStream(tempFile, { start: 4, end: '6' }); fs.createReadStream(tempFile, { start: 4, end: '6' });
}, /^TypeError: "end" option must be a Number$/, },
"end as string didn't throw an error for createReadStream"); {
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError
});
assert.throws(function() { common.expectsError(
() => {
fs.createWriteStream(tempFile, { start: '4' }); fs.createWriteStream(tempFile, { start: '4' });
}, /^TypeError: "start" option must be a Number$/, },
"start as string didn't throw an error for createWriteStream"); {
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError
});
saneEmitter.on('data', common.mustCall(function(data) { saneEmitter.on('data', common.mustCall(function(data) {
assert.strictEqual( assert.strictEqual(

View File

@ -26,17 +26,27 @@ const fs = require('fs');
const URL = require('url').URL; const URL = require('url').URL;
function check(async, sync) { function check(async, sync) {
const expected = /Path must be a string without null bytes/;
const argsSync = Array.prototype.slice.call(arguments, 2); const argsSync = Array.prototype.slice.call(arguments, 2);
const argsAsync = argsSync.concat((er) => { const argsAsync = argsSync.concat((er) => {
assert(er && expected.test(er.message)); common.expectsError(
assert.strictEqual(er.code, 'ENOENT'); () => {
throw er;
},
{
code: 'ERR_INVALID_ARG_TYPE',
type: Error
});
}); });
if (sync) { if (sync) {
assert.throws(() => { common.expectsError(
() => {
sync.apply(null, argsSync); sync.apply(null, argsSync);
}, expected); },
{
code: 'ERR_INVALID_ARG_TYPE',
type: Error,
});
} }
if (async) { if (async) {

View File

@ -108,9 +108,18 @@ const rangeFile = path.join(common.fixturesDir, 'x.txt');
} }
{ {
assert.throws(function() { const message =
'The value of "start" must be <= "end". Received "{start: 10, end: 2}"';
common.expectsError(
() => {
fs.createReadStream(rangeFile, Object.create({ start: 10, end: 2 })); fs.createReadStream(rangeFile, Object.create({ start: 10, end: 2 }));
}, /"start" option must be <= "end" option/); },
{
code: 'ERR_VALUE_OUT_OF_RANGE',
message,
type: RangeError
});
} }
{ {

View File

@ -19,16 +19,18 @@ assert.doesNotThrow(function() {
fs.createReadStream(example, { encoding: 'utf8' }); fs.createReadStream(example, { encoding: 'utf8' });
}); });
const errMessage = /"options" must be a string or an object/; const createReadStreamErr = (path, opt) => {
assert.throws(function() { common.expectsError(
fs.createReadStream(example, 123); () => {
}, errMessage); fs.createReadStream(path, opt);
assert.throws(function() { },
fs.createReadStream(example, 0); {
}, errMessage); code: 'ERR_INVALID_ARG_TYPE',
assert.throws(function() { type: TypeError
fs.createReadStream(example, true); });
}, errMessage); };
assert.throws(function() {
fs.createReadStream(example, false); createReadStreamErr(example, 123);
}, errMessage); createReadStreamErr(example, 0);
createReadStreamErr(example, true);
createReadStreamErr(example, false);

View File

@ -140,9 +140,16 @@ const rangeFile = fixtures.path('x.txt');
})); }));
} }
assert.throws(function() { common.expectsError(
() => {
fs.createReadStream(rangeFile, { start: 10, end: 2 }); fs.createReadStream(rangeFile, { start: 10, end: 2 });
}, /"start" option must be <= "end" option/); },
{
code: 'ERR_VALUE_OUT_OF_RANGE',
message:
'The value of "start" must be <= "end". Received "{start: 10, end: 2}"',
type: RangeError
});
{ {
const stream = fs.createReadStream(rangeFile, { start: 0, end: 0 }); const stream = fs.createReadStream(rangeFile, { start: 0, end: 0 });

View File

@ -1,15 +1,27 @@
'use strict'; 'use strict';
require('../common'); const common = require('../common');
const fs = require('fs'); const fs = require('fs');
const assert = require('assert'); const assert = require('assert');
[Infinity, -Infinity, NaN].forEach((input) => { [Infinity, -Infinity, NaN].forEach((input) => {
assert.throws(() => fs._toUnixTimestamp(input), common.expectsError(
new RegExp(`^Error: Cannot parse time: ${input}$`)); () => {
fs._toUnixTimestamp(input);
},
{
code: 'ERR_INVALID_ARG_TYPE',
type: Error
});
}); });
assert.throws(() => fs._toUnixTimestamp({}), common.expectsError(
/^Error: Cannot parse time: \[object Object\]$/); () => {
fs._toUnixTimestamp({});
},
{
code: 'ERR_INVALID_ARG_TYPE',
type: Error
});
const okInputs = [1, -1, '1', '-1', Date.now()]; const okInputs = [1, -1, '1', '-1', Date.now()];
okInputs.forEach((input) => { okInputs.forEach((input) => {

View File

@ -6,13 +6,23 @@ const fs = require('fs');
const path = require('path'); const path = require('path');
// Basic usage tests. // Basic usage tests.
assert.throws(function() { common.expectsError(
() => {
fs.watchFile('./some-file'); fs.watchFile('./some-file');
}, /^Error: "watchFile\(\)" requires a listener function$/); },
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError
});
assert.throws(function() { common.expectsError(
() => {
fs.watchFile('./another-file', {}, 'bad listener'); fs.watchFile('./another-file', {}, 'bad listener');
}, /^Error: "watchFile\(\)" requires a listener function$/); },
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError
});
assert.throws(function() { assert.throws(function() {
fs.watchFile(new Object(), common.mustNotCall()); fs.watchFile(new Object(), common.mustNotCall());

View File

@ -36,9 +36,14 @@ fs.readFile(httpUrl, common.expectsError({
// pct-encoded characters in the path will be decoded and checked // pct-encoded characters in the path will be decoded and checked
fs.readFile(new URL('file:///c:/tmp/%00test'), common.mustCall((err) => { fs.readFile(new URL('file:///c:/tmp/%00test'), common.mustCall((err) => {
assert(err); common.expectsError(
assert.strictEqual(err.message, () => {
'Path must be a string without null bytes'); throw err;
},
{
code: 'ERR_INVALID_ARG_TYPE',
type: Error
});
})); }));
if (common.isWindows) { if (common.isWindows) {

View File

@ -4,12 +4,6 @@ const assert = require('assert');
const fs = require('fs'); const fs = require('fs');
const path = require('path'); const path = require('path');
const numberError =
/^TypeError: "options" must be a string or an object, got number instead\.$/;
const booleanError =
/^TypeError: "options" must be a string or an object, got boolean instead\.$/;
const example = path.join(common.tmpDir, 'dummy'); const example = path.join(common.tmpDir, 'dummy');
common.refreshTmpDir(); common.refreshTmpDir();
@ -30,18 +24,18 @@ assert.doesNotThrow(() => {
fs.createWriteStream(example, { encoding: 'utf8' }); fs.createWriteStream(example, { encoding: 'utf8' });
}); });
assert.throws(() => { const createWriteStreamErr = (path, opt) => {
fs.createWriteStream(example, 123); common.expectsError(
}, numberError); () => {
fs.createWriteStream(path, opt);
},
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError
});
};
assert.throws(() => { createWriteStreamErr(example, 123);
fs.createWriteStream(example, 0); createWriteStreamErr(example, 0);
}, numberError); createWriteStreamErr(example, true);
createWriteStreamErr(example, false);
assert.throws(() => {
fs.createWriteStream(example, true);
}, booleanError);
assert.throws(() => {
fs.createWriteStream(example, false);
}, booleanError);