src: add missing TryCatch

Otherwise re-entering V8 doesn't work as expected after exceptions were thrown. Refs: https://chromium-review.googlesource.com/c/v8/v8/+/5050065 Co-Authored-By: Toon Verwaest <[email protected]> Co-Authored-By: deepak1556 <[email protected]> PR-URL: nodejs#51362 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
joyeecheung · Jan 22, 2025 · 6c2f360 · 6c2f360
1 parent d0b6ea1
commit 6c2f360
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 0 deletions.
diff --git a/src/js_stream.cc b/src/js_stream.cc
@@ -20,6 +20,7 @@ using v8::Int32;
 using v8::Isolate;
 using v8::Local;
 using v8::Object;
+using v8::TryCatch;
 using v8::Value;
 
 
@@ -169,6 +170,8 @@ void JSStream::ReadBuffer(const FunctionCallbackInfo<Value>& args) {
   const char* data = buffer.data();
   int len = buffer.length();
 
+  TryCatch try_catch(args.GetIsolate());
+
   // Repeatedly ask the stream's owner for memory, copy the data that we
   // just read from JS into those buffers and emit them as reads.
   while (len != 0) {
@@ -182,6 +185,10 @@ void JSStream::ReadBuffer(const FunctionCallbackInfo<Value>& args) {
     len -= static_cast<int>(avail);
     wrap->EmitRead(avail, buf);
   }
+
+  if (try_catch.HasCaught()) {
+    try_catch.ReThrow();
+  }
 }
 
 

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -489,13 +489,16 @@ bool ContextifyContext::IsStillInitializing(const ContextifyContext* ctx) {
 void ContextifyContext::PropertyGetterCallback(
     Local<Name> property,
     const PropertyCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
   ContextifyContext* ctx = ContextifyContext::Get(args);
 
   // Still initializing
   if (IsStillInitializing(ctx)) return;
 
   Local<Context> context = ctx->context();
   Local<Object> sandbox = ctx->sandbox();
+
+  TryCatchScope try_catch(env);
   MaybeLocal<Value> maybe_rv =
       sandbox->GetRealNamedProperty(context, property);
   if (maybe_rv.IsEmpty()) {
@@ -505,6 +508,9 @@ void ContextifyContext::PropertyGetterCallback(
 
   Local<Value> rv;
   if (maybe_rv.ToLocal(&rv)) {
+    if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
+      try_catch.ReThrow();
+    }
     if (rv == sandbox)
       rv = ctx->global_proxy();
 

diff --git a/src/node_messaging.cc b/src/node_messaging.cc
@@ -907,6 +907,7 @@ Maybe<bool> MessagePort::PostMessage(Environment* env,
                                      const TransferList& transfer_v) {
   Isolate* isolate = env->isolate();
   Local<Object> obj = object(isolate);
+  TryCatchScope try_catch(env);
 
   std::shared_ptr<Message> msg = std::make_shared<Message>();
 
@@ -915,6 +916,9 @@ Maybe<bool> MessagePort::PostMessage(Environment* env,
 
   Maybe<bool> serialization_maybe =
       msg->Serialize(env, context, message_v, transfer_v, obj);
+  if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
+    try_catch.ReThrow();
+  }
   if (data_ == nullptr) {
     return serialization_maybe;
   }

diff --git a/test/addons/make-callback-recurse/binding.cc b/test/addons/make-callback-recurse/binding.cc
@@ -8,6 +8,7 @@ using v8::FunctionCallbackInfo;
 using v8::Isolate;
 using v8::Local;
 using v8::Object;
+using v8::TryCatch;
 using v8::Value;
 
 namespace {
@@ -19,8 +20,12 @@ void MakeCallback(const FunctionCallbackInfo<Value>& args) {
   Local<Object> recv = args[0].As<Object>();
   Local<Function> method = args[1].As<Function>();
 
+  TryCatch try_catch(isolate);
   node::MakeCallback(isolate, recv, method, 0, nullptr,
                      node::async_context{0, 0});
+  if (try_catch.HasCaught()) {
+    try_catch.ReThrow();
+  }
 }
 
 void Initialize(Local<Object> exports) {