src: improve error handling in encoding_binding.cc

PR-URL: #56915
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Xuguang Mei <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>

Author: jasnell

src/encoding_binding.cc

@@ -20,7 +20,6 @@ using v8::Context;
 using v8::FunctionCallbackInfo;
 using v8::Isolate;
 using v8::Local;
-using v8::MaybeLocal;
 using v8::Object;
 using v8::ObjectTemplate;
 using v8::String;
@@ -139,8 +138,7 @@ void BindingData::EncodeUtf8String(const FunctionCallbackInfo<Value>& args) {
     ab = ArrayBuffer::New(isolate, std::move(bs));
   }
 
-  auto array = Uint8Array::New(ab, 0, length);
-  args.GetReturnValue().Set(array);
+  args.GetReturnValue().Set(Uint8Array::New(ab, 0, length));
 }
 
 // Convert the input into an encoded string
@@ -184,11 +182,10 @@ void BindingData::DecodeUTF8(const FunctionCallbackInfo<Value>& args) {
   if (length == 0) return args.GetReturnValue().SetEmptyString();
 
   Local<Value> error;
-  MaybeLocal<Value> maybe_ret =
-      StringBytes::Encode(env->isolate(), data, length, UTF8, &error);
   Local<Value> ret;
 
-  if (!maybe_ret.ToLocal(&ret)) {
+  if (!StringBytes::Encode(env->isolate(), data, length, UTF8, &error)
+           .ToLocal(&ret)) {
     CHECK(!error.IsEmpty());
     env->isolate()->ThrowException(error);
     return;
@@ -204,8 +201,10 @@ void BindingData::ToASCII(const v8::FunctionCallbackInfo<v8::Value>& args) {
 
   Utf8Value input(env->isolate(), args[0]);
   auto out = ada::idna::to_ascii(input.ToStringView());
-  args.GetReturnValue().Set(
-      String::NewFromUtf8(env->isolate(), out.c_str()).ToLocalChecked());
+  Local<Value> ret;
+  if (ToV8Value(env->context(), out, env->isolate()).ToLocal(&ret)) {
+    args.GetReturnValue().Set(ret);
+  }
 }
 
 void BindingData::ToUnicode(const v8::FunctionCallbackInfo<v8::Value>& args) {
@@ -215,8 +214,10 @@ void BindingData::ToUnicode(const v8::FunctionCallbackInfo<v8::Value>& args) {
 
   Utf8Value input(env->isolate(), args[0]);
   auto out = ada::idna::to_unicode(input.ToStringView());
-  args.GetReturnValue().Set(
-      String::NewFromUtf8(env->isolate(), out.c_str()).ToLocalChecked());
+  Local<Value> ret;
+  if (ToV8Value(env->context(), out, env->isolate()).ToLocal(&ret)) {
+    args.GetReturnValue().Set(ret);
+  }
 }
 
 void BindingData::CreatePerIsolateProperties(IsolateData* isolate_data,
@@ -286,11 +287,12 @@ void BindingData::DecodeLatin1(const FunctionCallbackInfo<Value>& args) {
         env->isolate(), "The encoded data was not valid for encoding latin1");
   }
 
-  Local<String> output =
-      String::NewFromUtf8(
-          env->isolate(), result.c_str(), v8::NewStringType::kNormal, written)
-          .ToLocalChecked();
-  args.GetReturnValue().Set(output);
+  std::string_view view(result.c_str(), written);
+
+  Local<Value> ret;
+  if (ToV8Value(env->context(), view, env->isolate()).ToLocal(&ret)) {
+    args.GetReturnValue().Set(ret);
+  }
 }
 
 }  // namespace encoding_binding

src/inspector_agent.cc

@@ -51,6 +51,7 @@ using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
 using v8::Message;
+using v8::Name;
 using v8::Object;
 using v8::Value;
 using v8_inspector::StringBuffer;
@@ -411,12 +412,12 @@ class SameThreadInspectorSession : public InspectorSession {
 void NotifyClusterWorkersDebugEnabled(Environment* env) {
   Isolate* isolate = env->isolate();
   HandleScope handle_scope(isolate);
-  Local<Context> context = env->context();
 
   // Send message to enable debug in cluster workers
-  Local<Object> message = Object::New(isolate);
-  message->Set(context, FIXED_ONE_BYTE_STRING(isolate, "cmd"),
-               FIXED_ONE_BYTE_STRING(isolate, "NODE_DEBUG_ENABLED")).Check();
+  Local<Name> name = FIXED_ONE_BYTE_STRING(isolate, "cmd");
+  Local<Value> value = FIXED_ONE_BYTE_STRING(isolate, "NODE_DEBUG_ENABLED");
+  Local<Object> message =
+      Object::New(isolate, Object::New(isolate), &name, &value, 1);
   ProcessEmit(env, "internalMessage", message);
 }
 
@@ -441,11 +442,13 @@ bool IsFilePath(const std::string& path) {
 void ThrowUninitializedInspectorError(Environment* env) {
   HandleScope scope(env->isolate());
 
-  const char* msg = "This Environment was initialized without a V8::Inspector";
-  Local<Value> exception =
-    v8::String::NewFromUtf8(env->isolate(), msg).ToLocalChecked();
-
-  env->isolate()->ThrowException(exception);
+  std::string_view msg =
+      "This Environment was initialized without a V8::Inspector";
+  Local<Value> exception;
+  if (ToV8Value(env->context(), msg, env->isolate()).ToLocal(&exception)) {
+    env->isolate()->ThrowException(exception);
+  }
+  // V8 will have scheduled a superseding error here.
 }
 
 }  // namespace

src/inspector_js_api.cc

@@ -188,11 +188,12 @@ void CallAndPauseOnStart(const FunctionCallbackInfo<v8::Value>& args) {
   CHECK(args[0]->IsFunction());
   SlicedArguments call_args(args, /* start */ 2);
   env->inspector_agent()->PauseOnNextJavascriptStatement("Break on start");
-  v8::MaybeLocal<v8::Value> retval =
-      args[0].As<v8::Function>()->Call(env->context(), args[1],
-                                       call_args.length(), call_args.out());
-  if (!retval.IsEmpty()) {
-    args.GetReturnValue().Set(retval.ToLocalChecked());
+  Local<Value> ret;
+  if (args[0]
+          .As<v8::Function>()
+          ->Call(env->context(), args[1], call_args.length(), call_args.out())
+          .ToLocal(&ret)) {
+    args.GetReturnValue().Set(ret);
   }
 }
 

src/node_contextify.cc

@@ -111,9 +111,8 @@ using v8::Value;
 namespace {
 
 // Convert an int to a V8 Name (String or Symbol).
-Local<Name> Uint32ToName(Local<Context> context, uint32_t index) {
-  return Uint32::New(context->GetIsolate(), index)->ToString(context)
-      .ToLocalChecked();
+MaybeLocal<String> Uint32ToName(Local<Context> context, uint32_t index) {
+  return Uint32::New(context->GetIsolate(), index)->ToString(context);
 }
 
 }  // anonymous namespace
@@ -852,8 +851,11 @@ Intercepted ContextifyContext::IndexedPropertyQueryCallback(
     return Intercepted::kNo;
   }
 
-  return ContextifyContext::PropertyQueryCallback(
-      Uint32ToName(ctx->context(), index), args);
+  Local<String> name;
+  if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
+    return ContextifyContext::PropertyQueryCallback(name, args);
+  }
+  return Intercepted::kNo;
 }
 
 // static
@@ -866,8 +868,11 @@ Intercepted ContextifyContext::IndexedPropertyGetterCallback(
     return Intercepted::kNo;
   }
 
-  return ContextifyContext::PropertyGetterCallback(
-      Uint32ToName(ctx->context(), index), args);
+  Local<String> name;
+  if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
+    return ContextifyContext::PropertyGetterCallback(name, args);
+  }
+  return Intercepted::kNo;
 }
 
 Intercepted ContextifyContext::IndexedPropertySetterCallback(
@@ -881,8 +886,11 @@ Intercepted ContextifyContext::IndexedPropertySetterCallback(
     return Intercepted::kNo;
   }
 
-  return ContextifyContext::PropertySetterCallback(
-      Uint32ToName(ctx->context(), index), value, args);
+  Local<String> name;
+  if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
+    return ContextifyContext::PropertySetterCallback(name, value, args);
+  }
+  return Intercepted::kNo;
 }
 
 // static
