-
Notifications
You must be signed in to change notification settings - Fork 29.3k
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.
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
lib/fs: convert to using internal/errors #15043
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,7 @@ const { isUint8Array, createPromise, promiseResolve } = process.binding('util'); | |
const binding = process.binding('fs'); | ||
const fs = exports; | ||
const Buffer = require('buffer').Buffer; | ||
const errors = require('internal/errors'); | ||
const Stream = require('stream').Stream; | ||
const EventEmitter = require('events'); | ||
const FSReqWrap = binding.FSReqWrap; | ||
|
@@ -72,8 +73,10 @@ function getOptions(options, defaultOptions) { | |
defaultOptions.encoding = options; | ||
options = defaultOptions; | ||
} else if (typeof options !== 'object') { | ||
throw new TypeError('"options" must be a string or an object, got ' + | ||
typeof options + ' instead.'); | ||
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', | ||
'options', | ||
['string', 'object'], | ||
options); | ||
} | ||
|
||
if (options.encoding !== 'buffer') | ||
|
@@ -128,7 +131,7 @@ function makeCallback(cb) { | |
} | ||
|
||
if (typeof cb !== 'function') { | ||
throw new TypeError('"callback" argument must be a function'); | ||
throw new errors.TypeError('ERR_INVALID_CALLBACK'); | ||
} | ||
|
||
return function() { | ||
|
@@ -145,7 +148,7 @@ function makeStatsCallback(cb) { | |
} | ||
|
||
if (typeof cb !== 'function') { | ||
throw new TypeError('"callback" argument must be a function'); | ||
throw new errors.TypeError('ERR_INVALID_CALLBACK'); | ||
} | ||
|
||
return function(err) { | ||
|
@@ -156,8 +159,11 @@ function makeStatsCallback(cb) { | |
|
||
function nullCheck(path, callback) { | ||
if (('' + path).indexOf('\u0000') !== -1) { | ||
var er = new Error('Path must be a string without null bytes'); | ||
er.code = 'ENOENT'; | ||
const er = new errors.Error('ERR_INVALID_ARG_TYPE', | ||
'path', | ||
'string without null bytes', | ||
path); | ||
|
||
if (typeof callback !== 'function') | ||
throw er; | ||
process.nextTick(callback, er); | ||
|
@@ -274,7 +280,7 @@ fs.access = function(path, mode, callback) { | |
callback = mode; | ||
mode = fs.F_OK; | ||
} 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)) | ||
|
@@ -1193,7 +1199,10 @@ function toUnixTimestamp(time) { | |
// convert to 123.456 UNIX timestamp | ||
return time.getTime() / 1000; | ||
} | ||
throw new Error('Cannot parse time: ' + time); | ||
throw new errors.Error('ERR_INVALID_ARG_TYPE', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
tl;dr 3rd arg should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2.You are right I will go with ['Date', 'time in seconds'] There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I updated it to |
||
'time', | ||
['Date', 'time in seconds'], | ||
time); | ||
} | ||
|
||
// exported for unit tests, not for public consumption | ||
|
@@ -1495,7 +1504,10 @@ fs.watchFile = function(filename, options, listener) { | |
} | ||
|
||
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); | ||
|
@@ -1842,8 +1854,12 @@ fs.realpath = function realpath(p, options, callback) { | |
fs.mkdtemp = function(prefix, options, callback) { | ||
callback = makeCallback(typeof options === 'function' ? options : callback); | ||
options = getOptions(options, {}); | ||
if (!prefix || typeof prefix !== 'string') | ||
throw new TypeError('filename prefix is required'); | ||
if (!prefix || typeof prefix !== 'string') { | ||
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', | ||
'prefix', | ||
'string', | ||
prefix); | ||
} | ||
if (!nullCheck(prefix, callback)) { | ||
return; | ||
} | ||
|
@@ -1856,8 +1872,12 @@ fs.mkdtemp = function(prefix, options, callback) { | |
|
||
|
||
fs.mkdtempSync = function(prefix, options) { | ||
if (!prefix || typeof prefix !== 'string') | ||
throw new TypeError('filename prefix is required'); | ||
if (!prefix || typeof prefix !== 'string') { | ||
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', | ||
'prefix', | ||
'string', | ||
prefix); | ||
} | ||
options = getOptions(options, {}); | ||
nullCheck(prefix); | ||
return binding.mkdtemp(prefix + 'XXXXXX', options.encoding); | ||
|
@@ -1903,16 +1923,26 @@ function ReadStream(path, options) { | |
|
||
if (this.start !== undefined) { | ||
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) { | ||
this.end = Infinity; | ||
} 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) { | ||
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; | ||
|
@@ -2069,10 +2099,17 @@ function WriteStream(path, options) { | |
|
||
if (this.start !== undefined) { | ||
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) { | ||
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; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO this could be changed to
TypeError
.Feedback anyone?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's really difficult to say. The coercion to a string means that any value can be used, it's not really a
TypeError
. The real check here is the null-character bit, which doesn't feel like aTypeError
to me. I could live with it tho.