diff --git a/lib/fs.js b/lib/fs.js index 558e9ffa19f7ee..e3bfdabe885734 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -38,6 +38,7 @@ const O_WRONLY = constants.O_WRONLY || 0; const isWindows = process.platform === 'win32'; +const DEBUG = process.env.NODE_DEBUG && /fs/.test(process.env.NODE_DEBUG); const errnoException = util._errnoException; var printDeprecation; @@ -81,10 +82,39 @@ function throwOptionsError(options) { 'but got ' + typeof options + ' instead'); } +function rethrow() { + // Only enable in debug mode. A backtrace uses ~1000 bytes of heap space and + // is fairly slow to generate. + if (DEBUG) { + var backtrace = new Error(); + return function(err) { + if (err) { + backtrace.stack = err.name + ': ' + err.message + + backtrace.stack.substr(backtrace.name.length); + throw backtrace; + } + }; + } + + return function(err) { + if (err) { + throw err; // Forgot a callback but don't know where? Use NODE_DEBUG=fs + } + }; +} + +function maybeCallback(cb) { + return typeof cb === 'function' ? cb : rethrow(); +} + // Ensure that callbacks run in the global context. Only use this function // for callbacks that are passed to the binding layer, callbacks that are // invoked from JS already run in the proper scope. function makeCallback(cb) { + if (cb === undefined) { + return rethrow(); + } + if (typeof cb !== 'function') { throw new TypeError('"callback" argument must be a function'); } @@ -191,9 +221,11 @@ fs.Stats.prototype.isSocket = function() { }); fs.access = function(path, mode, callback) { - callback = makeCallback(arguments[arguments.length - 1]); if (typeof mode === 'function') { + callback = mode; mode = fs.F_OK; + } else if (typeof callback !== 'function') { + throw new TypeError('"callback" argument must be a function'); } if (!nullCheck(path, callback)) @@ -201,7 +233,7 @@ fs.access = function(path, mode, callback) { mode = mode | 0; var req = new FSReqWrap(); - req.oncomplete = callback; + req.oncomplete = makeCallback(callback); binding.access(pathModule._makeLong(path), mode, req); }; @@ -217,13 +249,12 @@ fs.accessSync = function(path, mode) { }; fs.exists = function(path, callback) { - callback = makeCallback(arguments[arguments.length - 1]); if (!nullCheck(path, cb)) return; var req = new FSReqWrap(); req.oncomplete = cb; binding.stat(pathModule._makeLong(path), req); function cb(err, stats) { - callback(err ? false : true); + if (callback) callback(err ? false : true); } }; @@ -237,8 +268,8 @@ fs.existsSync = function(path) { } }; -fs.readFile = function(path, options, callback) { - callback = makeCallback(arguments[arguments.length - 1]); +fs.readFile = function(path, options, callback_) { + var callback = maybeCallback(arguments[arguments.length - 1]); if (!options || typeof options === 'function') { options = { encoding: null, flag: 'r' }; @@ -570,7 +601,7 @@ Object.defineProperty(exports, '_stringToFlags', { fs.close = function(fd, callback) { var req = new FSReqWrap(); - req.oncomplete = makeCallback(arguments[arguments.length - 1]); + req.oncomplete = makeCallback(callback); binding.close(fd, req); }; @@ -588,8 +619,8 @@ function modeNum(m, def) { return undefined; } -fs.open = function(path, flags, mode, callback) { - callback = makeCallback(arguments[arguments.length - 1]); +fs.open = function(path, flags, mode, callback_) { + var callback = makeCallback(arguments[arguments.length - 1]); mode = modeNum(mode, 0o666); if (!nullCheck(path, callback)) return; @@ -611,7 +642,6 @@ fs.openSync = function(path, flags, mode) { var readWarned = false; fs.read = function(fd, buffer, offset, length, position, callback) { - callback = makeCallback(arguments[arguments.length - 1]); if (!(buffer instanceof Buffer)) { // legacy string interface (fd, length, position, encoding, callback) readWarned = printDeprecation('fs.read\'s legacy String interface ' + @@ -635,20 +665,20 @@ fs.read = function(fd, buffer, offset, length, position, callback) { if (bytesRead > 0) { tryToStringWithEnd(buffer, encoding, bytesRead, cb); } else { - cb(err, '', bytesRead); + (cb)(err, '', bytesRead); } }; } if (length === 0) { return process.nextTick(function() { - callback(null, 0, buffer); + callback && callback(null, 0, buffer); }); } function wrapper(err, bytesRead) { // Retain a reference to buffer so that it can't be GC'ed too soon. - callback(err, bytesRead || 0, buffer); + callback && callback(err, bytesRead || 0, buffer); } var req = new FSReqWrap(); @@ -712,7 +742,6 @@ fs.readSync = function(fd, buffer, offset, length, position) { // OR // fs.write(fd, string[, position[, encoding]], callback); fs.write = function(fd, buffer, offset, length, position, callback) { - callback = makeCallback(arguments[arguments.length - 1]); function wrapper(err, written) { // Retain a reference to buffer so that it can't be GC'ed too soon. callback(err, written || 0, buffer); @@ -724,8 +753,10 @@ fs.write = function(fd, buffer, offset, length, position, callback) { if (buffer instanceof Buffer) { // if no position is passed then assume null if (typeof position === 'function') { + callback = position; position = null; } + callback = maybeCallback(callback); return binding.writeBuffer(fd, buffer, offset, length, position, req); } @@ -740,6 +771,7 @@ fs.write = function(fd, buffer, offset, length, position, callback) { } length = 'utf8'; } + callback = maybeCallback(position); return binding.writeString(fd, buffer, offset, length, req); }; @@ -761,7 +793,7 @@ fs.writeSync = function(fd, buffer, offset, length, position) { }; fs.rename = function(oldPath, newPath, callback) { - callback = makeCallback(arguments[arguments.length - 1]); + callback = makeCallback(callback); if (!nullCheck(oldPath, callback)) return; if (!nullCheck(newPath, callback)) return; var req = new FSReqWrap(); @@ -782,13 +814,14 @@ fs.truncate = function(path, len, callback) { if (typeof path === 'number') { return fs.ftruncate(path, len, callback); } - - callback = makeCallback(arguments[arguments.length - 1]); - - if (typeof len === 'function' || len === undefined) { + if (typeof len === 'function') { + callback = len; + len = 0; + } else if (len === undefined) { len = 0; } + callback = maybeCallback(callback); fs.open(path, 'r+', function(er, fd) { if (er) return callback(er); var req = new FSReqWrap(); @@ -822,11 +855,14 @@ fs.truncateSync = function(path, len) { }; fs.ftruncate = function(fd, len, callback) { - if (typeof len === 'function' || len === undefined) { + if (typeof len === 'function') { + callback = len; + len = 0; + } else if (len === undefined) { len = 0; } var req = new FSReqWrap(); - req.oncomplete = makeCallback(arguments[arguments.length - 1]); + req.oncomplete = makeCallback(callback); binding.ftruncate(fd, len, req); }; @@ -838,7 +874,7 @@ fs.ftruncateSync = function(fd, len) { }; fs.rmdir = function(path, callback) { - callback = makeCallback(arguments[arguments.length - 1]); + callback = maybeCallback(callback); if (!nullCheck(path, callback)) return; var req = new FSReqWrap(); req.oncomplete = callback; @@ -862,7 +898,7 @@ fs.fdatasyncSync = function(fd) { fs.fsync = function(fd, callback) { var req = new FSReqWrap(); - req.oncomplete = makeCallback(arguments[arguments.length - 1]); + req.oncomplete = makeCallback(callback); binding.fsync(fd, req); }; @@ -871,7 +907,8 @@ fs.fsyncSync = function(fd) { }; fs.mkdir = function(path, mode, callback) { - callback = makeCallback(arguments[arguments.length - 1]); + if (typeof mode === 'function') callback = mode; + callback = makeCallback(callback); if (!nullCheck(path, callback)) return; var req = new FSReqWrap(); req.oncomplete = callback; @@ -887,9 +924,9 @@ fs.mkdirSync = function(path, mode) { }; fs.readdir = function(path, options, callback) { - callback = makeCallback(arguments[arguments.length - 1]); options = options || {}; if (typeof options === 'function') { + callback = options; options = {}; } else if (typeof options === 'string') { options = {encoding: options}; @@ -897,6 +934,7 @@ fs.readdir = function(path, options, callback) { if (typeof options !== 'object') throw new TypeError('"options" must be a string or an object'); + callback = makeCallback(callback); if (!nullCheck(path, callback)) return; var req = new FSReqWrap(); req.oncomplete = callback; @@ -915,12 +953,12 @@ fs.readdirSync = function(path, options) { fs.fstat = function(fd, callback) { var req = new FSReqWrap(); - req.oncomplete = makeCallback(arguments[arguments.length - 1]); + req.oncomplete = makeCallback(callback); binding.fstat(fd, req); }; fs.lstat = function(path, callback) { - callback = makeCallback(arguments[arguments.length - 1]); + callback = makeCallback(callback); if (!nullCheck(path, callback)) return; var req = new FSReqWrap(); req.oncomplete = callback; @@ -928,7 +966,7 @@ fs.lstat = function(path, callback) { }; fs.stat = function(path, callback) { - callback = makeCallback(arguments[arguments.length - 1]); + callback = makeCallback(callback); if (!nullCheck(path, callback)) return; var req = new FSReqWrap(); req.oncomplete = callback; @@ -950,15 +988,16 @@ fs.statSync = function(path) { }; fs.readlink = function(path, options, callback) { - callback = makeCallback(arguments[arguments.length - 1]); options = options || {}; if (typeof options === 'function') { + callback = options; options = {}; } else if (typeof options === 'string') { options = {encoding: options}; } if (typeof options !== 'object') throw new TypeError('"options" must be a string or an object'); + callback = makeCallback(callback); if (!nullCheck(path, callback)) return; var req = new FSReqWrap(); req.oncomplete = callback; @@ -1018,7 +1057,7 @@ fs.symlinkSync = function(target, path, type) { }; fs.link = function(srcpath, dstpath, callback) { - callback = makeCallback(arguments[arguments.length - 1]); + callback = makeCallback(callback); if (!nullCheck(srcpath, callback)) return; if (!nullCheck(dstpath, callback)) return; @@ -1038,7 +1077,7 @@ fs.linkSync = function(srcpath, dstpath) { }; fs.unlink = function(path, callback) { - callback = makeCallback(arguments[arguments.length - 1]); + callback = makeCallback(callback); if (!nullCheck(path, callback)) return; var req = new FSReqWrap(); req.oncomplete = callback; @@ -1052,7 +1091,7 @@ fs.unlinkSync = function(path) { fs.fchmod = function(fd, mode, callback) { var req = new FSReqWrap(); - req.oncomplete = makeCallback(arguments[arguments.length - 1]); + req.oncomplete = makeCallback(callback); binding.fchmod(fd, modeNum(mode), req); }; @@ -1062,7 +1101,7 @@ fs.fchmodSync = function(fd, mode) { if (constants.hasOwnProperty('O_SYMLINK')) { fs.lchmod = function(path, mode, callback) { - callback = makeCallback(arguments[arguments.length - 1]); + callback = maybeCallback(callback); fs.open(path, constants.O_WRONLY | constants.O_SYMLINK, function(err, fd) { if (err) { callback(err); @@ -1101,7 +1140,7 @@ if (constants.hasOwnProperty('O_SYMLINK')) { fs.chmod = function(path, mode, callback) { - callback = makeCallback(arguments[arguments.length - 1]); + callback = makeCallback(callback); if (!nullCheck(path, callback)) return; var req = new FSReqWrap(); req.oncomplete = callback; @@ -1117,7 +1156,7 @@ fs.chmodSync = function(path, mode) { if (constants.hasOwnProperty('O_SYMLINK')) { fs.lchown = function(path, uid, gid, callback) { - callback = makeCallback(arguments[arguments.length - 1]); + callback = maybeCallback(callback); fs.open(path, constants.O_WRONLY | constants.O_SYMLINK, function(err, fd) { if (err) { callback(err); @@ -1135,7 +1174,7 @@ if (constants.hasOwnProperty('O_SYMLINK')) { fs.fchown = function(fd, uid, gid, callback) { var req = new FSReqWrap(); - req.oncomplete = makeCallback(arguments[arguments.length - 1]); + req.oncomplete = makeCallback(callback); binding.fchown(fd, uid, gid, req); }; @@ -1144,7 +1183,7 @@ fs.fchownSync = function(fd, uid, gid) { }; fs.chown = function(path, uid, gid, callback) { - callback = makeCallback(arguments[arguments.length - 1]); + callback = makeCallback(callback); if (!nullCheck(path, callback)) return; var req = new FSReqWrap(); req.oncomplete = callback; @@ -1178,7 +1217,7 @@ function toUnixTimestamp(time) { fs._toUnixTimestamp = toUnixTimestamp; fs.utimes = function(path, atime, mtime, callback) { - callback = makeCallback(arguments[arguments.length - 1]); + callback = makeCallback(callback); if (!nullCheck(path, callback)) return; var req = new FSReqWrap(); req.oncomplete = callback; @@ -1196,11 +1235,10 @@ fs.utimesSync = function(path, atime, mtime) { }; fs.futimes = function(fd, atime, mtime, callback) { - callback = makeCallback(arguments[arguments.length - 1]); atime = toUnixTimestamp(atime); mtime = toUnixTimestamp(mtime); var req = new FSReqWrap(); - req.oncomplete = callback; + req.oncomplete = makeCallback(callback); binding.futimes(fd, atime, mtime, req); }; @@ -1210,8 +1248,8 @@ fs.futimesSync = function(fd, atime, mtime) { binding.futimes(fd, atime, mtime); }; -function writeAll(fd, isUserFd, buffer, offset, length, position, callback) { - callback = makeCallback(arguments[arguments.length - 1]); +function writeAll(fd, isUserFd, buffer, offset, length, position, callback_) { + var callback = maybeCallback(arguments[arguments.length - 1]); // write(fd, buffer, offset, length, position, callback) fs.write(fd, buffer, offset, length, position, function(writeErr, written) { @@ -1242,8 +1280,8 @@ function writeAll(fd, isUserFd, buffer, offset, length, position, callback) { }); } -fs.writeFile = function(path, data, options, callback) { - callback = makeCallback(arguments[arguments.length - 1]); +fs.writeFile = function(path, data, options, callback_) { + var callback = maybeCallback(arguments[arguments.length - 1]); if (!options || typeof options === 'function') { options = { encoding: 'utf8', mode: 0o666, flag: 'w' }; @@ -1314,8 +1352,8 @@ fs.writeFileSync = function(path, data, options) { } }; -fs.appendFile = function(path, data, options, callback) { - callback = makeCallback(arguments[arguments.length - 1]); +fs.appendFile = function(path, data, options, callback_) { + var callback = maybeCallback(arguments[arguments.length - 1]); if (!options || typeof options === 'function') { options = { encoding: 'utf8', mode: 0o666, flag: 'a' }; @@ -1470,7 +1508,6 @@ StatWatcher.prototype.stop = function() { const statWatchers = new Map(); fs.watchFile = function(filename, options, listener) { - listener = makeCallback(arguments[arguments.length - 1]); nullCheck(filename); filename = pathModule.resolve(filename); var stat; @@ -1486,9 +1523,14 @@ fs.watchFile = function(filename, options, listener) { if (options !== null && typeof options === 'object') { options = util._extend(defaults, options); } else { + listener = options; options = defaults; } + if (typeof listener !== 'function') { + throw new Error('"watchFile()" requires a listener function'); + } + stat = statWatchers.get(filename); if (stat === undefined) { @@ -1534,16 +1576,17 @@ fs.realpathSync = function realpathSync(path, options) { fs.realpath = function realpath(path, options, callback) { - callback = makeCallback(arguments[arguments.length - 1]); if (!options) { options = {}; } else if (typeof options === 'function') { + callback = options; options = {}; } else if (typeof options === 'string') { options = {encoding: options}; } else if (typeof options !== 'object') { throw new TypeError('"options" must be a string or an object'); } + callback = makeCallback(callback); if (!nullCheck(path, callback)) return; var req = new FSReqWrap(); @@ -1554,12 +1597,12 @@ fs.realpath = function realpath(path, options, callback) { fs.mkdtemp = function(prefix, options, callback) { - callback = makeCallback(arguments[arguments.length - 1]); if (!prefix || typeof prefix !== 'string') throw new TypeError('filename prefix is required'); options = options || {}; if (typeof options === 'function') { + callback = options; options = {}; } else if (typeof options === 'string') { options = {encoding: options}; @@ -1567,6 +1610,7 @@ fs.mkdtemp = function(prefix, options, callback) { if (typeof options !== 'object') throw new TypeError('"options" must be a string or an object'); + callback = makeCallback(callback); if (!nullCheck(prefix, callback)) { return; } diff --git a/test/fixtures/test-fs-readfile-error.js b/test/fixtures/test-fs-readfile-error.js new file mode 100644 index 00000000000000..0638d01ac7655a --- /dev/null +++ b/test/fixtures/test-fs-readfile-error.js @@ -0,0 +1 @@ +require('fs').readFile('/'); // throws EISDIR diff --git a/test/parallel/test-fs-chmod.js b/test/parallel/test-fs-chmod.js index 1564734ec64441..954916cbdbb365 100644 --- a/test/parallel/test-fs-chmod.js +++ b/test/parallel/test-fs-chmod.js @@ -101,7 +101,7 @@ fs.open(file2, 'a', function(err, fd) { assert.equal(mode_sync, fs.fstatSync(fd).mode & 0o777); } success_count++; - fs.closeSync(fd); + fs.close(fd); } }); }); diff --git a/test/parallel/test-fs-link.js b/test/parallel/test-fs-link.js index 89d4c25d5bb78e..2cba47bfec83df 100644 --- a/test/parallel/test-fs-link.js +++ b/test/parallel/test-fs-link.js @@ -23,14 +23,14 @@ fs.link(srcPath, dstPath, common.mustCall(callback)); assert.throws( function() { - fs.link(undefined, undefined, () => {}); + fs.link(); }, /src must be a string or Buffer/ ); assert.throws( function() { - fs.link('abc', undefined, () => {}); + fs.link('abc'); }, /dest must be a string or Buffer/ ); diff --git a/test/parallel/test-fs-make-callback.js b/test/parallel/test-fs-make-callback.js index 8ded34e2b088fd..4fbe64437eaff0 100644 --- a/test/parallel/test-fs-make-callback.js +++ b/test/parallel/test-fs-make-callback.js @@ -13,6 +13,9 @@ function test(cb) { // Verify the case where a callback function is provided assert.doesNotThrow(test(function() {})); +// Passing undefined calls rethrow() internally, which is fine +assert.doesNotThrow(test(undefined)); + // Anything else should throw assert.throws(test(null)); assert.throws(test(true)); diff --git a/test/parallel/test-fs-mkdtemp.js b/test/parallel/test-fs-mkdtemp.js index 60d1196c5a5631..dd4ab75c22c7aa 100644 --- a/test/parallel/test-fs-mkdtemp.js +++ b/test/parallel/test-fs-mkdtemp.js @@ -29,3 +29,8 @@ fs.mkdtemp(path.join(common.tmpDir, 'bar.'), common.mustCall(handler)); // Same test as above, but making sure that passing an options object doesn't // affect the way the callback function is handled. fs.mkdtemp(path.join(common.tmpDir, 'bar.'), {}, common.mustCall(handler)); + +// Making sure that not passing a callback doesn't crash, as a default function +// is passed internally. +assert.doesNotThrow(() => fs.mkdtemp(path.join(common.tmpDir, 'bar-'))); +assert.doesNotThrow(() => fs.mkdtemp(path.join(common.tmpDir, 'bar-'), {})); diff --git a/test/parallel/test-fs-no-callback-errors.js b/test/parallel/test-fs-no-callback-errors.js deleted file mode 100644 index 0b2bc0d2e3fbb9..00000000000000 --- a/test/parallel/test-fs-no-callback-errors.js +++ /dev/null @@ -1,17 +0,0 @@ -'use strict'; - -require('../common'); -const fs = require('fs'); -const assert = require('assert'); - -Object.getOwnPropertyNames(fs).filter( - (d) => !d.endsWith('Stream') && // ignore stream creation functions - !d.endsWith('Sync') && // ignore synchronous functions - typeof fs[d] === 'function' && // ignore anything other than functions - d.indexOf('_') === -1 && // ignore internal use functions - !/^[A-Z]/.test(d) && // ignore conventional constructors - d !== 'watch' && // watch's callback is optional - d !== 'unwatchFile' // unwatchFile's callback is optional -).forEach( - (d) => assert.throws(() => fs[d](), /"callback" argument must be a function/) -); diff --git a/test/parallel/test-fs-readfile-error.js b/test/parallel/test-fs-readfile-error.js new file mode 100644 index 00000000000000..86a2be4e1bc020 --- /dev/null +++ b/test/parallel/test-fs-readfile-error.js @@ -0,0 +1,34 @@ +'use strict'; +var common = require('../common'); +var assert = require('assert'); +var exec = require('child_process').exec; +var path = require('path'); + +// `fs.readFile('/')` does not fail on FreeBSD, because you can open and read +// the directory there. +if (process.platform === 'freebsd') { + common.skip('platform not supported.'); + return; +} + +function test(env, cb) { + var filename = path.join(common.fixturesDir, 'test-fs-readfile-error.js'); + var execPath = '"' + process.execPath + '" "' + filename + '"'; + var options = { env: Object.assign(process.env, env) }; + exec(execPath, options, function(err, stdout, stderr) { + assert(err); + assert.equal(stdout, ''); + assert.notEqual(stderr, ''); + cb('' + stderr); + }); +} + +test({ NODE_DEBUG: '' }, common.mustCall(function(data) { + assert(/EISDIR/.test(data)); + assert(!/test-fs-readfile-error/.test(data)); +})); + +test({ NODE_DEBUG: 'fs' }, common.mustCall(function(data) { + assert(/EISDIR/.test(data)); + assert(/test-fs-readfile-error/.test(data)); +})); diff --git a/test/parallel/test-fs-stat.js b/test/parallel/test-fs-stat.js index 081ace714becaf..ac68a9ae42c389 100644 --- a/test/parallel/test-fs-stat.js +++ b/test/parallel/test-fs-stat.js @@ -28,7 +28,7 @@ fs.open('.', 'r', undefined, common.mustCall(function(err, fd) { fs.fstat(fd, common.mustCall(function(err, stats) { assert.ifError(err); assert.ok(stats.mtime instanceof Date); - fs.closeSync(fd); + fs.close(fd); assert(this === global); })); @@ -47,7 +47,7 @@ fs.open('.', 'r', undefined, common.mustCall(function(err, fd) { console.dir(stats); assert.ok(stats.mtime instanceof Date); } - fs.closeSync(fd); + fs.close(fd); })); console.log(`stating: ${__filename}`); diff --git a/test/parallel/test-fs-watchfile.js b/test/parallel/test-fs-watchfile.js index 270bf270d7fa04..d30261859c39b7 100644 --- a/test/parallel/test-fs-watchfile.js +++ b/test/parallel/test-fs-watchfile.js @@ -8,11 +8,11 @@ const assert = require('assert'); // Basic usage tests. assert.throws(function() { fs.watchFile('./some-file'); -}, /"callback" argument must be a function/); +}, /"watchFile\(\)" requires a listener function/); assert.throws(function() { fs.watchFile('./another-file', {}, 'bad listener'); -}, /"callback" argument must be a function/); +}, /"watchFile\(\)" requires a listener function/); assert.throws(function() { fs.watchFile(new Object(), function() {});