Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: assert module in the renderer process #39624

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions patches/node/.patches
Expand Up @@ -38,3 +38,4 @@ fix_account_for_createexternalizablestring_v8_global.patch
fix_wunreachable-code_warning_in_ares_init_rand_engine.patch
fix_do_not_resolve_electron_entrypoints.patch
dns_expose_getdefaultresultorder.patch
fix_assert_module_in_the_renderer_process.patch
75 changes: 75 additions & 0 deletions patches/node/fix_assert_module_in_the_renderer_process.patch
@@ -0,0 +1,75 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
Date: Wed, 16 Aug 2023 19:15:29 +0200
Subject: fix: assert module in the renderer process

When creating a Node.js Environment, embedders have the option to disable Node.js'
default overriding of Error.prepareStackTrace. However, the assert module depends on
a WeakMap that is populated with the error stacktraces in the overridden function.

This adds handling to fall back to the default implementation if Error.prepareStackTrace
if the override has been disabled.

This will be upstreamed.

diff --git a/lib/assert.js b/lib/assert.js
index 04c2dd3bfcfdfbb4b8079c306e1d80aa48027787..34658819d09cc20f372798caec79e19c4a36565d 100644
--- a/lib/assert.js
+++ b/lib/assert.js
@@ -66,6 +66,7 @@ const { inspect } = require('internal/util/inspect');
const { isPromise, isRegExp } = require('internal/util/types');
const { EOL } = require('internal/constants');
const { BuiltinModule } = require('internal/bootstrap/loaders');
+const { getEmbedderOptions } = require('internal/options');
const { isError } = require('internal/util');

const errorCache = new SafeMap();
@@ -293,8 +294,16 @@ function getErrMessage(message, fn) {
ErrorCaptureStackTrace(err, fn);
if (errorStackTraceLimitIsWritable) Error.stackTraceLimit = tmpLimit;

- overrideStackTrace.set(err, (_, stack) => stack);
- const call = err.stack[0];
+ let call;
+ if (getEmbedderOptions().hasPrepareStackTraceCallback) {
+ overrideStackTrace.set(err, (_, stack) => stack);
+ call = err.stack[0];
+ } else {
+ const tmpPrepare = Error.prepareStackTrace;
+ Error.prepareStackTrace = (_, stack) => stack;
+ call = err.stack[0];
+ Error.prepareStackTrace = tmpPrepare;
+ }

const filename = call.getFileName();
const line = call.getLineNumber() - 1;
diff --git a/src/api/environment.cc b/src/api/environment.cc
index 53db99ed5d2d9811eef2db26021c982890cc9cbe..7732f17c48a1e9db71b6c4dacfe08d686182c174 100644
--- a/src/api/environment.cc
+++ b/src/api/environment.cc
@@ -262,6 +262,9 @@ void SetIsolateErrorHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
auto* prepare_stack_trace_cb = s.prepare_stack_trace_callback ?
s.prepare_stack_trace_callback : PrepareStackTraceCallback;
isolate->SetPrepareStackTraceCallback(prepare_stack_trace_cb);
+ } else {
+ auto env = Environment::GetCurrent(isolate);
+ env->set_prepare_stack_trace_callback(Local<Function>());
}
}

diff --git a/src/node_options.cc b/src/node_options.cc
index 8bbee83ddb32a31ada18d7a6e8be7cdf33321b69..8765936e97a0fb2a3cf6ed4f2ae6ef539d778da1 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -1211,6 +1211,11 @@ void GetEmbedderOptions(const FunctionCallbackInfo<Value>& args) {
Local<Context> context = env->context();
Local<Object> ret = Object::New(isolate);

+ if (ret->Set(context,
+ FIXED_ONE_BYTE_STRING(env->isolate(), "hasPrepareStackTraceCallback"),
+ Boolean::New(isolate, !env->prepare_stack_trace_callback().IsEmpty()))
+ .IsNothing()) return;
+
if (ret->Set(context,
FIXED_ONE_BYTE_STRING(env->isolate(), "shouldNotRegisterESMLoader"),
Boolean::New(isolate, env->should_not_register_esm_loader()))