Repository: nodejs/node
Pull request: src: improve error handling in node_url

Eliminate uses of USE and ToLocalChecked to properly propagate errors.

Also clean up `v8::` prefix uses and switch to using `ToV8Value(...)` where appropriate

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: src/node_url.cc
patch: @@ -26,17 +26,17 @@ using v8::FunctionCallbackInfo;
 using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
-using v8::NewStringType;
 using v8::Object;
 using v8::ObjectTemplate;
+using v8::SnapshotCreator;
 using v8::String;
 using v8::Value;
 
 void BindingData::MemoryInfo(MemoryTracker* tracker) const {
   tracker->TrackField("url_components_buffer", url_components_buffer_);
 }
 
-BindingData::BindingData(Realm* realm, v8::Local<v8::Object> object)
+BindingData::BindingData(Realm* realm, Local<Object> object)
     : SnapshotableObject(realm, object, type_int),
       url_components_buffer_(realm->isolate(), kURLComponentsLength) {
   object
@@ -47,8 +47,8 @@ BindingData::BindingData(Realm* realm, v8::Local<v8::Object> object)
   url_components_buffer_.MakeWeak();
 }
 
-bool BindingData::PrepareForSerialization(v8::Local<v8::Context> context,
-                                          v8::SnapshotCreator* creator) {
+bool BindingData::PrepareForSerialization(Local<Context> context,
+                                          SnapshotCreator* creator) {
   // We'll just re-initialize the buffers in the constructor since their
   // contents can be thrown away once consumed in the previous call.
   url_components_buffer_.Release();
@@ -64,12 +64,12 @@ InternalFieldInfoBase* BindingData::Serialize(int index) {
   return info;
 }
 
-void BindingData::Deserialize(v8::Local<v8::Context> context,
-                              v8::Local<v8::Object> holder,
+void BindingData::Deserialize(Local<Context> context,
+                              Local<Object> holder,
                               int index,
                               InternalFieldInfoBase* info) {
   DCHECK_IS_SNAPSHOT_SLOT(index);
-  v8::HandleScope scope(context->GetIsolate());
+  HandleScope scope(context->GetIsolate());
   Realm* realm = Realm::GetCurrent(context);
   BindingData* binding = realm->AddBindingData<BindingData>(holder);
   CHECK_NOT_NULL(binding);
@@ -173,8 +173,11 @@ void BindingData::PathToFileURL(const FunctionCallbackInfo<Value>& args) {
 
   binding_data->UpdateComponents(out->get_components(), out->type);
 
-  args.GetReturnValue().Set(
-      ToV8Value(realm->context(), out->get_href(), isolate).ToLocalChecked());
+  Local<Value> ret;
+  if (ToV8Value(realm->context(), out->get_href(), isolate).ToLocal(&ret))
+      [[likely]] {
+    args.GetReturnValue().Set(ret);
+  }
 }
 
 void BindingData::DomainToASCII(const FunctionCallbackInfo<Value>& args) {
@@ -196,8 +199,12 @@ void BindingData::DomainToASCII(const FunctionCallbackInfo<Value>& args) {
     return args.GetReturnValue().Set(String::Empty(env->isolate()));
   }
   std::string host = out->get_hostname();
-  args.GetReturnValue().Set(
-      String::NewFromUtf8(env->isolate(), host.c_str()).ToLocalChecked());
+
+  Local<Value> ret;
+  if (ToV8Value(env->context(), host, env->isolate()).ToLocal(&ret))
+      [[likely]] {
+    args.GetReturnValue().Set(ret);
+  }
 }
 
 void BindingData::DomainToUnicode(const FunctionCallbackInfo<Value>& args) {
@@ -220,14 +227,14 @@ void BindingData::DomainToUnicode(const FunctionCallbackInfo<Value>& args) {
   }
   std::string result = ada::idna::to_unicode(out->get_hostname());
 
-  args.GetReturnValue().Set(String::NewFromUtf8(env->isolate(),
-                                                result.c_str(),
-                                                NewStringType::kNormal,
-                                                result.length())
-                                .ToLocalChecked());
+  Local<Value> ret;
+  if (ToV8Value(env->context(), result, env->isolate()).ToLocal(&ret))
+      [[likely]] {
+    args.GetReturnValue().Set(ret);
+  }
 }
 
-void BindingData::GetOrigin(const v8::FunctionCallbackInfo<Value>& args) {
+void BindingData::GetOrigin(const FunctionCallbackInfo<Value>& args) {
   CHECK_GE(args.Length(), 1);
   CHECK(args[0]->IsString());  // input
 
@@ -244,11 +251,12 @@ void BindingData::GetOrigin(const v8::FunctionCallbackInfo<Value>& args) {
   }
 
   std::string origin = out->get_origin();
-  args.GetReturnValue().Set(String::NewFromUtf8(env->isolate(),
-                                                origin.data(),
-                                                NewStringType::kNormal,
-                                                origin.length())
-                                .ToLocalChecked());
+
+  Local<Value> ret;
+  if (ToV8Value(env->context(), origin, env->isolate()).ToLocal(&ret))
+      [[likely]] {
+    args.GetReturnValue().Set(ret);
+  }
 }
 
 void BindingData::CanParse(const FunctionCallbackInfo<Value>& args) {
@@ -328,11 +336,12 @@ void BindingData::Format(const FunctionCallbackInfo<Value>& args) {
   }
 
   std::string result = out->get_href();
-  args.GetReturnValue().Set(String::NewFromUtf8(env->isolate(),
-                                                result.data(),
-                                                NewStringType::kNormal,
-                                                result.length())
-                                .ToLocalChecked());
+
+  Local<Value> ret;
+  if (ToV8Value(env->context(), result, env->isolate()).ToLocal(&ret))
+      [[likely]] {
+    args.GetReturnValue().Set(ret);
+  }
 }
 
 void BindingData::Parse(const FunctionCallbackInfo<Value>& args) {
@@ -372,8 +381,11 @@ void BindingData::Parse(const FunctionCallbackInfo<Value>& args) {
 
   binding_data->UpdateComponents(out->get_components(), out->type);
 
-  args.GetReturnValue().Set(
-      ToV8Value(realm->context(), out->get_href(), isolate).ToLocalChecked());
+  Local<Value> ret;
+  if (ToV8Value(realm->context(), out->get_href(), isolate).ToLocal(&ret))
+      [[likely]] {
+    args.GetReturnValue().Set(ret);
+  }
 }
 
 void BindingData::Update(const FunctionCallbackInfo<Value>& args) {
@@ -446,8 +458,12 @@ void BindingData::Update(const FunctionCallbackInfo<Value>& args) {
   }
 
   binding_data->UpdateComponents(out->get_components(), out->type);
-  args.GetReturnValue().Set(
-      ToV8Value(realm->context(), out->get_href(), isolate).ToLocalChecked());
+
+  Local<Value> ret;
+  if (ToV8Value(realm->context(), out->get_href(), isolate).ToLocal(&ret))
+      [[likely]] {
+    args.GetReturnValue().Set(ret);
+  }
 }
 
 void BindingData::UpdateComponents(const ada::url_components& components,
@@ -513,22 +529,21 @@ void ThrowInvalidURL(node::Environment* env,
 
   auto err_object = err.As<Object>();
 
-  USE(err_object->Set(env->context(),
-                      env->input_string(),
-                      v8::String::NewFromUtf8(env->isolate(),
-                                              input.data(),
-                                              v8::NewStringType::kNormal,
-                                              input.size())
-                          .ToLocalChecked()));
+  Local<Value> tmp;
+  if (!ToV8Value(env->context(), input, env->isolate()).ToLocal(&tmp) ||
+      err_object->Set(env->context(), env->input_string(), tmp).IsNothing())
+      [[unlikely]] {
+    // A superseding error has been thrown.
+    return;
+  }
 
   if (base.has_value()) {
-    USE(err_object->Set(env->context(),
-                        env->base_string(),
-                        v8::String::NewFromUtf8(env->isolate(),
-                                                base.value().c_str(),
-                                                v8::NewStringType::kNormal,
-                                                base.value().size())
-                            .ToLocalChecked()));
+    if (!ToV8Value(env->context(), base.value(), env->isolate())
+             .ToLocal(&tmp) ||
+        err_object->Set(env->context(), env->base_string(), tmp).IsNothing())
+        [[unlikely]] {
+      return;
+    }
   }
 
   env->isolate()->ThrowException(err);