fix(modules): settle failed dynamic import fetches

moli-renderer-v8/src/module_runtime/graph.rs

@@ -832,23 +832,8 @@ impl NativeModuleGraphJob {
         request: &NativeModuleGraphFetchRequest,
         source: std::result::Result<ModuleGraphFetchedSource, ModuleLoadError>,
     ) -> std::result::Result<NativeModuleGraphJobAdvance, ModuleLoadError> {
-        let client = request.tree_client.ok_or_else(|| {
-            ModuleLoadError::new(
-                ModuleLoadStage::Fetch,
-                "module graph fetch request was missing its module tree client token",
-            )
-        })?;
-        if let Err(error) = &source {
-            let key = request.pending_fetch_key().cloned().ok_or_else(|| {
-                ModuleLoadError::new(
-                    ModuleLoadStage::Fetch,
-                    "failed module graph fetch request was missing its module map key",
-                )
-            })?;
-            vm.document_runtime
-                .mark_native_module_failed(key, error.clone());
-        }
-        self.finish_pending_chromium_tree_fetch_for_client(vm, client, request, source)
+        let mut owner = NativeModuleTreeDocumentOwner::new(vm);
+        self.finish_pending_chromium_tree_fetch_for_request_with_owner(&mut owner, request, source)
     }
 
     fn finish_pending_chromium_tree_fetch_for_request_with_owner<O>(
@@ -866,28 +851,22 @@ impl NativeModuleGraphJob {
                 "module graph fetch request was missing its module tree client token",
             )
         })?;
+        if let Err(error) = &source {
+            let key = request.pending_fetch_key().cloned().ok_or_else(|| {
+                ModuleLoadError::new(
+                    ModuleLoadStage::Fetch,
+                    "failed module graph fetch request was missing its module map key",
+                )
+            })?;
+            // Settle the shared fetch before failing this graph, so joined
+            // clients and later imports observe the cached fetch failure.
+            owner.mark_module_failed(key, error.clone());
+        }
         self.finish_pending_chromium_tree_fetch_for_client_with_owner(
             owner, client, request, source,
         )
     }
 
