Skip to content

Commit

Permalink
fs: do not pass Buffer when toString() fails
Browse files Browse the repository at this point in the history
Even though an Error object is passed to the callback when readFile()
fails due to toString() failing, it is a bit strange to still see
data passed as the second argument. This commit changes that and only
passes the Error object in that case.

PR-URL: #9670
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Prince John Wesley <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
mscdex committed Dec 14, 2016
1 parent 4f97a14 commit f3cf8e9
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 7 deletions.
11 changes: 4 additions & 7 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,8 @@ function readFileAfterClose(err) {
var buffer = null;
var callback = context.callback;

if (context.err)
return callback(context.err);
if (context.err || err)
return callback(context.err || err);

if (context.size === 0)
buffer = Buffer.concat(context.buffers, context.pos);
Expand All @@ -406,8 +406,6 @@ function readFileAfterClose(err) {
else
buffer = context.buffer;

if (err) return callback(err, buffer);

if (context.encoding) {
return tryToString(buffer, context.encoding, callback);
}
Expand All @@ -416,13 +414,12 @@ function readFileAfterClose(err) {
}

function tryToString(buf, encoding, callback) {
var e = null;
try {
buf = buf.toString(encoding);
} catch (err) {
e = err;
return callback(err);
}
callback(e, buf);
callback(null, buf);
}

function tryStatSync(fd, isUserFd) {
Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-fs-readfile-tostring-fail.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ stream.on('finish', common.mustCall(function() {
fs.readFile(file, 'utf8', common.mustCall(function(err, buf) {
assert.ok(err instanceof Error);
assert.strictEqual('"toString()" failed', err.message);
assert.strictEqual(buf, undefined);
}));
}));

Expand Down

0 comments on commit f3cf8e9

Please sign in to comment.