Repository: nodejs/node
Pull request: src: simply uint32 to string as it must not fail

`v8::Uint32::ToString` does not return an empty local handle. V8 internal [`NumberToString`](https://github.com/nodejs/node/blob/fc203b36f4930968f978a8d3c5248efc46a81595/deps/v8/src/heap/factory-base.cc#L1066) does not return an empty handle. It should be safe to assert in `node::Uint32ToString` that this function returns a non empty string handle, and remove the redundant error recovering logics.

Refs: https://github.com/nodejs/node/pull/53959

Changed files:
filename: src/node_contextify.cc
patch: @@ -85,7 +85,6 @@ using v8::ScriptCompiler;
 using v8::ScriptOrigin;
 using v8::String;
 using v8::Symbol;
-using v8::Uint32;
 using v8::UnboundScript;
 using v8::Value;
 
@@ -109,15 +108,6 @@ using v8::Value;
 // For every `set` of a global property, the interceptor callback defines or
 // changes the property both on the sandbox and the global proxy.
 
-namespace {
-
-// Convert an int to a V8 Name (String or Symbol).
-MaybeLocal<String> Uint32ToName(Local<Context> context, uint32_t index) {
-  return Uint32::New(Isolate::GetCurrent(), index)->ToString(context);
-}
-
-}  // anonymous namespace
-
 ContextifyContext* ContextifyContext::New(Environment* env,
                                           Local<Object> sandbox_obj,
                                           ContextOptions* options) {
@@ -849,11 +839,8 @@ Intercepted ContextifyContext::IndexedPropertyQueryCallback(
     return Intercepted::kNo;
   }
 
-  Local<String> name;
-  if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
-    return ContextifyContext::PropertyQueryCallback(name, args);
-  }
-  return Intercepted::kNo;
+  Local<String> name = Uint32ToString(ctx->context(), index);
+  return ContextifyContext::PropertyQueryCallback(name, args);
 }
 
 // static
@@ -866,11 +853,8 @@ Intercepted ContextifyContext::IndexedPropertyGetterCallback(
     return Intercepted::kNo;
   }
 
-  Local<String> name;
-  if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
-    return ContextifyContext::PropertyGetterCallback(name, args);
-  }
-  return Intercepted::kNo;
+  Local<String> name = Uint32ToString(ctx->context(), index);
+  return ContextifyContext::PropertyGetterCallback(name, args);
 }
 
 Intercepted ContextifyContext::IndexedPropertySetterCallback(
@@ -884,11 +868,8 @@ Intercepted ContextifyContext::IndexedPropertySetterCallback(
     return Intercepted::kNo;
   }
 
-  Local<String> name;
-  if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
-    return ContextifyContext::PropertySetterCallback(name, value, args);
-  }
-  return Intercepted::kNo;
+  Local<String> name = Uint32ToString(ctx->context(), index);
+  return ContextifyContext::PropertySetterCallback(name, value, args);
 }
 
 // static
@@ -901,11 +882,8 @@ Intercepted ContextifyContext::IndexedPropertyDescriptorCallback(
     return Intercepted::kNo;
   }
 
-  Local<String> name;
-  if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
-    return ContextifyContext::PropertyDescriptorCallback(name, args);
-  }
-  return Intercepted::kNo;
+  Local<String> name = Uint32ToString(ctx->context(), index);
+  return ContextifyContext::PropertyDescriptorCallback(name, args);
 }
 
 Intercepted ContextifyContext::IndexedPropertyDefinerCallback(
@@ -919,11 +897,8 @@ Intercepted ContextifyContext::IndexedPropertyDefinerCallback(
     return Intercepted::kNo;
   }
 
-  Local<String> name;
-  if (Uint32ToName(ctx->context(), index).ToLocal(&name)) {
-    return ContextifyContext::PropertyDefinerCallback(name, desc, args);
-  }
-  return Intercepted::kNo;
+  Local<String> name = Uint32ToString(ctx->context(), index);
+  return ContextifyContext::PropertyDefinerCallback(name, desc, args);
 }
 
 // static

filename: src/node_webstorage.cc
patch: @@ -41,7 +41,6 @@ using v8::PropertyCallbackInfo;
 using v8::PropertyDescriptor;
 using v8::PropertyHandlerFlags;
 using v8::String;
-using v8::Uint32;
 using v8::Value;
 
 #define THROW_SQLITE_ERROR(env, r)                                             \
