Skip to content

Commit 9f122e3

Browse files
committed
fs: throw fs.close errors in JS
* Collect the error context in both JS and C++, then throw the error in JS * Test that the errors thrown from fs.close and fs.closeSync includes the correct error code, error number and syscall properties PR-URL: #17338 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
1 parent 6ca10de commit 9f122e3

File tree

3 files changed

+61
-6
lines changed

3 files changed

+61
-6
lines changed

lib/fs.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,11 @@ fs.closeSync = function(fd) {
661661
if (!isUint32(fd))
662662
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'fd', 'integer');
663663

664-
return binding.close(fd);
664+
const ctx = {};
665+
binding.close(fd, undefined, ctx);
666+
if (ctx.errno !== undefined) {
667+
throw new errors.uvException(ctx);
668+
}
665669
};
666670

667671
function modeNum(m, def) {

src/node_file.cc

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -429,15 +429,23 @@ void Access(const FunctionCallbackInfo<Value>& args) {
429429

430430
void Close(const FunctionCallbackInfo<Value>& args) {
431431
Environment* env = Environment::GetCurrent(args);
432+
Local<Context> context = env->context();
432433

434+
int length = args.Length();
435+
CHECK_GE(length, 2);
433436
CHECK(args[0]->IsInt32());
434437

435-
int fd = args[0]->Int32Value();
438+
int fd = static_cast<int>(args[0]->Int32Value(context).FromJust());
436439

437-
if (args[1]->IsObject()) {
438-
ASYNC_CALL(AfterNoArgs, close, args[1], UTF8, fd)
439-
} else {
440-
SYNC_CALL(close, 0, fd)
440+
if (args[1]->IsObject()) { // close(fd, req)
441+
Local<Object> req_obj = args[1]->ToObject(context).ToLocalChecked();
442+
FSReqWrap* req_wrap = AsyncCall(
443+
env, req_obj, UTF8, "close", AfterNoArgs, uv_fs_close, fd);
444+
if (req_wrap != nullptr) {
445+
args.GetReturnValue().Set(req_wrap->persistent());
446+
}
447+
} else { // close(fd, undefined, ctx)
448+
SyncCall(env, args[2], "close", uv_fs_close, fd);
441449
}
442450
}
443451

test/parallel/test-fs-close-errors.js

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
'use strict';
22

3+
// This tests that the errors thrown from fs.close and fs.closeSync
4+
// include the desired properties
5+
36
const common = require('../common');
7+
const assert = require('assert');
48
const fs = require('fs');
9+
const uv = process.binding('uv');
510

611
['', false, null, undefined, {}, []].forEach((i) => {
712
common.expectsError(
@@ -21,3 +26,41 @@ const fs = require('fs');
2126
}
2227
);
2328
});
29+
30+
{
31+
assert.throws(
32+
() => {
33+
const fd = fs.openSync(__filename, 'r');
34+
fs.closeSync(fd);
35+
fs.closeSync(fd);
36+
},
37+
(err) => {
38+
assert.strictEqual(err.code, 'EBADF');
39+
assert.strictEqual(
40+
err.message,
41+
'EBADF: bad file descriptor, close'
42+
);
43+
assert.strictEqual(err.constructor, Error);
44+
assert.strictEqual(err.syscall, 'close');
45+
assert.strictEqual(err.errno, uv.UV_EBADF);
46+
return true;
47+
}
48+
);
49+
}
50+
51+
{
52+
const fd = fs.openSync(__filename, 'r');
53+
fs.close(fd, common.mustCall((err) => {
54+
assert.ifError(err);
55+
fs.close(fd, common.mustCall((err) => {
56+
assert.strictEqual(err.code, 'EBADF');
57+
assert.strictEqual(
58+
err.message,
59+
'EBADF: bad file descriptor, close'
60+
);
61+
assert.strictEqual(err.constructor, Error);
62+
assert.strictEqual(err.syscall, 'close');
63+
assert.strictEqual(err.errno, uv.UV_EBADF);
64+
}));
65+
}));
66+
}

0 commit comments

Comments
 (0)