Skip to content

Commit fb16718

Browse files
divybotlittledivy
andauthored
fix(ext/node): report directory imports with node error code (#34076)
Closes denoland/orchid#87 Maps local directory imports in the file fetcher to a Node-compatible TypeError with code ERR_UNSUPPORTED_DIR_IMPORT instead of surfacing raw IsADirectory/EISDIR OS errors. The graph loader now propagates that error through module loading. Tests: - cargo fmt --check - cargo check -p deno_cache_dir -p deno_resolver - cargo test -p deno_cache_dir test_file_fetcher_redirects -- --nocapture - ./x test-spec node/esm_dir_import/local_relative Note: ./x test-compat --list could not run in this checkout because the Node test suite directory is missing. --------- Co-authored-by: divybot <divybot@users.noreply.github.com> Co-authored-by: Divy Srivastava <me@littledivy.com>
1 parent 3090d6f commit fb16718

8 files changed

Lines changed: 96 additions & 8 deletions

File tree

cli/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ use util::fs::canonicalize_path;
6161

6262
const MODULE_NOT_FOUND: &str = "Module not found";
6363
const UNSUPPORTED_SCHEME: &str = "Unsupported scheme";
64+
const UNSUPPORTED_DIR_IMPORT: &str =
65+
"[ERR_UNSUPPORTED_DIR_IMPORT] Directory import";
6466

6567
use self::util::draw_thread::DrawThread;
6668
use self::util::env::resolve_cwd;
@@ -533,6 +535,7 @@ async fn run_subcommand(
533535
fn should_fallback_on_run_error(script_err: &str) -> bool {
534536
if script_err.starts_with(MODULE_NOT_FOUND)
535537
|| script_err.starts_with(UNSUPPORTED_SCHEME)
538+
|| script_err.starts_with(UNSUPPORTED_DIR_IMPORT)
536539
{
537540
return true;
538541
}

libs/cache_dir/file_fetcher/mod.rs

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ use http::header::IF_NONE_MATCH;
2121
use http::header::LOCATION;
2222
use log::debug;
2323
use sys_traits::FsFileMetadata;
24+
use sys_traits::FsMetadata;
2425
use sys_traits::FsMetadataValue;
2526
use sys_traits::FsOpen;
2627
use sys_traits::OpenOptions;
@@ -251,6 +252,24 @@ pub struct FailedReadingLocalFileError {
251252
pub source: std::io::Error,
252253
}
253254

255+
#[derive(Debug, Error, JsError)]
256+
#[class(type)]
257+
#[error(
258+
"[{}] Directory import '{}' is not supported resolving ES modules",
259+
self.code(),
260+
url
261+
)]
262+
#[property("code" = self.code())]
263+
pub struct UnsupportedDirImportError {
264+
pub url: Url,
265+
}
266+
267+
impl UnsupportedDirImportError {
268+
pub fn code(&self) -> &'static str {
269+
"ERR_UNSUPPORTED_DIR_IMPORT"
270+
}
271+
}
272+
254273
#[derive(Debug, Error, JsError)]
255274
#[class("Http")]
256275
#[error("Fetch '{0}' failed, too many redirects.")]
@@ -297,6 +316,9 @@ pub enum FetchNoFollowErrorKind {
297316
#[class(inherit)]
298317
#[error(transparent)]
299318
ReadingFile(#[from] FailedReadingLocalFileError),
319+
#[class(inherit)]
320+
#[error(transparent)]
321+
UnsupportedDirImport(#[from] UnsupportedDirImportError),
300322
#[class(generic)]
301323
#[error("Import '{url}' failed.")]
302324
FetchingRemote {
@@ -386,13 +408,17 @@ pub enum FetchLocalErrorKind {
386408
#[class(inherit)]
387409
#[error(transparent)]
388410
ReadingFile(#[from] FailedReadingLocalFileError),
411+
#[class(inherit)]
412+
#[error(transparent)]
413+
UnsupportedDirImport(#[from] UnsupportedDirImportError),
389414
}
390415

391416
impl From<FetchLocalError> for FetchNoFollowError {
392417
fn from(err: FetchLocalError) -> Self {
393418
match err.into_kind() {
394419
FetchLocalErrorKind::UrlToFilePath(err) => err.into(),
395420
FetchLocalErrorKind::ReadingFile(err) => err.into(),
421+
FetchLocalErrorKind::UnsupportedDirImport(err) => err.into(),
396422
}
397423
}
398424
}
@@ -491,7 +517,7 @@ pub struct FileFetcherOptions {
491517
}
492518

493519
#[sys_traits::auto_impl]
494-
pub trait FileFetcherSys: FsOpen + SystemTimeNow {}
520+
pub trait FileFetcherSys: FsOpen + FsMetadata + SystemTimeNow {}
495521

496522
/// A structure for resolving, fetching and caching source files.
497523
#[derive(Debug)]
@@ -978,11 +1004,36 @@ impl<TBlobStore: BlobStore, TSys: FileFetcherSys, THttpClient: HttpClient>
9781004
options: &FetchLocalOptions,
9791005
) -> Result<Option<File>, FetchLocalError> {
9801006
let local = url_to_file_path(url)?;
1007+
let metadata = self.sys.fs_metadata(&local).ok();
1008+
if metadata
1009+
.as_ref()
1010+
.is_some_and(|metadata| metadata.file_type().is_dir())
1011+
{
1012+
return Err(
1013+
FetchLocalErrorKind::UnsupportedDirImport(UnsupportedDirImportError {
1014+
url: url.clone(),
1015+
})
1016+
.into_box(),
1017+
);
1018+
}
9811019
let Some(file) = self.handle_open_file(url, &local)? else {
9821020
return Ok(None);
9831021
};
984-
match self.fetch_local_inner(file, url, &local, options) {
1022+
let mtime = if options.include_mtime {
1023+
metadata
1024+
.and_then(|metadata| metadata.modified().ok())
1025+
.or_else(|| file.fs_file_metadata().and_then(|m| m.modified()).ok())
1026+
} else {
1027+
None
1028+
};
1029+
match self.fetch_local_inner(file, url, &local, mtime) {
9851030
Ok(file) => Ok(Some(file)),
1031+
Err(err) if err.kind() == std::io::ErrorKind::IsADirectory => Err(
1032+
FetchLocalErrorKind::UnsupportedDirImport(UnsupportedDirImportError {
1033+
url: url.clone(),
1034+
})
1035+
.into_box(),
1036+
),
9861037
Err(err) => Err(
9871038
FetchLocalErrorKind::ReadingFile(FailedReadingLocalFileError {
9881039
url: url.clone(),
@@ -1001,6 +1052,12 @@ impl<TBlobStore: BlobStore, TSys: FileFetcherSys, THttpClient: HttpClient>
10011052
match self.sys.fs_open(path, &OpenOptions::new_read()) {
10021053
Ok(file) => Ok(Some(file)),
10031054
Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(None),
1055+
Err(err) if err.kind() == std::io::ErrorKind::IsADirectory => Err(
1056+
FetchLocalErrorKind::UnsupportedDirImport(UnsupportedDirImportError {
1057+
url: url.clone(),
1058+
})
1059+
.into_box(),
1060+
),
10041061
Err(err) => Err(
10051062
FetchLocalErrorKind::ReadingFile(FailedReadingLocalFileError {
10061063
url: url.clone(),
@@ -1016,13 +1073,8 @@ impl<TBlobStore: BlobStore, TSys: FileFetcherSys, THttpClient: HttpClient>
10161073
mut file: TSys::File,
10171074
url: &Url,
10181075
path: &Path,
1019-
options: &FetchLocalOptions,
1076+
mtime: Option<SystemTime>,
10201077
) -> std::io::Result<File> {
1021-
let mtime = if options.include_mtime {
1022-
file.fs_file_metadata().and_then(|m| m.modified()).ok()
1023-
} else {
1024-
None
1025-
};
10261078
let mut bytes = Vec::new();
10271079
file.read_to_end(&mut bytes)?;
10281080
// If it doesnt have a extension, we want to treat it as typescript by default

libs/cache_dir/tests/file_fetcher_test.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,23 @@ async fn test_file_fetcher_redirects() {
8181
FileOrRedirect::Redirect(_) => unreachable!(),
8282
}
8383
}
84+
85+
sys.fs_create_dir_all("/dir").unwrap();
86+
let dir_url = Url::parse("file:///dir").unwrap();
87+
let result = file_fetcher
88+
.fetch_no_follow(&dir_url, FetchNoFollowOptions::default())
89+
.await;
90+
let err = result.unwrap_err();
91+
assert_eq!(
92+
err.to_string(),
93+
"[ERR_UNSUPPORTED_DIR_IMPORT] Directory import 'file:///dir' is not supported resolving ES modules"
94+
);
95+
match err.into_kind() {
96+
FetchNoFollowErrorKind::UnsupportedDirImport(err) => {
97+
assert_eq!(err.url, dir_url);
98+
}
99+
err => unreachable!("{err:?}"),
100+
}
84101
}
85102

86103
#[tokio::test]

libs/resolver/file_fetcher.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,7 @@ impl<
573573
UrlToFilePath { .. }
574574
| ReadingBlobUrl { .. }
575575
| ReadingFile { .. }
576+
| UnsupportedDirImport { .. }
576577
| FetchingRemote { .. }
577578
| ClientError { .. }
578579
| NoRemote { .. }
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"args": "run main.mjs",
3+
"output": "main.out"
4+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
data
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
try {
2+
await import("./dir");
3+
} catch (err) {
4+
console.log(err instanceof TypeError);
5+
console.log(String(err).includes("ERR_UNSUPPORTED_DIR_IMPORT"));
6+
console.log(String(err).includes("os error"));
7+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
true
2+
true
3+
false

0 commit comments

Comments
 (0)