fix(modules): preserve graph error constructors

This commit is contained in:
ldm0
2026-09-27 07:56:05 +08:00
parent 8fcb49a30f
commit 645c8f6979
3 changed files with 130 additions and 38 deletions
@@ -1320,6 +1320,7 @@ impl<O: NativeModuleTreeDocumentOwnerAdapter> module_tree::ModuleScriptTreeHost
"failed to resolve module specifier `{specifier}` from `{base_url}`: {error}"
),
)
.with_error_constructor(module_tree::ModuleErrorConstructorKind::TypeError)
})?;
let attributes = local_attributes(attributes);
let local_key = ModuleMapKey::from_url_and_attributes(&resolved_url, &attributes).map_err(
@@ -2589,6 +2590,7 @@ where
request.base_url()
),
)
.with_error_constructor(ScriptErrorConstructorKind::TypeError)
})?,
};
let integrity = owner.resolve_module_integrity(&source_url);
@@ -3446,6 +3448,44 @@ import "./c.mjs";
);
}
#[test]
fn static_bare_specifier_resolve_failure_is_a_type_error() {
let mut vm = new_test_vm("https://app.example.test/page");
let root_url = url("https://app.example.test/root.mjs");
let job = parser_owned_external_module_script_graph_job(
&mut vm,
&root_url,
&url("https://app.example.test/page"),
&ScriptFetchMetadata::default(),
);
let root_fetch = expect_single_fetch(
advance_module_script_graph(&mut vm, job)
.expect("external parser graph should request root"),
"bare-specifier root",
);
let error = match root_fetch.finish_source_for_test(
&mut vm,
Ok(ModuleSource::text(
r#"import "unmapped-bare-specifier";"#.to_owned(),
)),
) {
Ok(_) => panic!("an unmapped bare specifier should fail during graph resolution"),
Err(error) => error,
};
assert_eq!(error.stage(), ModuleLoadStage::Resolve);
assert_eq!(
error.error_constructor(),
Some(ScriptErrorConstructorKind::TypeError)
);
assert!(
error.message().contains("unmapped-bare-specifier"),
"{}",
error.message()
);
}
#[test]
fn static_import_with_invalid_attribute_key_fails_before_dependency_fetch() {
let mut vm = new_test_vm("https://app.example.test/page");
@@ -3551,6 +3591,10 @@ import "./c.mjs";
assert!(!job.has_chromium_tree_for_test());
assert_eq!(error.stage(), ModuleLoadStage::Resolve);
assert_eq!(
error.error_constructor(),
Some(ScriptErrorConstructorKind::TypeError)
);
assert!(
error.message().contains(
"module type `text` is not a valid module type for module `https://example.test/app/dep.txt`"
+68 -19
View File
@@ -3564,6 +3564,7 @@ impl ScriptVm {
format!("native root module entry {root_entry:?} is not compiled"),
)
})?;
let mut caught_error_constructor = None;
self.renderer_document_isolate
.with_entered_renderer_document_isolate(|isolate| {
let scope = pin!(v8::HandleScope::new(isolate));
@@ -3571,7 +3572,7 @@ impl ScriptVm {
let context = unsafe { v8::Local::new(scope, &*context_ptr) };
let scope = &mut v8::ContextScope::new(scope, context);
let try_catch = pin!(v8::TryCatch::new(scope));
let scope = try_catch.init();
let mut scope = try_catch.init();
let root_module = v8::Local::new(&scope, &root_module);
let _resolver_scope = ResolverScopeGuard::new(document_modulator);
@@ -3583,11 +3584,17 @@ impl ScriptVm {
Some(true) => Ok(()),
Some(false) => Err(anyhow::anyhow!("v8 reported module instantiate failure")),
None => {
let exception = scope
.exception()
.and_then(|exception| exception.to_string(&scope))
.map(|message| message.to_rust_string_lossy(&scope))
.unwrap_or_else(|| "unknown instantiate exception".to_owned());
let exception = match scope.exception() {
Some(exception) => {
caught_error_constructor =
script_error_constructor_kind_from_value(&mut scope, exception);
exception
.to_string(&scope)
.map(|message| message.to_rust_string_lossy(&scope))
.unwrap_or_else(|| "unknown instantiate exception".to_owned())
}
None => "unknown instantiate exception".to_owned(),
};
Err(anyhow::anyhow!(
"{}",
canonical_native_module_instantiate_error(&exception, &graph_urls)
@@ -3596,19 +3603,11 @@ impl ScriptVm {
}
})
.map_err(|error| {
let message = error.to_string();
let load_error =
ModuleLoadError::new(ModuleLoadStage::Instantiate, message.clone());
if message.contains("does not provide an export named")
|| message.contains("does not export")
{
load_error.with_error_constructor(ScriptErrorConstructorKind::SyntaxError)
} else if has_wasm_entry {
load_error
.with_error_constructor(ScriptErrorConstructorKind::WebAssemblyLinkError)
} else {
load_error
}
native_module_instantiate_load_error(
error.to_string(),
caught_error_constructor,
has_wasm_entry,
)
})?;
self.document_runtime
.mark_native_module_instantiated(root_entry);
@@ -4811,6 +4810,20 @@ fn canonical_native_module_instantiate_error(exception: &str, graph_urls: &[Url]
format!("v8 failed to instantiate native module graph: {exception}")
}
fn native_module_instantiate_load_error(
message: String,
caught_error_constructor: Option<ScriptErrorConstructorKind>,
has_wasm_entry: bool,
) -> ModuleLoadError {
let fallback_constructor = if has_wasm_entry {
ScriptErrorConstructorKind::WebAssemblyLinkError
} else {
ScriptErrorConstructorKind::SyntaxError
};
ModuleLoadError::new(ModuleLoadStage::Instantiate, message)
.with_error_constructor(caught_error_constructor.unwrap_or(fallback_constructor))
}
fn canonical_missing_export_link_error(exception: &str, graph_urls: &[Url]) -> Option<String> {
let module = quoted_value_after(exception, "The requested module ")?;
let export = quoted_value_after(exception, "does not provide an export named ")?;
@@ -5084,6 +5097,7 @@ mod tests {
use crate::script_vm::{ScriptVm, ScriptVmDefaultWorldBootstrap, StandaloneScriptVmHarness};
use crate::types::{
ModuleGraphFetchCompletion, ModuleGraphFetchOrdering, ModuleGraphFetchRequester,
ScriptErrorConstructorKind,
};
use crate::util::v8str;
use moli_fetch::FetchConfig;
@@ -5395,6 +5409,41 @@ mod tests {
);
}
#[test]
fn native_module_instantiate_errors_use_stage_appropriate_constructors() {
let javascript_error = super::native_module_instantiate_load_error(
"javascript link failed".to_owned(),
None,
false,
);
assert_eq!(javascript_error.stage(), ModuleLoadStage::Instantiate);
assert_eq!(
javascript_error.error_constructor(),
Some(ScriptErrorConstructorKind::SyntaxError)
);
let wasm_error = super::native_module_instantiate_load_error(
"WebAssembly link failed".to_owned(),
None,
true,
);
assert_eq!(
wasm_error.error_constructor(),
Some(ScriptErrorConstructorKind::WebAssemblyLinkError)
);
let caught_syntax_error = super::native_module_instantiate_load_error(
"mixed graph failed".to_owned(),
Some(ScriptErrorConstructorKind::SyntaxError),
true,
);
assert_eq!(
caught_syntax_error.error_constructor(),
Some(ScriptErrorConstructorKind::SyntaxError),
"an exact V8 exception constructor must win over the module-kind fallback"
);
}
#[test]
fn dynamic_import_fetch_completion_requires_owner_facade() {
let mut vm = new_test_vm("https://app.example.test/page.html");
@@ -863,6 +863,7 @@ impl ScriptVm {
format!("native root module entry {root_entry:?} is not compiled"),
)
})?;
let mut caught_error_constructor = None;
self.renderer_document_isolate
.with_entered_renderer_document_isolate(|isolate| {
let scope = pin!(v8::HandleScope::new(isolate));
@@ -870,7 +871,7 @@ impl ScriptVm {
let context = unsafe { v8::Local::new(scope, &*context_ptr) };
let scope = &mut v8::ContextScope::new(scope, context);
let try_catch = pin!(v8::TryCatch::new(scope));
let scope = try_catch.init();
let mut scope = try_catch.init();
let root_module = v8::Local::new(&scope, &root_module);
let _resolver_scope = ResolverScopeGuard::new(document_modulator_ptr);
@@ -882,11 +883,17 @@ impl ScriptVm {
Some(true) => Ok(()),
Some(false) => Err(anyhow::anyhow!("v8 reported module instantiate failure")),
None => {
let exception = scope
.exception()
.and_then(|exception| exception.to_string(&scope))
.map(|message| message.to_rust_string_lossy(&scope))
.unwrap_or_else(|| "unknown instantiate exception".to_owned());
let exception = match scope.exception() {
Some(exception) => {
caught_error_constructor =
script_error_constructor_kind_from_value(&mut scope, exception);
exception
.to_string(&scope)
.map(|message| message.to_rust_string_lossy(&scope))
.unwrap_or_else(|| "unknown instantiate exception".to_owned())
}
None => "unknown instantiate exception".to_owned(),
};
Err(anyhow::anyhow!(
"{}",
canonical_native_module_instantiate_error(&exception, &graph_urls)
@@ -895,19 +902,11 @@ impl ScriptVm {
}
})
.map_err(|error| {
let message = error.to_string();
let load_error =
ModuleLoadError::new(ModuleLoadStage::Instantiate, message.clone());
if message.contains("does not provide an export named")
|| message.contains("does not export")
{
load_error.with_error_constructor(ScriptErrorConstructorKind::SyntaxError)
} else if has_wasm_entry {
load_error
.with_error_constructor(ScriptErrorConstructorKind::WebAssemblyLinkError)
} else {
load_error
}
native_module_instantiate_load_error(
error.to_string(),
caught_error_constructor,
has_wasm_entry,
)
})?;
document_modulator.mark_instantiated(root_entry);
Ok(())