Permalink
Browse files

fs: fix 'object is not a function' callback errors

Use a default callback if the user omitted one. Avoids errors like the one
below:

  fs.js:777
      if (err) return callback(err);
                      ^
  TypeError: object is not a function
          at fs.appendFile (fs.js:777:21)
          at Object.oncomplete (fs.js:297:15)

This commit fixes the behavior of fs.lchmod(), fs.lchown() and fs.readFile()
when the callback is omitted. Before, they silently swallowed errors.

Fixes #4352.
  • Loading branch information...
1 parent 03b00dc commit a80434736bce22a9ac00376bb5786806752ef3dd @bnoordhuis bnoordhuis committed Dec 4, 2012
Showing with 57 additions and 29 deletions.
  1. +25 −2 doc/api/fs.markdown
  2. +32 −27 lib/fs.js
View
@@ -59,8 +59,31 @@ In busy processes, the programmer is _strongly encouraged_ to use the
asynchronous versions of these calls. The synchronous versions will block
the entire process until they complete--halting all connections.
-Relative path to filename can be used, remember however that this path will be relative
-to `process.cwd()`.
+Relative path to filename can be used, remember however that this path will be
+relative to `process.cwd()`.
+
+Most fs functions let you omit the callback argument. If you do, a default
+callback is used that rethrows errors. To get a trace to the original call
+site, set the NODE_DEBUG environment variable:
+
+ $ cat script.js
+ function bad() {
+ require('fs').readFile('/');
+ }
+ bad();
+
+ $ env NODE_DEBUG=fs node script.js
+ fs.js:66
+ throw err;
+ ^
+ Error: EISDIR, read
+ at rethrow (fs.js:61:21)
+ at maybeCallback (fs.js:79:42)
+ at Object.fs.readFile (fs.js:153:18)
+ at bad (/path/to/script.js:2:17)
+ at Object.<anonymous> (/path/to/script.js:5:1)
+ <etc.>
+
## fs.rename(oldPath, newPath, [callback])
View
@@ -52,6 +52,27 @@ var O_WRONLY = constants.O_WRONLY || 0;
var isWindows = process.platform === 'win32';
+function rethrow(err) {
+ if (err) throw err;
+}
+
+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 (typeof cb !== 'function') {
+ return rethrow;
+ }
+
+ return function() {
+ return cb.apply(null, arguments);
+ };
+}
+
function assertEncoding(encoding) {
if (encoding && !Buffer.isEncoding(encoding)) {
throw new Error('Unknown encoding: ' + encoding);
@@ -110,8 +131,7 @@ fs.existsSync = function(path) {
fs.readFile = function(path, encoding_) {
var encoding = typeof(encoding_) === 'string' ? encoding_ : null;
- var callback = arguments[arguments.length - 1];
- if (typeof(callback) !== 'function') callback = function() {};
+ var callback = maybeCallback(arguments[arguments.length - 1]);
assertEncoding(encoding);
@@ -295,21 +315,6 @@ Object.defineProperty(exports, '_stringToFlags', {
});
-// 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 (typeof cb !== 'function') {
- // faster than returning a ref to a global no-op function
- return function() {};
- }
-
- return function() {
- return cb.apply(null, arguments);
- };
-}
-
-
// Yes, the follow could be easily DRYed up but I provide the explicit
// list to make the arguments clear.
@@ -425,9 +430,11 @@ fs.write = function(fd, buffer, offset, length, position, callback) {
return;
}
+ callback = maybeCallback(callback);
+
function wrapper(err, written) {
// Retain a reference to buffer so that it can't be GC'ed too soon.
- callback && callback(err, written || 0, buffer);
+ callback(err, written || 0, buffer);
}
binding.write(fd, buffer, offset, length, position, wrapper);
@@ -470,6 +477,7 @@ fs.truncate = function(path, len, callback) {
} else if (typeof len === 'undefined') {
len = 0;
}
+ callback = maybeCallback(callback);
fs.open(path, 'w', function(er, fd) {
if (er) return callback(er);
binding.ftruncate(fd, len, function(er) {
@@ -659,7 +667,7 @@ fs.fchmodSync = function(fd, mode) {
if (constants.hasOwnProperty('O_SYMLINK')) {
fs.lchmod = function(path, mode, callback) {
- callback = callback || (function() {});
+ callback = maybeCallback(callback);
fs.open(path, constants.O_WRONLY | constants.O_SYMLINK, function(err, fd) {
if (err) {
callback(err);
@@ -709,7 +717,7 @@ fs.chmodSync = function(path, mode) {
if (constants.hasOwnProperty('O_SYMLINK')) {
fs.lchown = function(path, uid, gid, callback) {
- callback = callback || (function() {});
+ callback = maybeCallback(callback);
fs.open(path, constants.O_WRONLY | constants.O_SYMLINK, function(err, fd) {
if (err) {
callback(err);
@@ -782,8 +790,7 @@ fs.futimesSync = function(fd, atime, mtime) {
};
function writeAll(fd, buffer, offset, length, position, callback) {
- var callback_ = arguments[arguments.length - 1];
- callback = (typeof(callback_) == 'function' ? callback_ : null);
+ callback = maybeCallback(arguments[arguments.length - 1]);
// write(fd, buffer, offset, length, position, callback)
fs.write(fd, buffer, offset, length, position, function(writeErr, written) {
@@ -808,8 +815,7 @@ fs.writeFile = function(path, data, encoding_, callback) {
var encoding = (typeof(encoding_) == 'string' ? encoding_ : 'utf8');
assertEncoding(encoding);
- var callback_ = arguments[arguments.length - 1];
- callback = (typeof(callback_) == 'function' ? callback_ : null);
+ callback = maybeCallback(arguments[arguments.length - 1]);
fs.open(path, 'w', 438 /*=0666*/, function(openErr, fd) {
if (openErr) {
if (callback) callback(openErr);
@@ -843,8 +849,7 @@ fs.appendFile = function(path, data, encoding_, callback) {
var encoding = (typeof(encoding_) == 'string' ? encoding_ : 'utf8');
assertEncoding(encoding);
- var callback_ = arguments[arguments.length - 1];
- callback = (typeof(callback_) == 'function' ? callback_ : null);
+ callback = maybeCallback(arguments[arguments.length - 1]);
fs.open(path, 'a', 438 /*=0666*/, function(err, fd) {
if (err) return callback(err);
@@ -1158,7 +1163,7 @@ fs.realpathSync = function realpathSync(p, cache) {
fs.realpath = function realpath(p, cache, cb) {
if (typeof cb !== 'function') {
- cb = cache;
+ cb = maybeCallback(cache);
cache = null;
}

0 comments on commit a804347

Please sign in to comment.