From 41e4953832be72d60c309738935b17f1bcb86a41 Mon Sep 17 00:00:00 2001
From: Chengzhong Wu <cwu631@bloomberg.net>
Date: Fri, 23 May 2025 10:20:14 +0100
Subject: [PATCH] src: improve CompileFunctionAndCacheResult error handling
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

PR-URL: https://github.com/nodejs/node/pull/58434
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
---
 src/node_contextify.cc | 43 +++++++++++++++++++++---------------------
 src/node_contextify.h  |  2 +-
 2 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index c0e84dbca0..520542e291 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -1511,24 +1511,22 @@ void ContextifyContext::CompileFunction(
   }
 
   TryCatchScope try_catch(env);
-  Local<Object> result = CompileFunctionAndCacheResult(env,
-                                                       parsing_context,
-                                                       &source,
-                                                       params,
-                                                       context_extensions,
-                                                       options,
-                                                       produce_cached_data,
-                                                       id_symbol,
-                                                       try_catch);
-
-  if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
+  MaybeLocal<Object> maybe_result =
+      CompileFunctionAndCacheResult(env,
+                                    parsing_context,
+                                    &source,
+                                    params,
+                                    context_extensions,
+                                    options,
+                                    produce_cached_data,
+                                    id_symbol,
+                                    try_catch);
+  Local<Object> result;
+  if (!maybe_result.ToLocal(&result)) {
+    CHECK(try_catch.HasCaught());
     try_catch.ReThrow();
     return;
   }
-
-  if (result.IsEmpty()) {
-    return;
-  }
   args.GetReturnValue().Set(result);
 }
 
@@ -1544,7 +1542,7 @@ static LocalVector<String> GetCJSParameters(IsolateData* data) {
   return result;
 }
 
-Local<Object> ContextifyContext::CompileFunctionAndCacheResult(
+MaybeLocal<Object> ContextifyContext::CompileFunctionAndCacheResult(
     Environment* env,
     Local<Context> parsing_context,
     ScriptCompiler::Source* source,
@@ -1566,28 +1564,29 @@ Local<Object> ContextifyContext::CompileFunctionAndCacheResult(
 
   Local<Function> fn;
   if (!maybe_fn.ToLocal(&fn)) {
-    if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
+    CHECK(try_catch.HasCaught());
+    if (!try_catch.HasTerminated()) {
       errors::DecorateErrorStack(env, try_catch);
-      return Object::New(env->isolate());
     }
+    return {};
   }
 
   Local<Context> context = env->context();
   if (fn->SetPrivate(context, env->host_defined_option_symbol(), id_symbol)
           .IsNothing()) {
-    return Object::New(env->isolate());
+    return {};
   }
 
   Isolate* isolate = env->isolate();
   Local<Object> result = Object::New(isolate);
   if (result->Set(parsing_context, env->function_string(), fn).IsNothing())
-    return Object::New(env->isolate());
+    return {};
   if (result
           ->Set(parsing_context,
                 env->source_map_url_string(),
                 fn->GetScriptOrigin().SourceMapUrl())
           .IsNothing())
-    return Object::New(env->isolate());
+    return {};
 
   std::unique_ptr<ScriptCompiler::CachedData> new_cached_data;
   if (produce_cached_data) {
@@ -1600,7 +1599,7 @@ Local<Object> ContextifyContext::CompileFunctionAndCacheResult(
                            produce_cached_data,
                            std::move(new_cached_data))
           .IsNothing()) {
-    return Object::New(env->isolate());
+    return {};
   }
 
   return result;
diff --git a/src/node_contextify.h b/src/node_contextify.h
index 7e5d90459b..cf5b3eb8f9 100644
--- a/src/node_contextify.h
+++ b/src/node_contextify.h
@@ -146,7 +146,7 @@ class ContextifyContext final : CPPGC_MIXIN(ContextifyContext) {
   static void IsContext(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void CompileFunction(
       const v8::FunctionCallbackInfo<v8::Value>& args);
-  static v8::Local<v8::Object> CompileFunctionAndCacheResult(
+  static v8::MaybeLocal<v8::Object> CompileFunctionAndCacheResult(
       Environment* env,
       v8::Local<v8::Context> parsing_context,
       v8::ScriptCompiler::Source* source,