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(ext/node): don't rely on Deno.env to read NODE_DEBUG #23694

Merged
merged 3 commits into from
May 5, 2024
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 cli/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,7 @@ impl CliFactory {
.options
.take_binary_npm_command_name()
.or(std::env::args().next()),
node_debug: std::env::var("NODE_DEBUG").ok(),
origin_data_folder_path: Some(self.deno_dir()?.origin_data_folder_path()),
seed: self.options.seed(),
unsafely_ignore_certificate_errors: self
Expand Down
1 change: 1 addition & 0 deletions cli/standalone/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,7 @@ pub async fn run(
.ok()
.map(|req_ref| npm_pkg_req_ref_to_binary_command(&req_ref))
.or(std::env::args().next()),
node_debug: std::env::var("NODE_DEBUG").ok(),
origin_data_folder_path: None,
seed: metadata.seed,
unsafely_ignore_certificate_errors: metadata
Expand Down
3 changes: 3 additions & 0 deletions cli/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ pub struct CliMainWorkerOptions {
pub is_npm_main: bool,
pub location: Option<Url>,
pub argv0: Option<String>,
pub node_debug: Option<String>,
pub origin_data_folder_path: Option<PathBuf>,
pub seed: Option<u64>,
pub unsafely_ignore_certificate_errors: Option<Vec<String>>,
Expand Down Expand Up @@ -607,6 +608,7 @@ impl CliMainWorkerFactory {
inspect: shared.options.is_inspecting,
has_node_modules_dir: shared.options.has_node_modules_dir,
argv0: shared.options.argv0.clone(),
node_debug: shared.options.node_debug.clone(),
node_ipc_fd: shared.node_ipc,
disable_deprecated_api_warning: shared.disable_deprecated_api_warning,
verbose_deprecated_api_warning: shared.verbose_deprecated_api_warning,
Expand Down Expand Up @@ -816,6 +818,7 @@ fn create_web_worker_callback(
inspect: shared.options.is_inspecting,
has_node_modules_dir: shared.options.has_node_modules_dir,
argv0: shared.options.argv0.clone(),
node_debug: shared.options.node_debug.clone(),
node_ipc_fd: None,
disable_deprecated_api_warning: shared.disable_deprecated_api_warning,
verbose_deprecated_api_warning: shared.verbose_deprecated_api_warning,
Expand Down
20 changes: 11 additions & 9 deletions ext/node/polyfills/02_init.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,16 @@ import "node:module";

let initialized = false;

function initialize(
usesLocalNodeModulesDir,
argv0,
runningOnMainThread,
workerId,
maybeWorkerMetadata,
warmup = false,
) {
function initialize(args) {
const {
usesLocalNodeModulesDir,
argv0,
runningOnMainThread,
workerId,
maybeWorkerMetadata,
nodeDebug,
warmup = false,
} = args;
if (!warmup) {
if (initialized) {
throw Error("Node runtime already initialized");
Expand All @@ -33,7 +35,7 @@ function initialize(
argv0,
Deno.args,
Deno.version,
Deno.env.get("NODE_DEBUG") ?? "",
nodeDebug ?? "",
);
internals.__initWorkerThreads(
runningOnMainThread,
Expand Down
40 changes: 24 additions & 16 deletions runtime/js/99_main.js
Original file line number Diff line number Diff line change
Expand Up @@ -706,12 +706,13 @@ function bootstrapMainRuntime(runtimeOptions, warmup = false) {
3: inspectFlag,
5: hasNodeModulesDir,
6: argv0,
7: shouldDisableDeprecatedApiWarning,
8: shouldUseVerboseDeprecatedApiWarning,
9: future,
10: mode,
11: servePort,
12: serveHost,
7: nodeDebug,
8: shouldDisableDeprecatedApiWarning,
9: shouldUseVerboseDeprecatedApiWarning,
10: future,
11: mode,
12: servePort,
13: serveHost,
} = runtimeOptions;

if (mode === executionModes.run || mode === executionModes.serve) {
Expand Down Expand Up @@ -859,7 +860,12 @@ function bootstrapMainRuntime(runtimeOptions, warmup = false) {
ObjectDefineProperty(globalThis, "Deno", core.propReadOnly(finalDenoNs));

if (nodeBootstrap) {
nodeBootstrap(hasNodeModulesDir, argv0, /* runningOnMainThread */ true);
nodeBootstrap({
usesLocalNodeModulesDir: hasNodeModulesDir,
runningOnMainThread: true,
argv0,
nodeDebug,
});
}
if (future) {
delete globalThis.window;
Expand Down Expand Up @@ -917,9 +923,10 @@ function bootstrapWorkerRuntime(
4: enableTestingFeaturesFlag,
5: hasNodeModulesDir,
6: argv0,
7: shouldDisableDeprecatedApiWarning,
8: shouldUseVerboseDeprecatedApiWarning,
9: future,
7: nodeDebug,
8: shouldDisableDeprecatedApiWarning,
9: shouldUseVerboseDeprecatedApiWarning,
10: future,
} = runtimeOptions;

// TODO(iuioiua): remove in Deno v2. This allows us to dynamically delete
Expand Down Expand Up @@ -1016,13 +1023,14 @@ function bootstrapWorkerRuntime(
: undefined;

if (nodeBootstrap) {
nodeBootstrap(
hasNodeModulesDir,
nodeBootstrap({
usesLocalNodeModulesDir: hasNodeModulesDir,
runningOnMainThread: false,
argv0,
/* runningOnMainThread */ false,
workerId,
workerMetadata,
);
maybeWorkerMetadata: workerMetadata,
nodeDebug,
});
}

if (future) {
Expand Down Expand Up @@ -1097,4 +1105,4 @@ bootstrapWorkerRuntime(
undefined,
true,
);
nodeBootstrap(undefined, undefined, undefined, undefined, undefined, true);
nodeBootstrap({ warmup: true });
5 changes: 5 additions & 0 deletions runtime/worker_bootstrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ pub struct BootstrapOptions {
pub inspect: bool,
pub has_node_modules_dir: bool,
pub argv0: Option<String>,
pub node_debug: Option<String>,
pub node_ipc_fd: Option<i64>,
pub disable_deprecated_api_warning: bool,
pub verbose_deprecated_api_warning: bool,
Expand Down Expand Up @@ -120,6 +121,7 @@ impl Default for BootstrapOptions {
args: Default::default(),
has_node_modules_dir: Default::default(),
argv0: None,
node_debug: None,
node_ipc_fd: None,
disable_deprecated_api_warning: false,
verbose_deprecated_api_warning: false,
Expand Down Expand Up @@ -156,6 +158,8 @@ struct BootstrapV8<'a>(
bool,
// argv0
Option<&'a str>,
// node_debug
Option<&'a str>,
// disable_deprecated_api_warning,
bool,
// verbose_deprecated_api_warning
Expand Down Expand Up @@ -187,6 +191,7 @@ impl BootstrapOptions {
self.enable_testing_features,
self.has_node_modules_dir,
self.argv0.as_deref(),
self.node_debug.as_deref(),
self.disable_deprecated_api_warning,
self.verbose_deprecated_api_warning,
self.future,
Expand Down