@@ -432,10 +431,6 @@ Maybe<void> Storage::Store(Local<Name> key, Local<Value> value) {
   return JustVoid();
 }
 
-static MaybeLocal<String> Uint32ToName(Local<Context> context, uint32_t index) {
-  return Uint32::New(Isolate::GetCurrent(), index)->ToString(context);
-}
-
 static void Clear(const FunctionCallbackInfo<Value>& info) {
   Storage* storage;
   ASSIGN_OR_RETURN_UNWRAP(&storage, info.This());
@@ -631,67 +626,37 @@ static Intercepted StorageDefiner(Local<Name> property,
 static Intercepted IndexedGetter(uint32_t index,
                                  const PropertyCallbackInfo<Value>& info) {
   Environment* env = Environment::GetCurrent(info);
-  Local<Name> name;
-  if (!Uint32ToName(env->context(), index).ToLocal(&name)) {
-    // There was an error converting the index to a name.
-    // We aren't going to return a result but let's indicate
-    // that we intercepted the operation.
-    return Intercepted::kYes;
-  }
+  Local<Name> name = Uint32ToString(env->context(), index);
   return StorageGetter(name, info);
 }
 
 static Intercepted IndexedSetter(uint32_t index,
                                  Local<Value> value,
                                  const PropertyCallbackInfo<void>& info) {
   Environment* env = Environment::GetCurrent(info);
-  Local<Name> name;
-  if (!Uint32ToName(env->context(), index).ToLocal(&name)) {
-    // There was an error converting the index to a name.
-    // We aren't going to return a result but let's indicate
-    // that we intercepted the operation.
-    return Intercepted::kYes;
-  }
+  Local<Name> name = Uint32ToString(env->context(), index);
   return StorageSetter(name, value, info);
 }
 
 static Intercepted IndexedQuery(uint32_t index,
                                 const PropertyCallbackInfo<Integer>& info) {
   Environment* env = Environment::GetCurrent(info);
-  Local<Name> name;
-  if (!Uint32ToName(env->context(), index).ToLocal(&name)) {
-    // There was an error converting the index to a name.
-    // We aren't going to return a result but let's indicate
-    // that we intercepted the operation.
-    return Intercepted::kYes;
-  }
+  Local<Name> name = Uint32ToString(env->context(), index);
   return StorageQuery(name, info);
 }
 
 static Intercepted IndexedDeleter(uint32_t index,
                                   const PropertyCallbackInfo<Boolean>& info) {
   Environment* env = Environment::GetCurrent(info);
-  Local<Name> name;
-  if (!Uint32ToName(env->context(), index).ToLocal(&name)) {
-    // There was an error converting the index to a name.
-    // We aren't going to return a result but let's indicate
-    // that we intercepted the operation.
-    return Intercepted::kYes;
-  }
+  Local<Name> name = Uint32ToString(env->context(), index);
   return StorageDeleter(name, info);
 }
 
 static Intercepted IndexedDefiner(uint32_t index,
                                   const PropertyDescriptor& desc,
                                   const PropertyCallbackInfo<void>& info) {
   Environment* env = Environment::GetCurrent(info);
-  Local<Name> name;
-  if (!Uint32ToName(env->context(), index).ToLocal(&name)) {
-    // There was an error converting the index to a name.
-    // We aren't going to return a result but let's indicate
-    // that we intercepted the operation.
-    return Intercepted::kYes;
-  }
+  Local<Name> name = Uint32ToString(env->context(), index);
   return StorageDefiner(name, desc, info);
 }
 

filename: src/util.h
patch: @@ -1063,6 +1063,16 @@ inline v8::MaybeLocal<v8::Object> NewDictionaryInstanceNullProto(
     v8::Local<v8::DictionaryTemplate> tmpl,
     v8::MemorySpan<v8::MaybeLocal<v8::Value>> property_values);
 
+// Convert an uint32 to a V8 String.
+inline v8::Local<v8::String> Uint32ToString(v8::Local<v8::Context> context,
+                                            uint32_t index) {
+  // V8 internally caches strings for small integers, and asserts that a
+  // non-empty string local handle is returned for `ToString`.
+  return v8::Uint32::New(v8::Isolate::GetCurrent(), index)
+      ->ToString(context)
+      .ToLocalChecked();
+}
+
 }  // namespace node
 
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS