src: improve StringBytes error handling

PR-URL: https://github.com/nodejs/node/pull/57706
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

src/api/encoding.cc

@@ -8,6 +8,8 @@ namespace node {
 using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
+using v8::MaybeLocal;
+using v8::TryCatch;
 using v8::Value;
 
 enum encoding ParseEncoding(const char* encoding,
@@ -133,20 +135,30 @@ enum encoding ParseEncoding(Isolate* isolate,
   return ParseEncoding(*encoding, default_encoding);
 }
 
+MaybeLocal<Value> TryEncode(Isolate* isolate,
+                            const char* buf,
+                            size_t len,
+                            enum encoding encoding) {
+  CHECK_NE(encoding, UCS2);
+  return StringBytes::Encode(isolate, buf, len, encoding);
+}
+
+MaybeLocal<Value> TryEncode(Isolate* isolate, const uint16_t* buf, size_t len) {
+  return StringBytes::Encode(isolate, buf, len).ToLocalChecked();
+}
+
 Local<Value> Encode(Isolate* isolate,
                     const char* buf,
                     size_t len,
                     enum encoding encoding) {
   CHECK_NE(encoding, UCS2);
-  Local<Value> error;
-  return StringBytes::Encode(isolate, buf, len, encoding, &error)
-      .ToLocalChecked();
+  TryCatch try_catch(isolate);
+  return StringBytes::Encode(isolate, buf, len, encoding).ToLocalChecked();
 }
 
 Local<Value> Encode(Isolate* isolate, const uint16_t* buf, size_t len) {
-  Local<Value> error;
-  return StringBytes::Encode(isolate, buf, len, &error)
-      .ToLocalChecked();
+  TryCatch try_catch(isolate);
+  return StringBytes::Encode(isolate, buf, len).ToLocalChecked();
 }
 
 // Returns -1 if the handle was not valid for decoding

src/crypto/crypto_ec.cc

@@ -800,17 +800,11 @@ bool ExportJWKEdKey(Environment* env,
                                        Local<Object> target,
                                        Local<String> key) {
     Local<Value> encoded;
-    Local<Value> error;
     if (!data) return false;
     const ncrypto::Buffer<const char> out = data;
-    if (!StringBytes::Encode(
-             env->isolate(), out.data, out.len, BASE64URL, &error)
-             .ToLocal(&encoded) ||
-        target->Set(env->context(), key, encoded).IsNothing()) {
-      if (!error.IsEmpty()) env->isolate()->ThrowException(error);
-      return false;
-    }
-    return true;
+    return StringBytes::Encode(env->isolate(), out.data, out.len, BASE64URL)
+               .ToLocal(&encoded) &&
+           target->Set(env->context(), key, encoded).IsJust();
   };
 
   return !(

src/crypto/crypto_hash.cc

@@ -252,19 +252,14 @@ void Hash::OneShotDigest(const FunctionCallbackInfo<Value>& args) {
     return ThrowCryptoError(env, ERR_get_error());
   }
 
-  Local<Value> error;
-  MaybeLocal<Value> rc =
-      StringBytes::Encode(env->isolate(),
+  Local<Value> ret;
+  if (StringBytes::Encode(env->isolate(),
                           static_cast<const char*>(output.get()),
                           output.size(),
-                          output_enc,
-                          &error);
-  if (rc.IsEmpty()) [[unlikely]] {
-    CHECK(!error.IsEmpty());
-    env->isolate()->ThrowException(error);
-    return;
+                          output_enc)
+          .ToLocal(&ret)) {
+    args.GetReturnValue().Set(ret);
   }
-  args.GetReturnValue().Set(rc.FromMaybe(Local<Value>()));
 }
 
 void Hash::Initialize(Environment* env, Local<Object> target) {
@@ -410,15 +405,12 @@ void Hash::HashDigest(const FunctionCallbackInfo<Value>& args) {
     hash->digest_ = ByteSource::Allocated(data.release());
   }
 
-  Local<Value> error;
-  MaybeLocal<Value> rc = StringBytes::Encode(
-      env->isolate(), hash->digest_.data<char>(), len, encoding, &error);
-  if (rc.IsEmpty()) [[unlikely]] {
-    CHECK(!error.IsEmpty());
-    env->isolate()->ThrowException(error);
-    return;
+  Local<Value> ret;
+  if (StringBytes::Encode(
+          env->isolate(), hash->digest_.data<char>(), len, encoding)
+          .ToLocal(&ret)) {
+    args.GetReturnValue().Set(ret);
   }
-  args.GetReturnValue().Set(rc.FromMaybe(Local<Value>()));
 }
 
 HashConfig::HashConfig(HashConfig&& other) noexcept
@@ -541,19 +533,14 @@ void InternalVerifyIntegrity(const v8::FunctionCallbackInfo<v8::Value>& args) {
 
   if (digest_size != expected.size() ||
       CRYPTO_memcmp(digest, expected.data(), digest_size) != 0) {
-    Local<Value> error;
-    MaybeLocal<Value> rc =
-        StringBytes::Encode(env->isolate(),
+    Local<Value> ret;
+    if (StringBytes::Encode(env->isolate(),
                             reinterpret_cast<const char*>(digest),
                             digest_size,
-                            BASE64,
-                            &error);
-    if (rc.IsEmpty()) [[unlikely]] {
-      CHECK(!error.IsEmpty());
-      env->isolate()->ThrowException(error);
-      return;
+                            BASE64)
+            .ToLocal(&ret)) {
+      args.GetReturnValue().Set(ret);
     }
-    args.GetReturnValue().Set(rc.FromMaybe(Local<Value>()));
   }
 }
 }  // namespace crypto

src/crypto/crypto_hmac.cc

@@ -145,19 +145,14 @@ void Hmac::HmacDigest(const FunctionCallbackInfo<Value>& args) {
     hmac->ctx_.reset();
   }
 
-  Local<Value> error;
-  MaybeLocal<Value> rc =
-      StringBytes::Encode(env->isolate(),
+  Local<Value> ret;
+  if (StringBytes::Encode(env->isolate(),
                           reinterpret_cast<const char*>(md_value),
                           buf.len,
-                          encoding,
-                          &error);
-  if (rc.IsEmpty()) [[unlikely]] {
-    CHECK(!error.IsEmpty());
-    env->isolate()->ThrowException(error);
-    return;
+                          encoding)
+          .ToLocal(&ret)) {
+    args.GetReturnValue().Set(ret);
   }
-  args.GetReturnValue().Set(rc.FromMaybe(Local<Value>()));
 }
 
 HmacConfig::HmacConfig(HmacConfig&& other) noexcept

src/crypto/crypto_keys.cc

@@ -130,20 +130,13 @@ bool ExportJWKSecretKey(Environment* env,
                         Local<Object> target) {
   CHECK_EQ(key.GetKeyType(), kKeyTypeSecret);
 
-  Local<Value> error;
   Local<Value> raw;
-  if (!StringBytes::Encode(env->isolate(),
-                           key.GetSymmetricKey(),
-                           key.GetSymmetricKeySize(),
-                           BASE64URL,
-                           &error)
-           .ToLocal(&raw)) {
-    DCHECK(!error.IsEmpty());
-    env->isolate()->ThrowException(error);
-    return false;
-  }
-
-  return target
+  return StringBytes::Encode(env->isolate(),
+                             key.GetSymmetricKey(),
+                             key.GetSymmetricKeySize(),
+                             BASE64URL)
+             .ToLocal(&raw) &&
+         target
              ->Set(env->context(), env->jwk_kty_string(), env->jwk_oct_string())
              .IsJust() &&
          target->Set(env->context(), env->jwk_k_string(), raw).IsJust();

src/crypto/crypto_spkac.cc

@@ -5,6 +5,7 @@
 #include "memory_tracker-inl.h"
 #include "ncrypto.h"
 #include "node.h"
+#include "string_bytes.h"
 #include "v8.h"
 
 namespace node {
@@ -52,18 +53,22 @@ void ExportChallenge(const FunctionCallbackInfo<Value>& args) {
   ArrayBufferOrViewContents<char> input(args[0]);
   if (input.empty()) return args.GetReturnValue().SetEmptyString();
 
-  if (!input.CheckSizeInt32()) [[unlikely]]
+  if (!input.CheckSizeInt32()) [[unlikely]] {
     return THROW_ERR_OUT_OF_RANGE(env, "spkac is too large");
+  }
 
   auto cert = ByteSource::Allocated(
       ncrypto::ExportChallenge(input.data(), input.size()));
-  if (!cert)
+  if (!cert) {
     return args.GetReturnValue().SetEmptyString();
+  }
 
-  Local<Value> outString =
-      Encode(env->isolate(), cert.data<char>(), cert.size(), BUFFER);
-
-  args.GetReturnValue().Set(outString);
+  Local<Value> outString;
+  if (StringBytes::Encode(
+          env->isolate(), cert.data<char>(), cert.size(), BUFFER)
+          .ToLocal(&outString)) {
+    args.GetReturnValue().Set(outString);
+  }
 }
 
 void Initialize(Environment* env, Local<Object> target) {

src/crypto/crypto_util.cc

@@ -36,6 +36,7 @@ using v8::ArrayBuffer;
 using v8::BackingStore;
 using v8::BigInt;
 using v8::Context;
+using v8::EscapableHandleScope;
 using v8::Exception;
 using v8::FunctionCallbackInfo;
 using v8::HandleScope;
@@ -605,21 +606,18 @@ void SetEngine(const FunctionCallbackInfo<Value>& args) {
 #endif  // !OPENSSL_NO_ENGINE
 
 MaybeLocal<Value> EncodeBignum(Environment* env, const BIGNUM* bn, int size) {
+  EscapableHandleScope scope(env->isolate());
   auto buf = BignumPointer::EncodePadded(bn, size);
   CHECK_EQ(buf.size(), static_cast<size_t>(size));
   Local<Value> ret;
-  Local<Value> error;
   if (!StringBytes::Encode(env->isolate(),
                            reinterpret_cast<const char*>(buf.get()),
                            buf.size(),
-                           BASE64URL,
-                           &error)
+                           BASE64URL)
            .ToLocal(&ret)) {
-    CHECK(!error.IsEmpty());
-    env->isolate()->ThrowException(error);
-    return MaybeLocal<Value>();
+    return {};
   }
-  return ret;
+  return scope.Escape(ret);
 }
 
 Maybe<void> SetEncodedValue(Environment* env,

src/encoding_binding.cc

@@ -183,17 +183,10 @@ void BindingData::DecodeUTF8(const FunctionCallbackInfo<Value>& args) {
 
   if (length == 0) return args.GetReturnValue().SetEmptyString();
 
-  Local<Value> error;
   Local<Value> ret;
-
-  if (!StringBytes::Encode(env->isolate(), data, length, UTF8, &error)
-           .ToLocal(&ret)) {
-    CHECK(!error.IsEmpty());
-    env->isolate()->ThrowException(error);
-    return;
+  if (StringBytes::Encode(env->isolate(), data, length, UTF8).ToLocal(&ret)) {
+    args.GetReturnValue().Set(ret);
   }
-
-  args.GetReturnValue().Set(ret);
 }
 
 void BindingData::ToASCII(const FunctionCallbackInfo<Value>& args) {

src/fs_event_wrap.cc

@@ -44,6 +44,7 @@ using v8::PropertyAttribute;
 using v8::ReadOnly;
 using v8::Signature;
 using v8::String;
+using v8::TryCatch;
 using v8::Value;
 
 namespace {
@@ -217,18 +218,19 @@ void FSEventWrap::OnEvent(uv_fs_event_t* handle, const char* filename,
   };
 
   if (filename != nullptr) {
-    Local<Value> error;
-    MaybeLocal<Value> fn = StringBytes::Encode(env->isolate(),
-                                               filename,
-                                               wrap->encoding_,
-                                               &error);
+    // TODO(@jasnell): Historically, this code has failed to correctly
+    // propagate any error returned by the StringBytes::Encode method,
+    // and would instead just crash the process. That behavior is preserved
+    // here but should be looked at. Preferrably errors would be handled
+    // correctly here.
+    TryCatch try_catch(env->isolate());
+    MaybeLocal<Value> fn =
+        StringBytes::Encode(env->isolate(), filename, wrap->encoding_);
     if (fn.IsEmpty()) {
       argv[0] = Integer::New(env->isolate(), UV_EINVAL);
-      argv[2] = StringBytes::Encode(env->isolate(),
-                                    filename,
-                                    strlen(filename),
-                                    BUFFER,
-                                    &error).ToLocalChecked();
+      argv[2] = StringBytes::Encode(
+                    env->isolate(), filename, strlen(filename), BUFFER)
+                    .ToLocalChecked();
     } else {
       argv[2] = fn.ToLocalChecked();
     }

src/node.h

@@ -1124,16 +1124,37 @@ NODE_EXTERN enum encoding ParseEncoding(
 NODE_EXTERN void FatalException(v8::Isolate* isolate,
                                 const v8::TryCatch& try_catch);
 
-NODE_EXTERN v8::Local<v8::Value> Encode(v8::Isolate* isolate,
-                                        const char* buf,
-                                        size_t len,
-                                        enum encoding encoding = LATIN1);
+NODE_EXTERN v8::MaybeLocal<v8::Value> TryEncode(
+    v8::Isolate* isolate,
+    const char* buf,
+    size_t len,
+    enum encoding encoding = LATIN1);
 
 // Warning: This reverses endianness on Big Endian platforms, even though the
 // signature using uint16_t implies that it should not.
-NODE_EXTERN v8::Local<v8::Value> Encode(v8::Isolate* isolate,
-                                        const uint16_t* buf,
-                                        size_t len);
+NODE_EXTERN v8::MaybeLocal<v8::Value> TryEncode(v8::Isolate* isolate,
+                                                const uint16_t* buf,
+                                                size_t len);
+
+// The original Encode(...) functions are deprecated because they do not
+// appropriately propagate exceptions and instead rely on ToLocalChecked()
+// which crashes the process if an exception occurs. We cannot just remove
+// these as it would break ABI compatibility, so we keep them around but
+// deprecate them in favor of the TryEncode(...) variations which return
+// a MaybeLocal<> and do not crash the process if an exception occurs.
+NODE_DEPRECATED(
+    "Use TryEncode(...) instead",
+    NODE_EXTERN v8::Local<v8::Value> Encode(v8::Isolate* isolate,
+                                            const char* buf,
+                                            size_t len,
+                                            enum encoding encoding = LATIN1));
+
+// Warning: This reverses endianness on Big Endian platforms, even though the
+// signature using uint16_t implies that it should not.
+NODE_DEPRECATED("Use TryEncode(...) instead",
+                NODE_EXTERN v8::Local<v8::Value> Encode(v8::Isolate* isolate,
+                                                        const uint16_t* buf,
+                                                        size_t len));
 
 // Returns -1 if the handle was not valid for decoding
 NODE_EXTERN ssize_t DecodeBytes(v8::Isolate* isolate,

src/node_buffer.cc

@@ -544,20 +544,11 @@ void StringSlice(const FunctionCallbackInfo<Value>& args) {
   THROW_AND_RETURN_IF_OOB(Just(end <= buffer.length()));
   size_t length = end - start;
 
-  Local<Value> error;
-  MaybeLocal<Value> maybe_ret =
-      StringBytes::Encode(isolate,
-                          buffer.data() + start,
-                          length,
-                          encoding,
-                          &error);
   Local<Value> ret;
-  if (!maybe_ret.ToLocal(&ret)) {
-    CHECK(!error.IsEmpty());
-    isolate->ThrowException(error);
-    return;
+  if (StringBytes::Encode(isolate, buffer.data() + start, length, encoding)
+          .ToLocal(&ret)) {
+    args.GetReturnValue().Set(ret);
   }
-  args.GetReturnValue().Set(ret);
 }
 
 // Assume caller has properly validated args.

src/node_dir.cc

@@ -42,6 +42,7 @@ using v8::Null;
 using v8::Number;
 using v8::Object;
 using v8::ObjectTemplate;
+using v8::TryCatch;
 using v8::Value;
 
 static const char* get_dir_func_name_by_type(uv_fs_type req_type) {
@@ -206,27 +207,20 @@ void DirHandle::Close(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
-static MaybeLocal<Array> DirentListToArray(
-    Environment* env,
-    uv_dirent_t* ents,
-    int num,
-    enum encoding encoding,
-    Local<Value>* err_out) {
+static MaybeLocal<Array> DirentListToArray(Environment* env,
+                                           uv_dirent_t* ents,
+                                           int num,
+                                           enum encoding encoding) {
   MaybeStackBuffer<Local<Value>, 64> entries(num * 2);
 
   // Return an array of all read filenames.
   int j = 0;
   for (int i = 0; i < num; i++) {
     Local<Value> filename;
-    Local<Value> error;
     const size_t namelen = strlen(ents[i].name);
-    if (!StringBytes::Encode(env->isolate(),
-                             ents[i].name,
-                             namelen,
-                             encoding,
-                             &error).ToLocal(&filename)) {
-      *err_out = error;
-      return MaybeLocal<Array>();
+    if (!StringBytes::Encode(env->isolate(), ents[i].name, namelen, encoding)
+             .ToLocal(&filename)) {
+      return {};
     }
 
     entries[j++] = filename;
@@ -258,18 +252,18 @@ static void AfterDirRead(uv_fs_t* req) {
 
   uv_dir_t* dir = static_cast<uv_dir_t*>(req->ptr);
 
-  Local<Value> error;
+  TryCatch try_catch(isolate);
   Local<Array> js_array;
   if (!DirentListToArray(env,
                          dir->dirents,
                          static_cast<int>(req->result),
-                         req_wrap->encoding(),
-                         &error)
+                         req_wrap->encoding())
            .ToLocal(&js_array)) {
     // Clear libuv resources *before* delivering results to JS land because
     // that can schedule another operation on the same uv_dir_t. Ditto below.
     after.Clear();
-    return req_wrap->Reject(error);
+    CHECK(try_catch.CanContinue());
+    return req_wrap->Reject(try_catch.Exception());
   }
 
   after.Clear();
@@ -320,16 +314,16 @@ void DirHandle::Read(const FunctionCallbackInfo<Value>& args) {
 
     CHECK_GE(req_wrap_sync.req.result, 0);
 
-    Local<Value> error;
+    TryCatch try_catch(isolate);
     Local<Array> js_array;
     if (!DirentListToArray(env,
                            dir->dir()->dirents,
                            static_cast<int>(req_wrap_sync.req.result),
-                           encoding,
-                           &error)
+                           encoding)
              .ToLocal(&js_array)) {
       // TODO(anonrig): Initializing BufferValue here is wasteful.
-      BufferValue error_payload(isolate, error);
+      CHECK(try_catch.CanContinue());
+      BufferValue error_payload(isolate, try_catch.Exception());
       env->ThrowError(error_payload.out());
       return;
     }

src/node_file.cc

@@ -82,6 +82,7 @@ using v8::Object;
 using v8::ObjectTemplate;
 using v8::Promise;
 using v8::String;
+using v8::TryCatch;
 using v8::Undefined;
 using v8::Value;
 
@@ -845,11 +846,13 @@ void AfterMkdirp(uv_fs_t* req) {
     if (first_path.empty())
       return req_wrap->Resolve(Undefined(req_wrap->env()->isolate()));
     Local<Value> path;
-    Local<Value> error;
-    if (!StringBytes::Encode(req_wrap->env()->isolate(), first_path.c_str(),
-                             req_wrap->encoding(),
-                             &error).ToLocal(&path)) {
-      return req_wrap->Reject(error);
+    TryCatch try_catch(req_wrap->env()->isolate());
+    if (!StringBytes::Encode(req_wrap->env()->isolate(),
+                             first_path.c_str(),
+                             req_wrap->encoding())
+             .ToLocal(&path)) {
+      CHECK(try_catch.CanContinue());
+      return req_wrap->Reject(try_catch.Exception());
     }
     return req_wrap->Resolve(path);
   }
@@ -861,15 +864,14 @@ void AfterStringPath(uv_fs_t* req) {
   FS_ASYNC_TRACE_END1(
       req->fs_type, req_wrap, "result", static_cast<int>(req->result))
   MaybeLocal<Value> link;
-  Local<Value> error;
 
   if (after.Proceed()) {
-    link = StringBytes::Encode(req_wrap->env()->isolate(),
-                               req->path,
-                               req_wrap->encoding(),
-                               &error);
+    TryCatch try_catch(req_wrap->env()->isolate());
+    link = StringBytes::Encode(
+        req_wrap->env()->isolate(), req->path, req_wrap->encoding());
     if (link.IsEmpty()) {
-      req_wrap->Reject(error);
+      CHECK(try_catch.CanContinue());
+      req_wrap->Reject(try_catch.Exception());
     } else {
       Local<Value> val;
       if (link.ToLocal(&val)) req_wrap->Resolve(val);
@@ -883,15 +885,15 @@ void AfterStringPtr(uv_fs_t* req) {
   FS_ASYNC_TRACE_END1(
       req->fs_type, req_wrap, "result", static_cast<int>(req->result))
   MaybeLocal<Value> link;
-  Local<Value> error;
 
   if (after.Proceed()) {
+    TryCatch try_catch(req_wrap->env()->isolate());
     link = StringBytes::Encode(req_wrap->env()->isolate(),
                                static_cast<const char*>(req->ptr),
-                               req_wrap->encoding(),
-                               &error);
+                               req_wrap->encoding());
     if (link.IsEmpty()) {
-      req_wrap->Reject(error);
+      CHECK(try_catch.CanContinue());
+      req_wrap->Reject(try_catch.Exception());
     } else {
       Local<Value> val;
       if (link.ToLocal(&val)) req_wrap->Resolve(val);
@@ -910,7 +912,6 @@ void AfterScanDir(uv_fs_t* req) {
 
   Environment* env = req_wrap->env();
   Isolate* isolate = env->isolate();
-  Local<Value> error;
   int r;
 
   LocalVector<Value> name_v(isolate);
@@ -930,9 +931,11 @@ void AfterScanDir(uv_fs_t* req) {
     }
 
     Local<Value> filename;
-    if (!StringBytes::Encode(isolate, ent.name, req_wrap->encoding(), &error)
+    TryCatch try_catch(isolate);
+    if (!StringBytes::Encode(isolate, ent.name, req_wrap->encoding())
              .ToLocal(&filename)) {
-      return req_wrap->Reject(error);
+      CHECK(try_catch.CanContinue());
+      return req_wrap->Reject(try_catch.Exception());
     }
     name_v.push_back(filename);
 
@@ -1422,16 +1425,10 @@ static void ReadLink(const FunctionCallbackInfo<Value>& args) {
     }
     const char* link_path = static_cast<const char*>(req_wrap_sync.req.ptr);
 
-    Local<Value> error;
     Local<Value> ret;
-    if (!StringBytes::Encode(isolate, link_path, encoding, &error)
-             .ToLocal(&ret)) {
-      DCHECK(!error.IsEmpty());
-      env->isolate()->ThrowException(error);
-      return;
+    if (StringBytes::Encode(isolate, link_path, encoding).ToLocal(&ret)) {
+      args.GetReturnValue().Set(ret);
     }
-
-    args.GetReturnValue().Set(ret);
   }
 }
 
@@ -1932,17 +1929,12 @@ static void MKDir(const FunctionCallbackInfo<Value>& args) {
         return;
       }
       if (!req_wrap_sync.continuation_data()->first_path().empty()) {
-        Local<Value> error;
         Local<Value> ret;
         std::string first_path(req_wrap_sync.continuation_data()->first_path());
-        if (!StringBytes::Encode(
-                 env->isolate(), first_path.c_str(), UTF8, &error)
-                 .ToLocal(&ret)) {
-          DCHECK(!error.IsEmpty());
-          env->isolate()->ThrowException(error);
-          return;
+        if (StringBytes::Encode(env->isolate(), first_path.c_str(), UTF8)
+                .ToLocal(&ret)) {
+          args.GetReturnValue().Set(ret);
         }
-        args.GetReturnValue().Set(ret);
       }
     } else {
       SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_mkdir, *path, mode);
@@ -1982,16 +1974,10 @@ static void RealPath(const FunctionCallbackInfo<Value>& args) {
 
     const char* link_path = static_cast<const char*>(req_wrap_sync.req.ptr);
 
-    Local<Value> error;
     Local<Value> ret;
-    if (!StringBytes::Encode(isolate, link_path, encoding, &error)
-             .ToLocal(&ret)) {
-      DCHECK(!error.IsEmpty());
-      env->isolate()->ThrowException(error);
-      return;
+    if (StringBytes::Encode(isolate, link_path, encoding).ToLocal(&ret)) {
+      args.GetReturnValue().Set(ret);
     }
-
-    args.GetReturnValue().Set(ret);
   }
 }
 
@@ -2077,12 +2063,8 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
         return;
       }
 
-      Local<Value> error;
       Local<Value> fn;
-      if (!StringBytes::Encode(isolate, ent.name, encoding, &error)
-               .ToLocal(&fn)) {
-        DCHECK(!error.IsEmpty());
-        isolate->ThrowException(error);
+      if (!StringBytes::Encode(isolate, ent.name, encoding).ToLocal(&fn)) {
         return;
       }
 
@@ -3141,15 +3123,11 @@ static void Mkdtemp(const FunctionCallbackInfo<Value>& args) {
     if (is_uv_error(result)) {
       return;
     }
-    Local<Value> error;
     Local<Value> ret;
-    if (!StringBytes::Encode(isolate, req_wrap_sync.req.path, encoding, &error)
-             .ToLocal(&ret)) {
-      DCHECK(!error.IsEmpty());
-      env->isolate()->ThrowException(error);
-      return;
+    if (StringBytes::Encode(isolate, req_wrap_sync.req.path, encoding)
+            .ToLocal(&ret)) {
+      args.GetReturnValue().Set(ret);
     }
-    args.GetReturnValue().Set(ret);
   }
 }
 

src/node_i18n.cc

@@ -500,7 +500,6 @@ void ConverterObject::Decode(const FunctionCallbackInfo<Value>& args) {
       }
     }
 
-    Local<Value> error;
     UChar* output = result.out();
     size_t beginning = 0;
     size_t length = result.length() * sizeof(UChar);
@@ -517,11 +516,9 @@ void ConverterObject::Decode(const FunctionCallbackInfo<Value>& args) {
       CHECK(nbytes::SwapBytes16(value, length));
     }
 
-    MaybeLocal<Value> encoded =
-        StringBytes::Encode(env->isolate(), value, length, UCS2, &error);
-
     Local<Value> ret;
-    if (encoded.ToLocal(&ret)) {
+    if (StringBytes::Encode(env->isolate(), value, length, UCS2)
+            .ToLocal(&ret)) {
       args.GetReturnValue().Set(ret);
       return;
     }

src/node_os.cc

@@ -307,8 +307,6 @@ static void GetUserInfo(const FunctionCallbackInfo<Value>& args) {
 
   auto free_passwd = OnScopeLeave([&] { uv_os_free_passwd(&pwd); });
 
-  Local<Value> error;
-
 #ifdef _WIN32
   Local<Value> uid = Number::New(
       env->isolate(),
@@ -322,27 +320,21 @@ static void GetUserInfo(const FunctionCallbackInfo<Value>& args) {
 #endif
 
   Local<Value> username;
-  if (!StringBytes::Encode(env->isolate(), pwd.username, encoding, &error)
+  if (!StringBytes::Encode(env->isolate(), pwd.username, encoding)
            .ToLocal(&username)) {
-    CHECK(!error.IsEmpty());
-    env->isolate()->ThrowException(error);
     return;
   }
 
   Local<Value> homedir;
-  if (!StringBytes::Encode(env->isolate(), pwd.homedir, encoding, &error)
+  if (!StringBytes::Encode(env->isolate(), pwd.homedir, encoding)
            .ToLocal(&homedir)) {
-    CHECK(!error.IsEmpty());
-    env->isolate()->ThrowException(error);
     return;
   }
 
   Local<Value> shell = Null(env->isolate());
   if (pwd.shell != nullptr &&
-      !StringBytes::Encode(env->isolate(), pwd.shell, encoding, &error)
+      !StringBytes::Encode(env->isolate(), pwd.shell, encoding)
            .ToLocal(&shell)) {
-    CHECK(!error.IsEmpty());
-    env->isolate()->ThrowException(error);
     return;
   }
 

src/string_bytes.cc

@@ -74,37 +74,34 @@ class ExternString: public ResourceType {
 
   static MaybeLocal<Value> NewFromCopy(Isolate* isolate,
                                        const TypeName* data,
-                                       size_t length,
-                                       Local<Value>* error) {
-    if (length == 0)
+                                       size_t length) {
+    if (length == 0) {
       return String::Empty(isolate);
+    }
 
-    if (length < EXTERN_APEX)
-      return NewSimpleFromCopy(isolate, data, length, error);
+    if (length < EXTERN_APEX) {
+      return NewSimpleFromCopy(isolate, data, length);
+    }
 
     TypeName* new_data = node::UncheckedMalloc<TypeName>(length);
     if (new_data == nullptr) {
-      *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
+      isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate));
       return MaybeLocal<Value>();
     }
     memcpy(new_data, data, length * sizeof(*new_data));
 
-    return ExternString<ResourceType, TypeName>::New(isolate,
-                                                     new_data,
-                                                     length,
-                                                     error);
+    return ExternString<ResourceType, TypeName>::New(isolate, new_data, length);
   }
 
   // uses "data" for external resource, and will be free'd on gc
   static MaybeLocal<Value> New(Isolate* isolate,
                                TypeName* data,
-                               size_t length,
-                               Local<Value>* error) {
+                               size_t length) {
     if (length == 0)
       return String::Empty(isolate);
 
     if (length < EXTERN_APEX) {
-      MaybeLocal<Value> str = NewSimpleFromCopy(isolate, data, length, error);
+      MaybeLocal<Value> str = NewSimpleFromCopy(isolate, data, length);
       free(data);
       return str;
     }
@@ -116,7 +113,7 @@ class ExternString: public ResourceType {
 
     if (!NewExternal(isolate, h_str).ToLocal(&str)) {
       delete h_str;
-      *error = node::ERR_STRING_TOO_LONG(isolate);
+      isolate->ThrowException(node::ERR_STRING_TOO_LONG(isolate));
       return MaybeLocal<Value>();
     }
 
@@ -136,8 +133,7 @@ class ExternString: public ResourceType {
   // This method does not actually create ExternString instances.
   static MaybeLocal<Value> NewSimpleFromCopy(Isolate* isolate,
                                              const TypeName* data,
-                                             size_t length,
-                                             Local<Value>* error);
+                                             size_t length);
 
   Isolate* isolate_;
   const TypeName* data_;
@@ -167,30 +163,27 @@ MaybeLocal<Value> ExternTwoByteString::NewExternal(
 template <>
 MaybeLocal<Value> ExternOneByteString::NewSimpleFromCopy(Isolate* isolate,
                                                          const char* data,
-                                                         size_t length,
-                                                         Local<Value>* error) {
+                                                         size_t length) {
   Local<String> str;
   if (!String::NewFromOneByte(isolate,
                               reinterpret_cast<const uint8_t*>(data),
                               v8::NewStringType::kNormal,
                               length)
            .ToLocal(&str)) {
-    *error = node::ERR_STRING_TOO_LONG(isolate);
+    isolate->ThrowException(node::ERR_STRING_TOO_LONG(isolate));
     return MaybeLocal<Value>();
   }
   return str;
 }
 
-
 template <>
 MaybeLocal<Value> ExternTwoByteString::NewSimpleFromCopy(Isolate* isolate,
                                                          const uint16_t* data,
-                                                         size_t length,
-                                                         Local<Value>* error) {
+                                                         size_t length) {
   Local<String> str;
   if (!String::NewFromTwoByte(isolate, data, v8::NewStringType::kNormal, length)
            .ToLocal(&str)) {
-    *error = node::ERR_STRING_TOO_LONG(isolate);
+    isolate->ThrowException(node::ERR_STRING_TOO_LONG(isolate));
     return MaybeLocal<Value>();
   }
   return str;
@@ -489,20 +482,18 @@ Maybe<size_t> StringBytes::Size(Isolate* isolate,
   UNREACHABLE();
 }
 
-#define CHECK_BUFLEN_IN_RANGE(len)                                    \
-  do {                                                                \
-    if ((len) > Buffer::kMaxLength) {                                 \
-      *error = node::ERR_BUFFER_TOO_LARGE(isolate);                   \
-      return MaybeLocal<Value>();                                     \
-    }                                                                 \
+#define CHECK_BUFLEN_IN_RANGE(len)                                             \
+  do {                                                                         \
+    if ((len) > Buffer::kMaxLength) {                                          \
+      isolate->ThrowException(node::ERR_BUFFER_TOO_LARGE(isolate));            \
+      return MaybeLocal<Value>();                                              \
+    }                                                                          \
   } while (0)
 
-
 MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
                                       const char* buf,
                                       size_t buflen,
-                                      enum encoding encoding,
-                                      Local<Value>* error) {
+                                      enum encoding encoding) {
   CHECK_BUFLEN_IN_RANGE(buflen);
 
   if (!buflen && encoding != BUFFER) {
@@ -517,7 +508,7 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
         auto maybe_buf = Buffer::Copy(isolate, buf, buflen);
         Local<v8::Object> buf;
         if (!maybe_buf.ToLocal(&buf)) {
-          *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
+          isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate));
         }
         return buf;
       }
@@ -528,13 +519,13 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
         // The input contains non-ASCII bytes.
         char* out = node::UncheckedMalloc(buflen);
         if (out == nullptr) {
-          *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
+          isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate));
           return MaybeLocal<Value>();
         }
         nbytes::ForceAscii(buf, out, buflen);
-        return ExternOneByteString::New(isolate, out, buflen, error);
+        return ExternOneByteString::New(isolate, out, buflen);
       } else {
-        return ExternOneByteString::NewFromCopy(isolate, buf, buflen, error);
+        return ExternOneByteString::NewFromCopy(isolate, buf, buflen);
       }
 
     case UTF8: {
@@ -543,28 +534,28 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
           String::NewFromUtf8(isolate, buf, v8::NewStringType::kNormal, buflen);
       Local<String> str;
       if (!val.ToLocal(&str)) {
-        *error = node::ERR_STRING_TOO_LONG(isolate);
+        isolate->ThrowException(node::ERR_STRING_TOO_LONG(isolate));
       }
       return str;
     }
 
     case LATIN1:
       buflen = keep_buflen_in_range(buflen);
-      return ExternOneByteString::NewFromCopy(isolate, buf, buflen, error);
+      return ExternOneByteString::NewFromCopy(isolate, buf, buflen);
 
     case BASE64: {
       buflen = keep_buflen_in_range(buflen);
       size_t dlen = simdutf::base64_length_from_binary(buflen);
       char* dst = node::UncheckedMalloc(dlen);
       if (dst == nullptr) {
-        *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
+        isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate));
         return MaybeLocal<Value>();
       }
 
       size_t written = simdutf::binary_to_base64(buf, buflen, dst);
       CHECK_EQ(written, dlen);
 
-      return ExternOneByteString::New(isolate, dst, dlen, error);
+      return ExternOneByteString::New(isolate, dst, dlen);
     }
 
     case BASE64URL: {
@@ -573,7 +564,7 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
           simdutf::base64_length_from_binary(buflen, simdutf::base64_url);
       char* dst = node::UncheckedMalloc(dlen);
       if (dst == nullptr) {
-        *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
+        isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate));
         return MaybeLocal<Value>();
       }
 
@@ -581,7 +572,7 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
           simdutf::binary_to_base64(buf, buflen, dst, simdutf::base64_url);
       CHECK_EQ(written, dlen);
 
-      return ExternOneByteString::New(isolate, dst, dlen, error);
+      return ExternOneByteString::New(isolate, dst, dlen);
     }
 
     case HEX: {
@@ -589,13 +580,13 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
       size_t dlen = buflen * 2;
       char* dst = node::UncheckedMalloc(dlen);
       if (dst == nullptr) {
-        *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
+        isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate));
         return MaybeLocal<Value>();
       }
       size_t written = nbytes::HexEncode(buf, buflen, dst, dlen);
       CHECK_EQ(written, dlen);
 
-      return ExternOneByteString::New(isolate, dst, dlen, error);
+      return ExternOneByteString::New(isolate, dst, dlen);
     }
 
     case UCS2: {
@@ -604,7 +595,7 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
       if constexpr (IsBigEndian()) {
         uint16_t* dst = node::UncheckedMalloc<uint16_t>(str_len);
         if (str_len != 0 && dst == nullptr) {
-          *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
+          isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate));
           return MaybeLocal<Value>();
         }
         for (size_t i = 0, k = 0; k < str_len; i += 2, k += 1) {
@@ -614,21 +605,21 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
           const uint8_t lo = static_cast<uint8_t>(buf[i + 0]);
           dst[k] = static_cast<uint16_t>(hi) << 8 | lo;
         }
-        return ExternTwoByteString::New(isolate, dst, str_len, error);
+        return ExternTwoByteString::New(isolate, dst, str_len);
       }
       if (reinterpret_cast<uintptr_t>(buf) % 2 != 0) {
         // Unaligned data still means we can't directly pass it to V8.
         char* dst = node::UncheckedMalloc(buflen);
         if (dst == nullptr) {
-          *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
+          isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate));
           return MaybeLocal<Value>();
         }
         memcpy(dst, buf, buflen);
         return ExternTwoByteString::New(
-            isolate, reinterpret_cast<uint16_t*>(dst), str_len, error);
+            isolate, reinterpret_cast<uint16_t*>(dst), str_len);
       }
       return ExternTwoByteString::NewFromCopy(
-          isolate, reinterpret_cast<const uint16_t*>(buf), str_len, error);
+          isolate, reinterpret_cast<const uint16_t*>(buf), str_len);
     }
 
     default:
@@ -636,11 +627,9 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
   }
 }
 
-
 MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
                                       const uint16_t* buf,
-                                      size_t buflen,
-                                      Local<Value>* error) {
+                                      size_t buflen) {
   if (buflen == 0) return String::Empty(isolate);
   CHECK_BUFLEN_IN_RANGE(buflen);
 
@@ -651,24 +640,23 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
   if constexpr (IsBigEndian()) {
     uint16_t* dst = node::UncheckedMalloc<uint16_t>(buflen);
     if (dst == nullptr) {
-      *error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
+      isolate->ThrowException(node::ERR_MEMORY_ALLOCATION_FAILED(isolate));
       return MaybeLocal<Value>();
     }
     size_t nbytes = buflen * sizeof(uint16_t);
     memcpy(dst, buf, nbytes);
     CHECK(nbytes::SwapBytes16(reinterpret_cast<char*>(dst), nbytes));
-    return ExternTwoByteString::New(isolate, dst, buflen, error);
+    return ExternTwoByteString::New(isolate, dst, buflen);
   } else {
-    return ExternTwoByteString::NewFromCopy(isolate, buf, buflen, error);
+    return ExternTwoByteString::NewFromCopy(isolate, buf, buflen);
   }
 }
 
 MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
                                       const char* buf,
-                                      enum encoding encoding,
-                                      Local<Value>* error) {
+                                      enum encoding encoding) {
   const size_t len = strlen(buf);
-  return Encode(isolate, buf, len, encoding, error);
+  return Encode(isolate, buf, len, encoding);
 }
 
 }  // namespace node

src/string_bytes.h

@@ -81,8 +81,7 @@ class StringBytes {
   static v8::MaybeLocal<v8::Value> Encode(v8::Isolate* isolate,
                                           const char* buf,
                                           size_t buflen,
-                                          enum encoding encoding,
-                                          v8::Local<v8::Value>* error);
+                                          enum encoding encoding);
 
   // Warning: This reverses endianness on BE platforms, even though the
   // signature using uint16_t implies that it should not.
@@ -90,13 +89,11 @@ class StringBytes {
   // be changed easily.
   static v8::MaybeLocal<v8::Value> Encode(v8::Isolate* isolate,
                                           const uint16_t* buf,
-                                          size_t buflen,
-                                          v8::Local<v8::Value>* error);
+                                          size_t buflen);
 
   static v8::MaybeLocal<v8::Value> Encode(v8::Isolate* isolate,
                                           const char* buf,
-                                          enum encoding encoding,
-                                          v8::Local<v8::Value>* error);
+                                          enum encoding encoding);
 
  private:
   static size_t WriteUCS2(v8::Isolate* isolate,

src/string_decoder.cc

@@ -28,7 +28,6 @@ MaybeLocal<String> MakeString(Isolate* isolate,
                               const char* data,
                               size_t length,
                               enum encoding encoding) {
-  Local<Value> error;
   MaybeLocal<Value> ret;
   if (encoding == UTF8) {
     MaybeLocal<String> utf8_string;
@@ -43,17 +42,11 @@ MaybeLocal<String> MakeString(Isolate* isolate,
       return utf8_string;
     }
   } else {
-    ret = StringBytes::Encode(
-        isolate,
-        data,
-        length,
-        encoding,
-        &error);
+    ret = StringBytes::Encode(isolate, data, length, encoding);
   }
 
   if (ret.IsEmpty()) {
-    CHECK(!error.IsEmpty());
-    isolate->ThrowException(error);
+    return {};
   }
 
   DCHECK(ret.IsEmpty() || ret.ToLocalChecked()->IsString());