fs: throw fs.fstat{Sync} errors in JS

PR-URL: #17914
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

lib/fs.js

@@ -633,12 +633,11 @@ function readFileAfterClose(err) {
 }
 
 function tryStatSync(fd, isUserFd) {
-  var threw = true;
-  try {
-    binding.fstat(fd);
-    threw = false;
-  } finally {
-    if (threw && !isUserFd) fs.closeSync(fd);
+  const ctx = {};
+  binding.fstat(fd, undefined, ctx);
+  if (ctx.errno !== undefined && !isUserFd) {
+    fs.closeSync(fd);
+    throw new errors.uvException(ctx);
   }
 }
 
@@ -1115,7 +1114,11 @@ fs.stat = function(path, callback) {
 
 fs.fstatSync = function(fd) {
   validateUint32(fd, 'fd');
-  binding.fstat(fd);
+  const ctx = { fd };
+  binding.fstat(fd, undefined, ctx);
+  if (ctx.errno !== undefined) {
+    throw new errors.uvException(ctx);
+  }
   return statsFromValues();
 };
 

src/node_file.cc

@@ -587,19 +587,24 @@ static void LStat(const FunctionCallbackInfo<Value>& args) {
 
 static void FStat(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
+  Local<Context> context = env->context();
 
   CHECK(args[0]->IsInt32());
 
-  int fd = args[0]->Int32Value();
+  int fd = static_cast<int>(args[0]->Int32Value(context).FromJust());
 
   if (args[1]->IsObject()) {  // fstat(fd, req)
     CHECK_EQ(args.Length(), 2);
     AsyncCall(env, args, "fstat", UTF8, AfterStat,
               uv_fs_fstat, fd);
-  } else {  // fstat(fd)
-    SYNC_CALL(fstat, nullptr, fd)
-    FillStatsArray(env->fs_stats_field_array(),
-                   static_cast<const uv_stat_t*>(SYNC_REQ.ptr));
+  } else {  // fstat(fd, undefined, ctx)
+    CHECK_EQ(args.Length(), 3);
+    fs_req_wrap req_wrap;
+    int err = SyncCall(env, args[2], &req_wrap, "fstat", uv_fs_fstat, fd);
+    if (err == 0) {
+      FillStatsArray(env->fs_stats_field_array(),
+                     static_cast<const uv_stat_t*>(req_wrap.req.ptr));
+    }
   }
 }
 

test/parallel/test-fs-sync-fd-leak.js

@@ -23,6 +23,7 @@
 require('../common');
 const assert = require('assert');
 const fs = require('fs');
+const uv = process.binding('uv');
 
 // ensure that (read|write|append)FileSync() closes the file descriptor
 fs.openSync = function() {
@@ -39,29 +40,30 @@ fs.writeSync = function() {
   throw new Error('BAM');
 };
 
-process.binding('fs').fstat = function() {
-  throw new Error('BAM');
+process.binding('fs').fstat = function(fd, _, ctx) {
+  ctx.errno = uv.UV_EBADF;
+  ctx.syscall = 'fstat';
 };
 
 let close_called = 0;
 ensureThrows(function() {
   fs.readFileSync('dummy');
-});
+}, 'EBADF: bad file descriptor, fstat');
 ensureThrows(function() {
   fs.writeFileSync('dummy', 'xxx');
-});
+}, 'BAM');
 ensureThrows(function() {
   fs.appendFileSync('dummy', 'xxx');
-});
+}, 'BAM');
 
-function ensureThrows(cb) {
+function ensureThrows(cb, message) {
   let got_exception = false;
 
   close_called = 0;
   try {
     cb();
   } catch (e) {
-    assert.strictEqual(e.message, 'BAM');
+    assert.strictEqual(e.message, message);
     got_exception = true;
   }