@@ -895,8 +903,11 @@ Intercepted ContextifyContext::IndexedPropertyDescriptorCallback(
     return Intercepted::kNo;
   }
 
-  return ContextifyContext::PropertyDescriptorCallback(
-      Uint32ToName(ctx->context(), index), args);
+  Local<String> name;
+  if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
+    return ContextifyContext::PropertyDescriptorCallback(name, args);
+  }
+  return Intercepted::kNo;
 }
 
 Intercepted ContextifyContext::IndexedPropertyDefinerCallback(
@@ -910,8 +921,11 @@ Intercepted ContextifyContext::IndexedPropertyDefinerCallback(
     return Intercepted::kNo;
   }
 
-  return ContextifyContext::PropertyDefinerCallback(
-      Uint32ToName(ctx->context(), index), desc, args);
+  Local<String> name;
+  if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
+    return ContextifyContext::PropertyDefinerCallback(name, desc, args);
+  }
+  return Intercepted::kNo;
 }
 
 // static
@@ -1130,22 +1144,20 @@ Maybe<void> StoreCodeCacheResult(
   if (produce_cached_data) {
     bool cached_data_produced = new_cached_data != nullptr;
     if (cached_data_produced) {
-      MaybeLocal<Object> buf =
-          Buffer::Copy(env,
-                       reinterpret_cast<const char*>(new_cached_data->data),
-                       new_cached_data->length);
-      if (target->Set(context, env->cached_data_string(), buf.ToLocalChecked())
+      Local<Object> buf;
+      if (!Buffer::Copy(env,
+                        reinterpret_cast<const char*>(new_cached_data->data),
+                        new_cached_data->length)
+               .ToLocal(&buf) ||
+          target->Set(context, env->cached_data_string(), buf).IsNothing() ||
+          target
+              ->Set(context,
+                    env->cached_data_produced_string(),
+                    Boolean::New(env->isolate(), cached_data_produced))
               .IsNothing()) {
         return Nothing<void>();
       }
     }
-    if (target
-            ->Set(context,
-                  env->cached_data_produced_string(),
-                  Boolean::New(env->isolate(), cached_data_produced))
-            .IsNothing()) {
-      return Nothing<void>();
-    }
   }
   return JustVoid();
 }
@@ -1179,14 +1191,19 @@ void ContextifyScript::CreateCachedData(
   ASSIGN_OR_RETURN_UNWRAP_CPPGC(&wrapped_script, args.This());
   std::unique_ptr<ScriptCompiler::CachedData> cached_data(
       ScriptCompiler::CreateCodeCache(wrapped_script->unbound_script()));
-  if (!cached_data) {
-    args.GetReturnValue().Set(Buffer::New(env, 0).ToLocalChecked());
-  } else {
-    MaybeLocal<Object> buf = Buffer::Copy(
-        env,
-        reinterpret_cast<const char*>(cached_data->data),
-        cached_data->length);
-    args.GetReturnValue().Set(buf.ToLocalChecked());
+
+  auto maybeRet = ([&] {
+    if (!cached_data) {
+      return Buffer::New(env, 0);
+    }
+    return Buffer::Copy(env,
+                        reinterpret_cast<const char*>(cached_data->data),
+                        cached_data->length);
+  })();
+
+  Local<Object> ret;
+  if (maybeRet.ToLocal(&ret)) {
+    args.GetReturnValue().Set(ret);
   }
 }
 

src/node_credentials.cc

@@ -26,7 +26,6 @@ using v8::Context;
 using v8::FunctionCallbackInfo;
 using v8::Isolate;
 using v8::Local;
-using v8::MaybeLocal;
 using v8::Object;
 using v8::Uint32;
 using v8::Value;
@@ -111,9 +110,10 @@ static void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
   Utf8Value strenvtag(isolate, args[0]);
   std::string text;
   if (!SafeGetenv(*strenvtag, &text, env)) return;
-  Local<Value> result =
-      ToV8Value(isolate->GetCurrentContext(), text).ToLocalChecked();
-  args.GetReturnValue().Set(result);
+  Local<Value> result;
+  if (ToV8Value(isolate->GetCurrentContext(), text).ToLocal(&result)) {
+    args.GetReturnValue().Set(result);
+  }
 }
 
 static void GetTempDir(const FunctionCallbackInfo<Value>& args) {
@@ -137,8 +137,10 @@ static void GetTempDir(const FunctionCallbackInfo<Value>& args) {
     dir.pop_back();
   }
 
-  args.GetReturnValue().Set(
-      ToV8Value(isolate->GetCurrentContext(), dir).ToLocalChecked());
+  Local<Value> result;
+  if (ToV8Value(isolate->GetCurrentContext(), dir).ToLocal(&result)) {
+    args.GetReturnValue().Set(result);
+  }
 }
 
 #ifdef NODE_IMPLEMENTS_POSIX_CREDENTIALS
@@ -385,9 +387,10 @@ static void GetGroups(const FunctionCallbackInfo<Value>& args) {
   gid_t egid = getegid();
   if (std::find(groups.begin(), groups.end(), egid) == groups.end())
     groups.push_back(egid);
-  MaybeLocal<Value> array = ToV8Value(env->context(), groups);
-  if (!array.IsEmpty())
-    args.GetReturnValue().Set(array.ToLocalChecked());
+  Local<Value> result;
+  if (ToV8Value(env->context(), groups).ToLocal(&result)) {
+    args.GetReturnValue().Set(result);
+  }
 }
 
 static void SetGroups(const FunctionCallbackInfo<Value>& args) {
@@ -403,8 +406,12 @@ static void SetGroups(const FunctionCallbackInfo<Value>& args) {
   MaybeStackBuffer<gid_t, 64> groups(size);
 
   for (size_t i = 0; i < size; i++) {
-    gid_t gid = gid_by_name(
-        env->isolate(), groups_list->Get(env->context(), i).ToLocalChecked());
+    Local<Value> val;
+    if (!groups_list->Get(env->context(), i).ToLocal(&val)) {
+      // V8 will have scheduled an error to be thrown.
+      return;
+    }
+    gid_t gid = gid_by_name(env->isolate(), val);
 
     if (gid == gid_not_found) {
       // Tells JS to throw ERR_INVALID_CREDENTIAL