-    fn finish_pending_chromium_tree_fetch_for_client(
-        &mut self,
-        vm: &mut ScriptVm,
-        client: module_tree::SingleModuleClientToken,
-        request: &NativeModuleGraphFetchRequest,
-        source: std::result::Result<ModuleGraphFetchedSource, ModuleLoadError>,
-    ) -> std::result::Result<NativeModuleGraphJobAdvance, ModuleLoadError> {
-        trace_module_tree_fetch_completed_to_job(client, source.is_ok());
-        let outcome = match source {
-            Ok(source) => module_tree::ModuleFetchOutcome::Fetched(Box::new(
-                chromium_fetched_source_for_request(source, request)?,
-            )),
-            Err(error) => module_tree::ModuleFetchOutcome::Failed(chromium_error(error)),
-        };
-        self.resume_chromium_tree_fetch_outcome(vm, client, outcome)
-    }
-
     fn finish_pending_chromium_tree_fetch_for_client_with_owner<O>(
         &mut self,
         owner: &mut O,
@@ -908,16 +887,6 @@ impl NativeModuleGraphJob {
         self.resume_chromium_tree_fetch_outcome_with_owner(owner, client, outcome)
     }
 
-    fn resume_chromium_tree_fetch_outcome(
-        &mut self,
-        vm: &mut ScriptVm,
-        client: module_tree::SingleModuleClientToken,
-        outcome: module_tree::ModuleFetchOutcome,
-    ) -> std::result::Result<NativeModuleGraphJobAdvance, ModuleLoadError> {
-        let mut owner = NativeModuleTreeDocumentOwner::new(vm);
-        self.resume_chromium_tree_fetch_outcome_with_owner(&mut owner, client, outcome)
-    }
-
     pub(crate) fn resume_chromium_tree_fetch_outcome_with_owner<O>(
         &mut self,
         owner: &mut O,

moli-renderer-v8/src/script_vm/native_module.rs

@@ -5118,8 +5118,8 @@ mod tests {
         DynamicModuleFetchFailure, DynamicModuleFetchOwnerAdvance, DynamicModuleImportOwner,
         ModuleAttributesKey, ModuleEntryId, ModuleFetchMetadata, ModuleGraphFetchedSource,
         ModuleGraphHandle, ModuleImportPhase, ModuleKind, ModuleLoadError, ModuleLoadStage,
-        ModuleMapKey, ModuleSource, NativeModuleGraphFetchRequest, NativeModuleGraphJob,
-        NativeModuleGraphJobAdvance, PendingDynamicModuleImport,
+        ModuleMapEntryState, ModuleMapKey, ModuleSource, NativeModuleGraphFetchRequest,
+        NativeModuleGraphJob, NativeModuleGraphJobAdvance, PendingDynamicModuleImport,
     };
     use crate::module_script_continuation::NativeDynamicModuleTerminalFanout;
     use crate::network::ResourceRequestClient;
@@ -5583,6 +5583,97 @@ mod tests {
         );
     }
 
+    #[test]
+    fn dynamic_import_fetch_failures_reject_joined_and_cached_imports() {
+        for shared_dependency in [false, true] {
+            let mut vm = new_test_vm("https://app.example.test/page.html");
+            let second_specifier = if shared_dependency {
+                "./second.mjs"
+            } else {
+                "./first.mjs"
+            };
+            vm.eval(&format!(
+                r#"
+globalThis.__fetchErrors = [];
+globalThis.__failedImport = specifier => import(specifier).then(
+  () => __fetchErrors.push(null),
+  error => __fetchErrors.push(error)
+);
+__failedImport('./first.mjs');
+__failedImport({second_specifier:?});
+"queued"
+"#
+            ))
+            .expect("both imports should enqueue their graph jobs");
+            for _ in 0..2 {
+                assert!(matches!(
+                    vm.run_next_native_dynamic_module_owner_action_selected_task_body(),
+                    MainNativeModuleSelectedTaskApplication::Applied(_)
+                ));
+            }
+
+            let (failed_load_id, failed_url) = if shared_dependency {
+                for (load_id, root) in [(0, "first"), (1, "second")] {
+                    let target = vm
+                        .current_main_dynamic_import_graph_fetch_target(load_id)
+                        .expect("each root should have its own fetch");
+                    let completion = dynamic_import_completion_with_source(
+                        load_id,
+                        &format!("https://app.example.test/{root}.mjs"),
+                        "import './shared.mjs';",
+                    );
+                    vm.complete_current_main_dynamic_import_graph_fetch_result(
+                        target,
+                        completion.result,
+                    )
+                    .expect("the roots should start or join the shared dependency fetch");
+                }
+                (2, "https://app.example.test/shared.mjs")
+            } else {
+                (0, "https://app.example.test/first.mjs")
+            };
+            assert_eq!(vm.eval("String(__fetchErrors.length)").unwrap(), "0");
+            let target = vm
+                .current_main_dynamic_import_graph_fetch_target(failed_load_id)
+                .expect("the shared request should retain its fetch owner");
+            vm.complete_current_main_dynamic_import_graph_fetch_result(
+                target,
+                Err("HTTP 404".to_owned()),
+            )
+            .expect("fetch failure should reject both graph clients");
+
+            let key = ModuleMapKey::java_script(url::Url::parse(failed_url).unwrap());
+            let entry = vm.document_runtime.native_module_entry_id(&key).unwrap();
+            assert_eq!(
+                vm.document_runtime.native_module_entry_state(entry),
+                ModuleMapEntryState::Failed,
+                "a failed request must settle the shared module map entry"
+            );
+            assert_eq!(
+                vm.eval("JSON.stringify([__fetchErrors.length, __fetchErrors.every(e => e instanceof TypeError), new Set(__fetchErrors).size])")
+                    .unwrap(),
+                "[2,true,2]",
+                "both concurrent imports must reject with distinct TypeErrors"
+            );
+
+            vm.eval("__failedImport('./first.mjs'); 'queued'").unwrap();
+            assert!(matches!(
+                vm.run_next_native_dynamic_module_owner_action_selected_task_body(),
+                MainNativeModuleSelectedTaskApplication::Applied(_)
+            ));
+            // The selected body leaves Promise reactions to its task-end checkpoint.
+            vm.perform_script_task_checkpoint(None)
+                .expect("the cached import task should dispatch its rejection reaction");
+            assert_eq!(
+                vm.eval("JSON.stringify([__fetchErrors.length, __fetchErrors.every(e => e instanceof TypeError), new Set(__fetchErrors).size])")
+                    .unwrap(),
+                "[3,true,3]",
+                "a cached fetch failure must reject a later import with a fresh TypeError"
+            );
+            assert!(!vm.has_inflight_dynamic_module_fetch());
+        }
+    }
+
     #[test]
     fn module_reaction_source_consumes_exactly_one_current_target_per_turn() {
         let mut vm = new_test_vm("https://module-reaction-one-turn.test/page.html");