fix: make can specify no_npm when creating a user worker in main worker (#631)

* fix: make can specify `no_npm` when creating a user worker in main worker

* chore: add an integration test

Author: nyannyacha

crates/base/src/runtime/mod.rs

@@ -470,6 +470,7 @@ where
       mut conf,
       service_path,
       no_module_cache,
+      no_npm,
       env_vars,
       maybe_eszip,
       maybe_entrypoint,
@@ -584,6 +585,8 @@ where
           if let Some(module_url) = main_module_url.as_ref() {
             builder.set_entrypoint(Some(module_url.to_file_path().unwrap()));
           }
+          builder.set_no_npm(no_npm);
+
           emitter_factory.set_deno_options(builder.build()?);
 
           let deno_options = emitter_factory.deno_options()?;
@@ -2465,6 +2468,7 @@ mod test {
             maybe_module_code: None,
 
             no_module_cache: false,
+            no_npm: None,
             env_vars: env_vars.unwrap_or_default(),
 
             static_patterns,
@@ -2558,6 +2562,7 @@ mod test {
         WorkerContextInitOpts {
           service_path: PathBuf::from("./test_cases/"),
           no_module_cache: false,
+          no_npm: None,
           env_vars: Default::default(),
           timing: None,
           maybe_eszip: None,
@@ -2626,6 +2631,7 @@ mod test {
         WorkerContextInitOpts {
           service_path: PathBuf::from("./test_cases/"),
           no_module_cache: false,
+          no_npm: None,
           env_vars: Default::default(),
           timing: None,
           maybe_eszip: Some(EszipPayloadKind::VecKind(eszip_code)),
@@ -2714,6 +2720,7 @@ mod test {
         WorkerContextInitOpts {
           service_path,
           no_module_cache: false,
+          no_npm: None,
           env_vars: Default::default(),
           timing: None,
           maybe_eszip: Some(EszipPayloadKind::VecKind(eszip_code)),

crates/base/src/utils/test_utils.rs

@@ -242,6 +242,7 @@ impl TestBedBuilder {
       .init_opts(WorkerContextInitOpts {
         service_path: self.main_service_path,
         no_module_cache: false,
+        no_npm: None,
         env_vars: std::env::vars().collect(),
         timing: None,
         maybe_eszip: None,

crates/base/src/worker/pool.rs

@@ -391,6 +391,7 @@ impl WorkerPool {
           let WorkerContextInitOpts {
             service_path,
             no_module_cache,
+            no_npm,
             env_vars,
             conf,
             maybe_eszip,
@@ -408,6 +409,7 @@ impl WorkerPool {
               WorkerContextInitOpts {
                 service_path,
                 no_module_cache,
+                no_npm,
                 env_vars,
                 timing: None,
                 conf,

crates/base/src/worker/worker_surface_creation.rs

@@ -630,6 +630,7 @@ impl MainWorkerSurfaceBuilder {
     inner.set_init_opts(Some(WorkerContextInitOpts {
       service_path,
       no_module_cache: no_module_cache.unwrap_or(flags.no_module_cache),
+      no_npm: None,
 
       timing: None,
       maybe_eszip,
@@ -783,6 +784,7 @@ impl EventWorkerSurfaceBuilder {
     inner.set_init_opts(Some(WorkerContextInitOpts {
       service_path,
       no_module_cache: no_module_cache.unwrap_or(flags.no_module_cache),
+      no_npm: None,
 
       env_vars: std::env::vars().collect(),
       timing: None,

crates/base/test_cases/main/index.ts

@@ -65,6 +65,13 @@ Deno.serve(async (req: Request) => {
       "x-cpu-time-hard-limit-ms",
       10 * 60 * 1000,
     );
+    const noNpm = parseIntFromHeadersOrDefault(
+        req,
+        "x-no-npm",
+        void 0,
+      ) === 1
+      ? true
+      : null;
     const noModuleCache = false;
     const envVarsObj = Deno.env.toObject();
     const envVars = Object.keys(envVarsObj).map((k) => [k, envVarsObj[k]]);
@@ -87,6 +94,7 @@ Deno.serve(async (req: Request) => {
       cpuTimeSoftLimitMs,
       cpuTimeHardLimitMs,
       noModuleCache,
+      noNpm,
       envVars,
       context,
     });

crates/base/test_cases/npm-import-with-package-json/index.ts

@@ -0,0 +1,3 @@
+import isEven from "npm:is-even";
+console.log(isEven);
+Deno.serve(() => new Response());

crates/base/test_cases/npm-import-with-package-json/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "meow",
+  "workspaces": []
+}

crates/base/tests/eszip_migration.rs

@@ -123,6 +123,7 @@ where
     .init_opts(WorkerContextInitOpts {
       service_path: PathBuf::from("meow"),
       no_module_cache: false,
+      no_npm: None,
       // XXX: This seems insufficient as it may rely on the env contained in
       // Edge Functions' metadata.
       env_vars: std::env::vars().collect(),

crates/base/tests/integration_tests.rs

@@ -29,6 +29,7 @@ use base::utils::test_utils::create_test_user_worker;
 use base::utils::test_utils::ensure_npm_package_installed;
 use base::utils::test_utils::test_user_runtime_opts;
 use base::utils::test_utils::test_user_worker_pool_policy;
+use base::utils::test_utils::TestBed;
 use base::utils::test_utils::TestBedBuilder;
 use base::utils::test_utils::{self};
 use base::worker;
@@ -212,6 +213,7 @@ async fn test_not_trigger_pku_sigsegv_due_to_jit_compilation_non_cli() {
     .init_opts(WorkerContextInitOpts {
       service_path: "./test_cases/slow_resp".into(),
       no_module_cache: false,
+      no_npm: None,
       env_vars: HashMap::new(),
       timing: None,
       maybe_eszip: None,
@@ -371,6 +373,7 @@ async fn test_main_worker_boot_error() {
     .init_opts(WorkerContextInitOpts {
       service_path: "./test_cases/meow".into(),
       no_module_cache: false,
+      no_npm: None,
       env_vars: HashMap::new(),
       timing: None,
       maybe_eszip: None,
@@ -494,6 +497,7 @@ async fn test_main_worker_user_worker_mod_evaluate_exception() {
     .init_opts(WorkerContextInitOpts {
       service_path: "./test_cases/main".into(),
       no_module_cache: false,
+      no_npm: None,
       env_vars: HashMap::new(),
       timing: None,
       maybe_eszip: None,
@@ -877,6 +881,7 @@ async fn test_worker_boot_invalid_imports() {
   let opts = WorkerContextInitOpts {
     service_path: "./test_cases/invalid_imports".into(),
     no_module_cache: false,
+    no_npm: None,
     env_vars: HashMap::new(),
     timing: None,
     maybe_eszip: None,
@@ -905,6 +910,7 @@ async fn test_worker_boot_with_0_byte_eszip() {
   let opts = WorkerContextInitOpts {
     service_path: "./test_cases/meow".into(),
     no_module_cache: false,
+    no_npm: None,
     env_vars: HashMap::new(),
     timing: None,
     maybe_eszip: Some(EszipPayloadKind::VecKind(vec![])),
@@ -932,6 +938,7 @@ async fn test_worker_boot_with_invalid_entrypoint() {
   let opts = WorkerContextInitOpts {
     service_path: "./test_cases/meow".into(),
     no_module_cache: false,
+    no_npm: None,
     env_vars: HashMap::new(),
     timing: None,
     maybe_eszip: None,
@@ -4095,6 +4102,80 @@ async fn test_user_workers_cleanup_idle_workers() {
   unreachable!("test failed");
 }
 
+#[tokio::test]
+#[serial]
+async fn test_no_npm() {
+  async fn send_it(
+    no_npm: bool,
+    tx: mpsc::UnboundedSender<WorkerEventWithMetadata>,
+  ) -> (TestBed, u16) {
+    let tb = TestBedBuilder::new("./test_cases/main")
+      .with_per_worker_policy(None)
+      .with_worker_event_sender(Some(tx))
+      .build()
+      .await;
+
+    let resp = tb
+      .request(|mut b| {
+        b = b.uri("/npm-import-with-package-json");
+        if no_npm {
+          b = b.header("x-no-npm", HeaderValue::from_static("1"))
+        }
+        b.body(Body::empty()).context("can't make request")
+      })
+      .await
+      .unwrap();
+
+    (tb, resp.status().as_u16())
+  }
+
+  {
+    // Since `noNpm` is configured, it will not discover package.json, and
+    // `npm:is-even` will resolve normally using Deno's original module
+    // resolution method.
+    let (tx, mut rx) = mpsc::unbounded_channel();
+    let (tb, status_code) = send_it(true, tx).await;
+
+    assert_eq!(status_code, StatusCode::OK);
+
+    rx.close();
+    tb.exit(Duration::from_secs(TESTBED_DEADLINE_SEC)).await;
+  }
+  {
+    // Note that `noNpm` is not set this time. In this case, it will try to
+    // discover package.json and will eventually find it.
+    //
+    // This causes it to switch to Byonm mode and try to resolve modules from
+    // the adjacent node_modules/.
+    //
+    // However, since node_modules/ does not exist anywhere, the attempt to
+    // resolve `npm:is-even` will fail.
+    let (tx, mut rx) = mpsc::unbounded_channel();
+    let (tb, status_code) = send_it(false, tx).await;
+
+    assert_eq!(status_code, StatusCode::INTERNAL_SERVER_ERROR);
+
+    rx.close();
+    tb.exit(Duration::from_secs(TESTBED_DEADLINE_SEC)).await;
+
+    while let Some(ev) = rx.recv().await {
+      let WorkerEvents::BootFailure(ev) = ev.event else {
+        continue;
+      };
+
+      assert!(ev.msg.starts_with(
+        "worker boot error: failed to bootstrap runtime: failed to create the \
+graph: Could not find a matching package for 'npm:is-even' in the node_modules \
+directory."
+      ));
+
+      return;
+    }
+
+    unreachable!("test failed");
+  }
+}
+
 #[derive(Deserialize)]
 struct ErrorResponsePayload {
   msg: String,

ext/workers/context.rs

@@ -256,6 +256,7 @@ impl Default for Timing {
 pub struct WorkerContextInitOpts {
   pub service_path: PathBuf,
   pub no_module_cache: bool,
+  pub no_npm: Option<bool>,
   pub env_vars: HashMap<String, String>,
   pub conf: WorkerRuntimeOpts,
   pub static_patterns: Vec<String>,