From 6bd8b7e5405e1cdc9f56214f5f6b741806c32e5f Mon Sep 17 00:00:00 2001 From: isaacs Date: Wed, 13 Mar 2013 14:59:42 -0700 Subject: [PATCH] fs: Missing cb errors are deprecated, not a throw Commit a804347 makes fs function rethrow errors when the callback is omitted. While the right thing to do, it's a change from the old v0.8 behavior where such errors were silently ignored. To give users time to upgrade, temporarily disable that and replace it with a function that warns once about the deprecated behavior. Close #5005 --- lib/fs.js | 34 +++++++++++++++++++-------- test/simple/test-fs-readfile-error.js | 11 ++++++++- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 3d37a648a7..49bb027376 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -59,22 +59,36 @@ var DEBUG = process.env.NODE_DEBUG && /fs/.test(process.env.NODE_DEBUG); function rethrow() { // Only enable in debug mode. A backtrace uses ~1000 bytes of heap space and // is fairly slow to generate. + var callback; if (DEBUG) { var backtrace = new Error; - return function(err) { - if (err) { - backtrace.message = err.message; - err = backtrace; - throw err; - } - }; + callback = debugCallback; + } else + callback = missingCallback; + + return callback; + + function debugCallback(err) { + if (err) { + backtrace.message = err.message; + err = backtrace; + missingCallback(err); + } } - return function(err) { + function missingCallback(err) { if (err) { - throw err; // Forgot a callback but don't know where? Use NODE_DEBUG=fs + if (process.throwDeprecation) + throw err; // Forgot a callback but don't know where? Use NODE_DEBUG=fs + else if (!process.noDeprecation) { + var msg = 'fs: missing callback ' + (err.stack || err.message); + if (process.traceDeprecation) + console.trace(msg); + else + console.error(msg); + } } - }; + } } function maybeCallback(cb) { diff --git a/test/simple/test-fs-readfile-error.js b/test/simple/test-fs-readfile-error.js index 72e1e2e7fb..97d2f06a61 100644 --- a/test/simple/test-fs-readfile-error.js +++ b/test/simple/test-fs-readfile-error.js @@ -28,7 +28,7 @@ var callbacks = 0; function test(env, cb) { var filename = path.join(common.fixturesDir, 'test-fs-readfile-error.js'); - var execPath = process.execPath + ' ' + filename; + var execPath = process.execPath + ' --throw-deprecation ' + filename; var options = { env: env || {} }; exec(execPath, options, function(err, stdout, stderr) { assert(err); @@ -53,3 +53,12 @@ test({ NODE_DEBUG: 'fs' }, function(data) { process.on('exit', function() { assert.equal(callbacks, 2); }); + +(function() { + console.error('the warnings are normal here.'); + // just make sure that this doesn't crash the process. + var fs = require('fs'); + fs.readFile(__dirname); + fs.readdir(__filename); + fs.unlink('gee-i-sure-hope-this-file-isnt-important-or-existing'); +})();