Permalink
Browse files

Revert "fs: make callback mandatory to all async functions"

This reverts commit 9359de9.

Original Commit Message:

    The "fs" module has two functions called `maybeCallback` and
    `makeCallback`, as of now.

    The `maybeCallback` creates a default function to report errors, if the
    parameter passed is not a function object. Basically, if the callback
    is omitted in some cases, this function is used to create a default
    callback function.

    The `makeCallback`, OTOH, creates a default function only if the
    parameter passed is `undefined`, and if it is not a function object it
    will throw an `Error`.

    This patch removes the `maybeCallback` function and makes the callback
    function argument mandatory for all the async functions.

    PR-URL: #7168
    Reviewed-By: Trevor Norris <trev.norris@gmail.com>

PR-URL: #7846
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information...
MylesBorins authored and addaleax committed Jul 22, 2016
1 parent 1a9e247 commit 21b0a27af8b0a171f0a3a2a365259706bccfe1a5
View
142 lib/fs.js

Large diffs are not rendered by default.

Oops, something went wrong.
@@ -0,0 +1 @@
require('fs').readFile('/'); // throws EISDIR
@@ -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);
}
});
});
@@ -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/
);
@@ -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));
@@ -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-'), {}));

This file was deleted.

Oops, something went wrong.
@@ -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));
}));
@@ -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}`);
@@ -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() {});

0 comments on commit 21b0a27

Please sign in to comment.