From 9f122e3b5513fd354b3876d06ea322b676b7350d Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Wed, 29 Nov 2017 12:52:16 +0800
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/17338
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
---
 lib/fs.js                             |  6 +++-
 src/node_file.cc                      | 18 +++++++----
 test/parallel/test-fs-close-errors.js | 43 +++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/lib/fs.js b/lib/fs.js
index f357ff2b9d7691..5cde94ec516b73 100644
--- a/lib/fs.js
+++ b/lib/fs.js
@@ -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) {
diff --git a/src/node_file.cc b/src/node_file.cc
index 3373ad8707e225..3f680c14d64702 100644
--- a/src/node_file.cc
+++ b/src/node_file.cc
@@ -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);
   }
 }
 
diff --git a/test/parallel/test-fs-close-errors.js b/test/parallel/test-fs-close-errors.js
index cadcf2f78c4aa4..4f0d31369a2651 100644
--- a/test/parallel/test-fs-close-errors.js
+++ b/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(
@@ -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);
+    }));
+  }));
+}