fix(ext/node): don't rely on Deno.env to read NODE_DEBUG (#23694)

This patch allows implementors to use ext/node without the need to implement Deno.env API. Closes #23687
denoland · May 5, 2024 · b2628e4 · b2628e4
1 parent d527b63
commit b2628e4
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 25 deletions.
diff --git a/cli/factory.rs b/cli/factory.rs
@@ -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

diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs
@@ -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

diff --git a/cli/worker.rs b/cli/worker.rs
@@ -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>>,
@@ -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,
@@ -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,

diff --git a/ext/node/polyfills/02_init.js b/ext/node/polyfills/02_init.js
@@ -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");
@@ -33,7 +35,7 @@ function initialize(
       argv0,
       Deno.args,
       Deno.version,
-      Deno.env.get("NODE_DEBUG") ?? "",
+      nodeDebug ?? "",
     );
     internals.__initWorkerThreads(
       runningOnMainThread,

diff --git a/runtime/js/99_main.js b/runtime/js/99_main.js
@@ -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) {
@@ -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;
@@ -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
@@ -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) {
@@ -1097,4 +1105,4 @@ bootstrapWorkerRuntime(
   undefined,
   true,
 );
-nodeBootstrap(undefined, undefined, undefined, undefined, undefined, true);
+nodeBootstrap({ warmup: true });
diff --git a/runtime/worker_bootstrap.rs b/runtime/worker_bootstrap.rs
@@ -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,
@@ -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,
@@ -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
@@ -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,