From 7d1d6df3fede0aa302806975a50ccc3a94ffcf87 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 15 Aug 2025 10:27:14 -0400 Subject: [PATCH 1/2] feat: add code and specifier to error --- deno | 2 +- src/rs_lib/lib.rs | 76 ++++++++++++++++++- tests/invalid_graph/main.test.ts | 2 +- tests/resolve_error/main.test.ts | 39 ++++++++++ tests/resolve_error/testdata/deno.json | 1 + tests/resolve_error/testdata/main.ts | 0 .../node_modules/export-package/index.js | 1 + .../node_modules/export-package/package.json | 5 ++ .../node_modules/open-package/index.js | 1 + .../node_modules/open-package/package.json | 2 + tests/resolve_error/testdata/package.json | 2 + 11 files changed, 127 insertions(+), 4 deletions(-) create mode 100644 tests/resolve_error/main.test.ts create mode 100644 tests/resolve_error/testdata/deno.json create mode 100644 tests/resolve_error/testdata/main.ts create mode 100644 tests/resolve_error/testdata/node_modules/export-package/index.js create mode 100644 tests/resolve_error/testdata/node_modules/export-package/package.json create mode 100644 tests/resolve_error/testdata/node_modules/open-package/index.js create mode 100644 tests/resolve_error/testdata/node_modules/open-package/package.json create mode 100644 tests/resolve_error/testdata/package.json diff --git a/deno b/deno index 228574b..9b16106 160000 --- a/deno +++ b/deno @@ -1 +1 @@ -Subproject commit 228574b04210ad54baf04126810e2c113a462bcc +Subproject commit 9b16106d286f731e9173df3d3b604fa51978ae7d diff --git a/src/rs_lib/lib.rs b/src/rs_lib/lib.rs index bd666f2..70d7118 100644 --- a/src/rs_lib/lib.rs +++ b/src/rs_lib/lib.rs @@ -2,6 +2,7 @@ mod http_client; use std::borrow::Cow; use std::cell::RefCell; +use std::error::Error; use std::path::Path; use std::path::PathBuf; use std::rc::Rc; @@ -29,6 +30,8 @@ use deno_npm_installer::NpmInstallerFactory; use deno_npm_installer::NpmInstallerFactoryOptions; use deno_npm_installer::Reporter; use deno_npm_installer::lifecycle_scripts::NullLifecycleScriptsExecutor; +use deno_resolver::DenoResolveError; +use deno_resolver::DenoResolveErrorKind; use deno_resolver::cache::ParsedSourceCache; use deno_resolver::cjs::CjsTrackerRc; use deno_resolver::deno_json::CompilerOptionsOverrides; @@ -46,12 +49,16 @@ use deno_resolver::file_fetcher::DenoGraphLoaderOptions; use deno_resolver::file_fetcher::PermissionedFileFetcher; use deno_resolver::file_fetcher::PermissionedFileFetcherOptions; use deno_resolver::graph::DefaultDenoResolverRc; +use deno_resolver::graph::ResolveWithGraphError; +use deno_resolver::graph::ResolveWithGraphErrorKind; use deno_resolver::graph::ResolveWithGraphOptions; use deno_resolver::loader::LoadCodeSourceErrorKind; use deno_resolver::loader::LoadedModuleOrAsset; +use deno_resolver::loader::MemoryFilesRc; use deno_resolver::loader::ModuleLoader; use deno_resolver::loader::RequestedModuleType; use deno_resolver::npm::DenoInNpmPackageChecker; +use deno_resolver::workspace::MappedResolutionError; use deno_semver::SmallStackString; use deno_semver::jsr::JsrPackageReqReference; use deno_semver::npm::NpmPackageReqReference; @@ -65,6 +72,7 @@ use node_resolver::NodeResolverOptions; use node_resolver::PackageJsonThreadLocalCache; use node_resolver::analyze::NodeCodeTranslatorMode; use node_resolver::cache::NodeResolutionThreadLocalCache; +use node_resolver::errors::NodeJsErrorCoded; use serde::Deserialize; use serde::Serialize; use sys_traits::EnvCurrentDir; @@ -340,6 +348,7 @@ impl DenoWorkspace { NullBlobStore, Arc::new(self.workspace_factory.http_cache()?.clone()), self.http_client.clone(), + MemoryFilesRc::default(), self.workspace_factory.sys().clone(), PermissionedFileFetcherOptions { allow_remote: true, @@ -804,6 +813,49 @@ impl DenoLoader { } } +fn resolve_with_graph_error_code( + err: &ResolveWithGraphError, +) -> Option<&'static str> { + match err.as_kind() { + ResolveWithGraphErrorKind::CouldNotResolveNpmNv(err) => { + Some(err.code().as_str()) + } + ResolveWithGraphErrorKind::ResolvePkgFolderFromDenoModule(_) => None, + ResolveWithGraphErrorKind::ResolveNpmReqRef(err) => { + err.err.maybe_code().map(|c| c.as_str()) + } + ResolveWithGraphErrorKind::Resolution(err) => err + .source() + .and_then(|s| s.downcast_ref::()) + .and_then(deno_resolve_error_code), + ResolveWithGraphErrorKind::Resolve(err) => deno_resolve_error_code(err), + ResolveWithGraphErrorKind::PathToUrl(_) => None, + } +} + +fn deno_resolve_error_code(err: &DenoResolveError) -> Option<&'static str> { + match err.as_kind() { + DenoResolveErrorKind::InvalidVendorFolderImport + | DenoResolveErrorKind::UnsupportedPackageJsonFileSpecifier + | DenoResolveErrorKind::UnsupportedPackageJsonJsrReq => None, + DenoResolveErrorKind::MappedResolution(err) => match err { + MappedResolutionError::Specifier(_) + | MappedResolutionError::ImportMap(_) + | MappedResolutionError::Workspace(_) => None, + }, + DenoResolveErrorKind::Node(err) => err.maybe_code().map(|c| c.as_str()), + DenoResolveErrorKind::ResolveNpmReqRef(err) => { + err.err.maybe_code().map(|c| c.as_str()) + } + DenoResolveErrorKind::NodeModulesOutOfDate(_) + | DenoResolveErrorKind::PackageJsonDepValueParse(_) + | DenoResolveErrorKind::PackageJsonDepValueUrlParse(_) + | DenoResolveErrorKind::PathToUrl(_) + | DenoResolveErrorKind::ResolvePkgFolderFromDenoReq(_) + | DenoResolveErrorKind::WorkspaceResolvePkgJsonFolder(_) => None, + } +} + fn create_module_response( url: &Url, media_type: MediaType, @@ -858,7 +910,27 @@ fn resolve_absolute_path( } fn create_js_error(err: anyhow::Error) -> JsValue { - wasm_bindgen::JsError::new(&format!("{:#}", err)).into() + let err_value: JsValue = + wasm_bindgen::JsError::new(&format!("{:#}", err)).into(); + if let Some(err) = err.downcast_ref::() { + if let Some(code) = resolve_with_graph_error_code(err) { + _ = js_sys::Reflect::set( + &err_value, + &JsValue::from_str("code"), + &JsValue::from_str(code), + ); + } + if let Some(specifier) = err.maybe_specifier() { + if let Ok(url) = specifier.into_owned().into_url() { + _ = js_sys::Reflect::set( + &err_value, + &JsValue::from_str("specifier"), + &JsValue::from_str(url.as_str()), + ); + } + } + } + err_value } fn parse_resolution_mode(resolution_mode: u8) -> node_resolver::ResolutionMode { @@ -943,7 +1015,7 @@ struct CapturingModuleAnalyzerRef<'a> { } impl<'a> CapturingModuleAnalyzerRef<'a> { - pub fn as_capturing_parser(&self) -> CapturingEsParser { + pub fn as_capturing_parser(&self) -> CapturingEsParser<'_> { CapturingEsParser::new(Some(self.parser), self.store) } } diff --git a/tests/invalid_graph/main.test.ts b/tests/invalid_graph/main.test.ts index abaa47c..3928001 100644 --- a/tests/invalid_graph/main.test.ts +++ b/tests/invalid_graph/main.test.ts @@ -1,7 +1,7 @@ import { assertEquals } from "@std/assert"; import { createLoaderWithDiagnostics } from "../helpers.ts"; -Deno.test("loads linked entrypoint", async () => { +Deno.test("surfaces graph diagnostic", async () => { const mainFile = import.meta.dirname + "/testdata/main.ts"; const { diagnostics } = await createLoaderWithDiagnostics({ configPath: import.meta.dirname + "/testdata/deno.json", diff --git a/tests/resolve_error/main.test.ts b/tests/resolve_error/main.test.ts new file mode 100644 index 0000000..086c0bd --- /dev/null +++ b/tests/resolve_error/main.test.ts @@ -0,0 +1,39 @@ +import { assertEquals, assertThrows } from "@std/assert"; +import { createLoader, ResolutionMode } from "../helpers.ts"; + +Deno.test("error has extra properties", async (t) => { + const mainFile = import.meta.dirname + "/testdata/main.ts"; + const { loader } = await createLoader({ + configPath: import.meta.dirname + "/testdata/deno.json", + }, { + entrypoints: [mainFile], + }); + + await t.step("code", () => { + const err = assertThrows(() => + loader.resolveSync( + "export-package/non-existent", + import.meta.resolve("./testdata/main.ts"), + ResolutionMode.Import, + ) + ); + assertEquals((err as any).code, "ERR_PACKAGE_PATH_NOT_EXPORTED"); + }); + + await t.step("specifier", () => { + const err = assertThrows(() => + loader.resolveSync( + "open-package/non-existent.js", + import.meta.resolve("./testdata/main.ts"), + ResolutionMode.Import, + ) + ); + assertEquals((err as any).code, "ERR_MODULE_NOT_FOUND"); + assertEquals( + (err as any).specifier, + import.meta.resolve( + "./testdata/node_modules/open-package/non-existent.js", + ), + ); + }); +}); diff --git a/tests/resolve_error/testdata/deno.json b/tests/resolve_error/testdata/deno.json new file mode 100644 index 0000000..0967ef4 --- /dev/null +++ b/tests/resolve_error/testdata/deno.json @@ -0,0 +1 @@ +{} diff --git a/tests/resolve_error/testdata/main.ts b/tests/resolve_error/testdata/main.ts new file mode 100644 index 0000000..e69de29 diff --git a/tests/resolve_error/testdata/node_modules/export-package/index.js b/tests/resolve_error/testdata/node_modules/export-package/index.js new file mode 100644 index 0000000..f053ebf --- /dev/null +++ b/tests/resolve_error/testdata/node_modules/export-package/index.js @@ -0,0 +1 @@ +module.exports = {}; diff --git a/tests/resolve_error/testdata/node_modules/export-package/package.json b/tests/resolve_error/testdata/node_modules/export-package/package.json new file mode 100644 index 0000000..98b4bb9 --- /dev/null +++ b/tests/resolve_error/testdata/node_modules/export-package/package.json @@ -0,0 +1,5 @@ +{ + "exports": { + "./test": "./index.js" + } +} \ No newline at end of file diff --git a/tests/resolve_error/testdata/node_modules/open-package/index.js b/tests/resolve_error/testdata/node_modules/open-package/index.js new file mode 100644 index 0000000..f053ebf --- /dev/null +++ b/tests/resolve_error/testdata/node_modules/open-package/index.js @@ -0,0 +1 @@ +module.exports = {}; diff --git a/tests/resolve_error/testdata/node_modules/open-package/package.json b/tests/resolve_error/testdata/node_modules/open-package/package.json new file mode 100644 index 0000000..7a73a41 --- /dev/null +++ b/tests/resolve_error/testdata/node_modules/open-package/package.json @@ -0,0 +1,2 @@ +{ +} \ No newline at end of file diff --git a/tests/resolve_error/testdata/package.json b/tests/resolve_error/testdata/package.json new file mode 100644 index 0000000..2c63c08 --- /dev/null +++ b/tests/resolve_error/testdata/package.json @@ -0,0 +1,2 @@ +{ +} From 435ad141cb7f9b342eddd837d8150b8cfdc16b11 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 15 Aug 2025 10:35:01 -0400 Subject: [PATCH 2/2] update --- src/mod.ts | 56 ++++++++++++++++++++++++-------- tests/resolve_error/main.test.ts | 29 +++++++++-------- 2 files changed, 58 insertions(+), 27 deletions(-) diff --git a/src/mod.ts b/src/mod.ts index 9b83e34..82c8ed5 100644 --- a/src/mod.ts +++ b/src/mod.ts @@ -73,6 +73,18 @@ export interface WorkspaceOptions { noTranspile?: boolean; } +export class ResolveError extends Error { + /** + * Possible specifier this would resolve to if the error did not occur. + * + * This is useful for implementing something like `import.meta.resolve` where + * you want the resolution to always occur and not error. + */ + specifier?: string; + /** Node.js error code. */ + code?: string; +} + /** File type. */ export enum MediaType { JavaScript = 0, @@ -203,7 +215,9 @@ export class Loader implements Disposable { return messages.map((message) => ({ message })); } - /** Synchronously resolves a specifier using the given referrer and resolution mode. */ + /** Synchronously resolves a specifier using the given referrer and resolution mode. + * @throws {ResolveError} + */ resolveSync( specifier: string, referrer: string | undefined, @@ -216,11 +230,20 @@ export class Loader implements Disposable { }' (${resolutionModeToString(resolutionMode)})`, ); } - const value = this.#inner.resolve_sync(specifier, referrer, resolutionMode); - if (this.#debug) { - console.error(`DEBUG - Resolved to '${value}'`); + try { + const value = this.#inner.resolve_sync( + specifier, + referrer, + resolutionMode, + ); + if (this.#debug) { + console.error(`DEBUG - Resolved to '${value}'`); + } + return value; + } catch (err) { + Object.setPrototypeOf(err, ResolveError.prototype); + throw err; } - return value; } /** Asynchronously resolves a specifier using the given referrer and resolution mode. @@ -230,6 +253,8 @@ export class Loader implements Disposable { * npm or jsr resolution than Deno. For that reason it's better to provide the list of * entrypoints up front so the loader can create the npm and jsr graph, and then after use * synchronous resolution to resolve jsr and npm specifiers. + * + * @throws {ResolveError} */ async resolve( specifier: string, @@ -243,15 +268,20 @@ export class Loader implements Disposable { }' (${resolutionModeToString(resolutionMode)})`, ); } - const value = await this.#inner.resolve( - specifier, - referrer, - resolutionMode, - ); - if (this.#debug) { - console.error(`DEBUG - Resolved to '${value}'`); + try { + const value = await this.#inner.resolve( + specifier, + referrer, + resolutionMode, + ); + if (this.#debug) { + console.error(`DEBUG - Resolved to '${value}'`); + } + return value; + } catch (err) { + Object.setPrototypeOf(err, ResolveError.prototype); + throw err; } - return value; } /** Loads a specifier. */ diff --git a/tests/resolve_error/main.test.ts b/tests/resolve_error/main.test.ts index 086c0bd..fc2dc5b 100644 --- a/tests/resolve_error/main.test.ts +++ b/tests/resolve_error/main.test.ts @@ -1,5 +1,5 @@ -import { assertEquals, assertThrows } from "@std/assert"; -import { createLoader, ResolutionMode } from "../helpers.ts"; +import { assertEquals, assertRejects, assertThrows } from "@std/assert"; +import { createLoader, ResolutionMode, ResolveError } from "../helpers.ts"; Deno.test("error has extra properties", async (t) => { const mainFile = import.meta.dirname + "/testdata/main.ts"; @@ -15,22 +15,23 @@ Deno.test("error has extra properties", async (t) => { "export-package/non-existent", import.meta.resolve("./testdata/main.ts"), ResolutionMode.Import, - ) - ); - assertEquals((err as any).code, "ERR_PACKAGE_PATH_NOT_EXPORTED"); + ), ResolveError); + assertEquals(err.code, "ERR_PACKAGE_PATH_NOT_EXPORTED"); }); - await t.step("specifier", () => { - const err = assertThrows(() => - loader.resolveSync( - "open-package/non-existent.js", - import.meta.resolve("./testdata/main.ts"), - ResolutionMode.Import, - ) + await t.step("specifier", async () => { + const err = await assertRejects( + () => + loader.resolve( + "open-package/non-existent.js", + import.meta.resolve("./testdata/main.ts"), + ResolutionMode.Import, + ), + ResolveError, ); - assertEquals((err as any).code, "ERR_MODULE_NOT_FOUND"); + assertEquals(err.code, "ERR_MODULE_NOT_FOUND"); assertEquals( - (err as any).specifier, + err.specifier, import.meta.resolve( "./testdata/node_modules/open-package/non-existent.js", ),