fs: throw writeSync errors in JS

PR-URL: https://github.com/nodejs/node/pull/19041
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

lib/fs.js

@@ -635,6 +635,8 @@ Object.defineProperty(fs.write, internalUtil.customPromisifyArgs,
 //  fs.writeSync(fd, string[, position[, encoding]]);
 fs.writeSync = function(fd, buffer, offset, length, position) {
   validateUint32(fd, 'fd');
+  const ctx = {};
+  let result;
   if (isUint8Array(buffer)) {
     if (position === undefined)
       position = null;
@@ -643,13 +645,18 @@ fs.writeSync = function(fd, buffer, offset, length, position) {
     if (typeof length !== 'number')
       length = buffer.length - offset;
     validateOffsetLengthWrite(offset, length, buffer.byteLength);
-    return binding.writeBuffer(fd, buffer, offset, length, position);
+    result = binding.writeBuffer(fd, buffer, offset, length, position,
+                                 undefined, ctx);
+  } else {
+    if (typeof buffer !== 'string')
+      buffer += '';
+    if (offset === undefined)
+      offset = null;
+    result = binding.writeString(fd, buffer, offset, length,
+                                 undefined, ctx);
   }
-  if (typeof buffer !== 'string')
+  handleErrorFromBinding(ctx);
-    buffer += '';
+  return result;
-  if (offset === undefined)
-    offset = null;
-  return binding.writeString(fd, buffer, offset, length, position);
 };
 
 fs.rename = function(oldPath, newPath, callback) {

src/node_file.cc

@@ -1270,35 +1270,43 @@ static void CopyFile(const FunctionCallbackInfo<Value>& args) {
 static void WriteBuffer(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
+  const int argc = args.Length();
+  CHECK_GE(argc, 4);
+
   CHECK(args[0]->IsInt32());
+  const int fd = args[0].As<Int32>()->Value();
+
   CHECK(Buffer::HasInstance(args[1]));
+  Local<Object> buffer_obj = args[1].As<Object>();
+  char* buffer_data = Buffer::Data(buffer_obj);
+  size_t buffer_length = Buffer::Length(buffer_obj);
 
-  int fd = args[0]->Int32Value();
+  CHECK(args[2]->IsInt32());
-  Local<Object> obj = args[1].As<Object>();
+  const size_t off = static_cast<size_t>(args[2].As<Int32>()->Value());
-  const char* buf = Buffer::Data(obj);
-  size_t buffer_length = Buffer::Length(obj);
-  size_t off = args[2]->Uint32Value();
-  size_t len = args[3]->Uint32Value();
-  int64_t pos = GET_OFFSET(args[4]);
-
   CHECK_LE(off, buffer_length);
+
+  CHECK(args[3]->IsInt32());
+  const size_t len = static_cast<size_t>(args[3].As<Int32>()->Value());
+  CHECK(Buffer::IsWithinBounds(off, len, buffer_length));
   CHECK_LE(len, buffer_length);
   CHECK_GE(off + len, off);
-  CHECK(Buffer::IsWithinBounds(off, len, buffer_length));
 
-  buf += off;
+  const int64_t pos = GET_OFFSET(args[4]);
 
+  char* buf = buffer_data + off;
   uv_buf_t uvbuf = uv_buf_init(const_cast<char*>(buf), len);
 
   FSReqBase* req_wrap = GetReqWrap(env, args[5]);
-  if (req_wrap != nullptr) {
+  if (req_wrap != nullptr) {  // write(fd, buffer, off, len, pos, req)
     AsyncCall(env, req_wrap, args, "write", UTF8, AfterInteger,
               uv_fs_write, fd, &uvbuf, 1, pos);
-    return;
+  } else {  // write(fd, buffer, off, len, pos, undefined, ctx)
+    CHECK_EQ(argc, 7);
+    fs_req_wrap req_wrap;
+    int bytesWritten = SyncCall(env, args[6], &req_wrap, "write",
+                                uv_fs_write, fd, &uvbuf, 1, pos);
+    args.GetReturnValue().Set(bytesWritten);
   }
-
-  SYNC_CALL(write, nullptr, fd, &uvbuf, 1, pos)
-  args.GetReturnValue().Set(SYNC_RESULT);
 }
 
 
@@ -1350,19 +1358,23 @@ static void WriteBuffers(const FunctionCallbackInfo<Value>& args) {
 static void WriteString(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  CHECK(args[0]->IsInt32());
+  const int argc = args.Length();
+  CHECK_GE(argc, 4);
+
+  CHECK(args[0]->IsInt32());
+  const int fd = args[0].As<Int32>()->Value();
 
-  std::unique_ptr<char[]> delete_on_return;
-  Local<Value> req;
-  Local<Value> value = args[1];
-  int fd = args[0]->Int32Value();
-  char* buf = nullptr;
-  size_t len;
   const int64_t pos = GET_OFFSET(args[2]);
+
   const auto enc = ParseEncoding(env->isolate(), args[3], UTF8);
 
+  std::unique_ptr<char[]> delete_on_return;
+  Local<Value> value = args[1];
+  char* buf = nullptr;
+  size_t len;
+
   FSReqBase* req_wrap = GetReqWrap(env, args[4]);
-  const auto is_async = req_wrap != nullptr;
+  const bool is_async = req_wrap != nullptr;
 
   // Avoid copying the string when it is externalized but only when:
   // 1. The target encoding is compatible with the string's encoding, and
@@ -1396,12 +1408,15 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {
 
   uv_buf_t uvbuf = uv_buf_init(buf, len);
 
-  if (req_wrap != nullptr) {
+  if (is_async) {  // write(fd, string, pos, enc, req)
     AsyncCall(env, req_wrap, args, "write", UTF8, AfterInteger,
               uv_fs_write, fd, &uvbuf, 1, pos);
-  } else {
+  } else {  // write(fd, string, pos, enc, undefined, ctx)
-    SYNC_CALL(write, nullptr, fd, &uvbuf, 1, pos)
+    CHECK_EQ(argc, 6);
-    return args.GetReturnValue().Set(SYNC_RESULT);
+    fs_req_wrap req_wrap;
+    int bytesWritten = SyncCall(env, args[5], &req_wrap, "write",
+                                uv_fs_write, fd, &uvbuf, 1, pos);
+    args.GetReturnValue().Set(bytesWritten);
   }
 }
 

test/parallel/test-fs-error-messages.js

@@ -770,3 +770,44 @@ if (!common.isWindows) {
     );
   });
 }
+
+// write buffer
+{
+  const validateError = (err) => {
+    assert.strictEqual(err.message, 'EBADF: bad file descriptor, write');
+    assert.strictEqual(err.errno, uv.UV_EBADF);
+    assert.strictEqual(err.code, 'EBADF');
+    assert.strictEqual(err.syscall, 'write');
+    return true;
+  };
+
+  common.runWithInvalidFD((fd) => {
+    const buf = Buffer.alloc(5);
+    fs.write(fd, buf, 0, 1, 1, common.mustCall(validateError));
+
+    assert.throws(
+      () => fs.writeSync(fd, buf, 0, 1, 1),
+      validateError
+    );
+  });
+}
+
+// write string
+{
+  const validateError = (err) => {
+    assert.strictEqual(err.message, 'EBADF: bad file descriptor, write');
+    assert.strictEqual(err.errno, uv.UV_EBADF);
+    assert.strictEqual(err.code, 'EBADF');
+    assert.strictEqual(err.syscall, 'write');
+    return true;
+  };
+
+  common.runWithInvalidFD((fd) => {
+    fs.write(fd, 'test', 1, common.mustCall(validateError));
+
+    assert.throws(
+      () => fs.writeSync(fd, 'test', 1),
+      validateError
+    );
+  });
+}