Browse Source

fix: stack traces in non-Node.js contexts (#26997)

* fix: stack traces in non-Node.js contexts

* update patches

Co-authored-by: Electron Bot <[email protected]>
Shelley Vohr 4 years ago
parent
commit
a4d73be9c5

+ 1 - 0
patches/node/.patches

@@ -48,3 +48,4 @@ n-api_src_provide_asynchronous_cleanup_hooks.patch
 fix_add_v8_enable_reverse_jsargs_defines_in_common_gypi.patch
 chore_expose_v8_initialization_isolate_callbacks.patch
 fix_add_safeforterminationscopes_for_sigint_interruptions.patch
+allow_preventing_preparestacktracecallback.patch

+ 44 - 0
patches/node/allow_preventing_preparestacktracecallback.patch

@@ -0,0 +1,44 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Shelley Vohr <[email protected]>
+Date: Mon, 7 Dec 2020 16:54:23 -0800
+Subject: Allow preventing PrepareStackTraceCallback
+
+Node.js sets a stack trace handler specific to the v8::Context
+corresponding to the current Environment. When we're running in a
+non-Node.js v8::Context, there will be no correspondent Environment - we
+therefore need to prevent this handler being set so that Blink falls back to its
+default handling and displays the correct stacktrace.
+
+diff --git a/src/api/environment.cc b/src/api/environment.cc
+index e42416b4807fcc9d35a93399b916968b45ed0c7a..adf033f2e1855ad1c9738f9746677566aabedd87 100644
+--- a/src/api/environment.cc
++++ b/src/api/environment.cc
+@@ -226,9 +226,11 @@ void SetIsolateErrorHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
+       s.fatal_error_callback : OnFatalError;
+   isolate->SetFatalErrorHandler(fatal_error_cb);
+ 
+-  auto* prepare_stack_trace_cb = s.prepare_stack_trace_callback ?
+-      s.prepare_stack_trace_callback : Environment::PrepareStackTraceCallback;
+-  isolate->SetPrepareStackTraceCallback(prepare_stack_trace_cb);
++  if ((s.flags & SHOULD_NOT_SET_PREPARE_STACK_TRACE_CALLBACK) == 0) {
++    auto* prepare_stack_trace_cb = s.prepare_stack_trace_callback ?
++        s.prepare_stack_trace_callback : Environment::PrepareStackTraceCallback;
++    isolate->SetPrepareStackTraceCallback(prepare_stack_trace_cb);
++  }
+ }
+ 
+ void SetIsolateMiscHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
+diff --git a/src/node.h b/src/node.h
+index 4c4e55e338d7b42c36818a45f6b41170c495adde..ad2727fbab366df0dcc60d7562951c953f952ae3 100644
+--- a/src/node.h
++++ b/src/node.h
+@@ -305,7 +305,8 @@ class NODE_EXTERN MultiIsolatePlatform : public v8::Platform {
+ enum IsolateSettingsFlags {
+   MESSAGE_LISTENER_WITH_ERROR_LEVEL = 1 << 0,
+   DETAILED_SOURCE_POSITIONS_FOR_PROFILING = 1 << 1,
+-  SHOULD_SET_PROMISE_REJECTION_CALLBACK = 1 << 2
++  SHOULD_SET_PROMISE_REJECTION_CALLBACK = 1 << 2,
++  SHOULD_NOT_SET_PREPARE_STACK_TRACE_CALLBACK = 1 << 3
+ };
+ 
+ struct IsolateSettings {

+ 7 - 0
shell/common/node_bindings.cc

@@ -506,6 +506,13 @@ node::Environment* NodeBindings::CreateEnvironment(
     // context. We need to use the one Blink already provides.
     is.flags &=
         ~node::IsolateSettingsFlags::SHOULD_SET_PROMISE_REJECTION_CALLBACK;
+
+    // We do not want to use the stack trace callback that Node.js uses,
+    // because it relies on Node.js being aware of the current Context and
+    // that's not always the case. We need to use the one Blink already
+    // provides.
+    is.flags |=
+        node::IsolateSettingsFlags::SHOULD_NOT_SET_PREPARE_STACK_TRACE_CALLBACK;
   }
 
   // This needs to be called before the inspector is initialized.