Skip to content

Commit

Permalink
src: remove 2nd undefined argument in node_file.cc
Browse files Browse the repository at this point in the history
In the document for fs, there are several functions that state "No
arguments other than a possible exception are given to the completion
callback." (ex> fs.access, fs.chmod, fs.close, ..)

But, the functions are invoking the callback with two parameters (err,
undefined)

It causes problems in using several API like
[async.waterfall](https://caolan.github.io/async/docs.html#waterfall).

PR-URL: #20629
Fixes: #20335
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
dankang authored and MylesBorins committed May 22, 2018
1 parent 83119db commit 3edb04d
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 4 deletions.
4 changes: 3 additions & 1 deletion src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,9 @@ void FSReqWrap::Resolve(Local<Value> value) {
Null(env()->isolate()),
value
};
MakeCallback(env()->oncomplete_string(), arraysize(argv), argv);
MakeCallback(env()->oncomplete_string(),
value->IsUndefined() ? 1 : arraysize(argv),
argv);
}

void FSReqWrap::SetReturnValue(const FunctionCallbackInfo<Value>& args) {
Expand Down
12 changes: 9 additions & 3 deletions test/parallel/test-fs-access.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,21 @@ assert.strictEqual(typeof fs.X_OK, 'number');

const throwNextTick = (e) => { process.nextTick(() => { throw e; }); };

fs.access(__filename, common.mustCall(assert.ifError));
fs.access(__filename, common.mustCall(function(...args) {
assert.deepStrictEqual(args, [null]);
}));
fs.promises.access(__filename)
.then(common.mustCall())
.catch(throwNextTick);
fs.access(__filename, fs.R_OK, common.mustCall(assert.ifError));
fs.access(__filename, fs.R_OK, common.mustCall(function(...args) {
assert.deepStrictEqual(args, [null]);
}));
fs.promises.access(__filename, fs.R_OK)
.then(common.mustCall())
.catch(throwNextTick);
fs.access(readOnlyFile, fs.F_OK | fs.R_OK, common.mustCall(assert.ifError));
fs.access(readOnlyFile, fs.F_OK | fs.R_OK, common.mustCall(function(...args) {
assert.deepStrictEqual(args, [null]);
}));
fs.promises.access(readOnlyFile, fs.F_OK | fs.R_OK)
.then(common.mustCall())
.catch(throwNextTick);
Expand Down
12 changes: 12 additions & 0 deletions test/parallel/test-fs-close.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use strict';

const common = require('../common');

const assert = require('assert');
const fs = require('fs');

const fd = fs.openSync(__filename, 'r');

fs.close(fd, common.mustCall(function(...args) {
assert.deepStrictEqual(args, [null]);
}));

0 comments on commit 3edb04d

Please sign in to comment.