Skip to content

Commit

Permalink
fs: throw fs.close errors in JS
Browse files Browse the repository at this point in the history
* 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 <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
joyeecheung committed Dec 27, 2017
1 parent 6ca10de commit 9f122e3
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 6 deletions.
6 changes: 5 additions & 1 deletion lib/fs.js
Expand Up @@ -661,7 +661,11 @@ fs.closeSync = function(fd) {
if (!isUint32(fd))
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'fd', 'integer');

return binding.close(fd);
const ctx = {};
binding.close(fd, undefined, ctx);
if (ctx.errno !== undefined) {
throw new errors.uvException(ctx);
}
};

function modeNum(m, def) {
Expand Down
18 changes: 13 additions & 5 deletions src/node_file.cc
Expand Up @@ -429,15 +429,23 @@ void Access(const FunctionCallbackInfo<Value>& args) {

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

int length = args.Length();
CHECK_GE(length, 2);
CHECK(args[0]->IsInt32());

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

if (args[1]->IsObject()) {
ASYNC_CALL(AfterNoArgs, close, args[1], UTF8, fd)
} else {
SYNC_CALL(close, 0, fd)
if (args[1]->IsObject()) { // close(fd, req)
Local<Object> req_obj = args[1]->ToObject(context).ToLocalChecked();
FSReqWrap* req_wrap = AsyncCall(
env, req_obj, UTF8, "close", AfterNoArgs, uv_fs_close, fd);
if (req_wrap != nullptr) {
args.GetReturnValue().Set(req_wrap->persistent());
}
} else { // close(fd, undefined, ctx)
SyncCall(env, args[2], "close", uv_fs_close, fd);
}
}

Expand Down
43 changes: 43 additions & 0 deletions test/parallel/test-fs-close-errors.js
@@ -1,7 +1,12 @@
'use strict';

// This tests that the errors thrown from fs.close and fs.closeSync
// include the desired properties

const common = require('../common');
const assert = require('assert');
const fs = require('fs');
const uv = process.binding('uv');

['', false, null, undefined, {}, []].forEach((i) => {
common.expectsError(
Expand All @@ -21,3 +26,41 @@ const fs = require('fs');
}
);
});

{
assert.throws(
() => {
const fd = fs.openSync(__filename, 'r');
fs.closeSync(fd);
fs.closeSync(fd);
},
(err) => {
assert.strictEqual(err.code, 'EBADF');
assert.strictEqual(
err.message,
'EBADF: bad file descriptor, close'
);
assert.strictEqual(err.constructor, Error);
assert.strictEqual(err.syscall, 'close');
assert.strictEqual(err.errno, uv.UV_EBADF);
return true;
}
);
}

{
const fd = fs.openSync(__filename, 'r');
fs.close(fd, common.mustCall((err) => {
assert.ifError(err);
fs.close(fd, common.mustCall((err) => {
assert.strictEqual(err.code, 'EBADF');
assert.strictEqual(
err.message,
'EBADF: bad file descriptor, close'
);
assert.strictEqual(err.constructor, Error);
assert.strictEqual(err.syscall, 'close');
assert.strictEqual(err.errno, uv.UV_EBADF);
}));
}));
}

0 comments on commit 9f122e3

Please sign in to comment.