From a673f438732b2b59ea3036a7bd0f7a47b1b34370 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 11 Sep 2026 01:36:11 +0000 Subject: [PATCH 1/7] error printer: never block on, or read from, a file that is not regular The error printer reads files by path when it prints an error: the top frame's file for the code frame, the .map sidecar, and an original source that a source map names. Each was a blocking open(2) with no file type check. When one of those paths named a FIFO, the process stayed in open() forever and never printed the error or exited. With a writer on the FIFO it took the bytes in the pipe and stayed in read(). The .map read is also reached by a bare error.stack. - bun_sys::File gets open_regular_at, ensure_regular and read_regular_from: O_NONBLOCK on unix, then an fstat of the descriptor that fails with EISDIR or ENODEV unless the file is regular. - cache::Fs::read_file_with_allocator takes a NonRegularFile policy. Only the PrintSource fetch passes Reject, so a module load keeps its single open and no fstat. - The two source map reads use read_regular_from. - The printer's fetch no longer looks up the directory's package.json. It never used the result, and the lookup scans a directory that the frame's URL chooses. --- src/bundler/ParseTask.rs | 1 + src/bundler/lib.rs | 1 - src/bundler/options.rs | 20 +-- src/bundler/transpiler.rs | 4 + src/jsc/RuntimeTranspilerStore.rs | 1 + src/jsc/VirtualMachine.rs | 10 +- src/resolver/lib.rs | 45 +++-- src/resolver/package_json.rs | 1 + src/resolver/resolver.rs | 1 + src/runtime/api/JSTranspiler.rs | 2 + src/runtime/jsc_hooks.rs | 34 ++-- src/sourcemap/Mapping.rs | 2 +- src/sourcemap/lib.rs | 6 +- src/sys/file.rs | 27 +++ test/js/bun/util/inspect-error.test.js | 222 ++++++++++++++++++++++--- 15 files changed, 288 insertions(+), 89 deletions(-) diff --git a/src/bundler/ParseTask.rs b/src/bundler/ParseTask.rs index bf59458d4fd6..e6b5d45718d9 100644 --- a/src/bundler/ParseTask.rs +++ b/src/bundler/ParseTask.rs @@ -1522,6 +1522,7 @@ pub mod parse_worker { false, contents_file.unwrap_valid(), read_arena, + bun_resolver::cache::NonRegularFile::Read, ) { Ok(e) => { // `bun_resolver::cache::Entry` ↔ `crate::cache::Entry` diff --git a/src/bundler/lib.rs b/src/bundler/lib.rs index e5415983e9bc..151692da312f 100644 --- a/src/bundler/lib.rs +++ b/src/bundler/lib.rs @@ -378,7 +378,6 @@ bun_dispatch::link_interface! { fn loaders() -> *const bun_collections::StringArrayHashMap; fn eval_source() -> Option<*const bun_ast::Source>; fn main() -> &'static [u8]; - fn read_dir_info_package_json(dir: &[u8]) -> Option<*const bun_resolver::PackageJSON>; fn is_blob_url(specifier: &[u8]) -> bool; fn resolve_blob(specifier: &[u8]) -> Option; fn blob_loader(blob: options::OpaqueBlob) -> Option; diff --git a/src/bundler/options.rs b/src/bundler/options.rs index 2540706e9d45..0f309f51337a 100644 --- a/src/bundler/options.rs +++ b/src/bundler/options.rs @@ -10,7 +10,7 @@ use bun_js_parser::parser::Runtime; use bun_options_types::schema::api; use bun_resolver::fs as Fs; use bun_resolver::fs::PathResolverExt as _; -use bun_resolver::package_json::{MacroMap as MacroRemap, PackageJSON}; +use bun_resolver::package_json::MacroMap as MacroRemap; use enum_map::EnumMap; use std::borrow::Cow; @@ -468,9 +468,6 @@ pub struct LoaderResult<'a> { pub path: Fs::Path<'a>, pub is_main: bool, pub specifier: &'a [u8], - /// NOTE: This is always `null` for non-js-like loaders since it's not - /// needed for them. - pub package_json: Option<&'a PackageJSON>, } pub fn get_loader_and_virtual_source<'a>( @@ -557,27 +554,12 @@ pub fn get_loader_and_virtual_source<'a>( let is_main = strings::eql_long(specifier, jsc_vm.main(), true); - let dir = path.name().dir; - // NOTE: we cannot trust `path.isFile()` since it's not always correct - // NOTE: assume we may need a package.json when no loader is specified - let is_js_like = loader.map(|l| l.is_js_like()).unwrap_or(true); - let package_json: Option<&PackageJSON> = if is_js_like && bun_paths::is_absolute(dir) { - jsc_vm - .read_dir_info_package_json(dir) - // SAFETY: the vtable returns a pointer into the resolver's DirInfo - // cache owned by `jsc_vm.owner`, which outlives `'a`. - .map(|p| unsafe { &*p }) - } else { - None - }; - Ok(LoaderResult { loader, virtual_source, path, is_main, specifier, - package_json, }) } diff --git a/src/bundler/transpiler.rs b/src/bundler/transpiler.rs index 1957363dd686..91af93e078d3 100644 --- a/src/bundler/transpiler.rs +++ b/src/bundler/transpiler.rs @@ -983,6 +983,7 @@ pub struct ParseOptions<'a, 'b> { pub arena: &'a Arena, pub dirname_fd: FD, pub file_descriptor: Option, + pub non_regular_file: resolver::cache::NonRegularFile, /// On exception, we might still want to watch the file. pub file_fd_ptr: Option<&'b mut FD>, @@ -1481,6 +1482,7 @@ impl<'a> Transpiler<'a> { USE_SHARED_BUFFER, file_descriptor, if USE_SHARED_BUFFER { None } else { Some(arena) }, + this_parse.non_regular_file, ) { Ok(e) => e, Err(err) => { @@ -2966,6 +2968,7 @@ impl<'a> Transpiler<'a> { loader, dirname_fd, file_descriptor: None, + non_regular_file: resolver::cache::NonRegularFile::Read, file_fd_ptr: None, macro_remappings, macro_js_ctx: default_macro_js_value(), @@ -3139,6 +3142,7 @@ impl<'a> Transpiler<'a> { false, None, None, + resolver::cache::NonRegularFile::Read, ) { Ok(e) => e, Err(err) => { diff --git a/src/jsc/RuntimeTranspilerStore.rs b/src/jsc/RuntimeTranspilerStore.rs index df08a4a8e78c..6c886c030ece 100644 --- a/src/jsc/RuntimeTranspilerStore.rs +++ b/src/jsc/RuntimeTranspilerStore.rs @@ -792,6 +792,7 @@ impl TranspilerJob { loader, dirname_fd: Fd::INVALID, file_descriptor: None, + non_regular_file: bun_resolver::cache::NonRegularFile::Read, // SAFETY: `input_file_fd` is a stack local declared above and // outlives `parse_options`; `addr_of_mut!` avoids forming an // intermediate `&mut` so the close-guard's later borrow stays sound. diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 6155a2b59163..d352d7962906 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -4425,11 +4425,6 @@ impl VirtualMachine { Ok(lr) => lr, Err(_) => return Err(crate::CrateError::ModuleNotFound), }; - let module_type = lr - .package_json - .map(|pkg| pkg.module_type) - .unwrap_or(bun_bundler::options::ModuleType::Unknown); - // A drop-guard so both the normal and error paths reset the arena on // the right edge. struct ArenaReset<'a>(&'a mut VirtualMachine, bool); @@ -4463,7 +4458,10 @@ impl VirtualMachine { } else { bun_ast::Loader::File }), - module_type, + // The one caller fetches with `PrintSource`, which returns the + // file as it is. A `package.json` lookup for the module type would + // scan a directory the error printer has no other reason to touch. + module_type: bun_bundler::options::ModuleType::Unknown, source_code_printer: printer.as_ptr(), // `fetchWithoutOnLoadPlugins` forbids the async path. promise_ptr: core::ptr::null_mut(), diff --git a/src/resolver/lib.rs b/src/resolver/lib.rs index 883137b80f80..626bf89268b9 100644 --- a/src/resolver/lib.rs +++ b/src/resolver/lib.rs @@ -2088,6 +2088,19 @@ pub mod cache { } } + /// What [`Fs::read_file_with_allocator`] does with a path that is not a + /// regular file. + #[derive(Clone, Copy, PartialEq, Eq)] + pub enum NonRegularFile { + /// Open and read it like a file. A FIFO blocks until it has a writer. + Read, + /// Fail (`EISDIR` for a directory, `ENODEV` otherwise) without + /// blocking on it or reading a byte from it. For a read that happens + /// after the fact, such as the error printer's re-read of a module + /// that has already run. + Reject, + } + /// File-read cache: shared read buffers plus flags controlling buffer /// reuse and streaming reads. pub struct Fs { @@ -2329,27 +2342,31 @@ pub mod cache { use_shared_buffer: bool, _file_handle: Option, arena: Option<&bun_alloc::Arena>, + non_regular_file: NonRegularFile, ) -> crate::CrateResult { let rfs = &_fs.fs; let will_close = rfs.need_to_close_files() && _file_handle.is_none(); + let open_at = |dir: Fd, path: &[u8], flags: i32| match non_regular_file { + NonRegularFile::Read => bun_sys::File::openat(dir, path, flags, 0), + NonRegularFile::Reject => { + bun_sys::File::open_regular_at(dir, path).map(|(file, _size)| file) + } + }; + let open_path = + |path: &[u8]| open_at(Fd::cwd(), path, bun_sys::O::RDONLY | bun_sys::O::CLOEXEC); + // A single let-expression avoids `mem::zeroed()` on a // type that may have niche (NonZero) fields. let file_handle: bun_sys::File = if let Some(f) = _file_handle { bun_sys::lseek(f, 0, libc::SEEK_SET).map_err(crate::Error::from)?; bun_sys::File::from_fd(f) } else if feature_flags::STORE_FILE_DESCRIPTORS && dirname_fd.is_valid() { - match bun_sys::openat_a( - dirname_fd, - bun_paths::basename(path), - bun_sys::O::RDONLY, - 0, - ) { - Ok(fd) => bun_sys::File::from_fd(fd), + match open_at(dirname_fd, bun_paths::basename(path), bun_sys::O::RDONLY) { + Ok(file) => file, Err(err) if err.get_errno() == bun_sys::E::ENOENT => { - let handle = bun_sys::open_file(path, bun_sys::OpenFlags::READ_ONLY) - .map_err(crate::Error::from)?; + let handle = open_path(path).map_err(crate::Error::from)?; bun_core::pretty_errorln!( "Internal error: directory mismatch for directory \"{}\", fd {}. You don't need to do anything, but this indicates a bug.", bstr::BStr::new(path), @@ -2360,8 +2377,7 @@ pub mod cache { Err(err) => return Err(err.into()), } } else { - bun_sys::open_file(path, bun_sys::OpenFlags::READ_ONLY) - .map_err(crate::Error::from)? + open_path(path).map_err(crate::Error::from)? }; let mut owned: Option = None; @@ -2374,6 +2390,13 @@ pub mod cache { }; let file_handle = bun_sys::File::borrow(&fd); + // A caller's handle did not go through `open_regular_at`. + if _file_handle.is_some() && non_regular_file == NonRegularFile::Reject { + file_handle + .ensure_regular(path) + .map_err(crate::Error::from)?; + } + #[cfg(not(windows))] // skip on Windows because NTCreateFile will do it. bun_core::scoped_log!( CacheFs, diff --git a/src/resolver/package_json.rs b/src/resolver/package_json.rs index a17e1cd9e20e..91949bdb8d4d 100644 --- a/src/resolver/package_json.rs +++ b/src/resolver/package_json.rs @@ -399,6 +399,7 @@ impl PackageJSON { false, None, None, + crate::cache::NonRegularFile::Read, ) { Ok(e) => e, Err(err) => { diff --git a/src/resolver/resolver.rs b/src/resolver/resolver.rs index 31dd498e7972..a698d2db9950 100644 --- a/src/resolver/resolver.rs +++ b/src/resolver/resolver.rs @@ -3988,6 +3988,7 @@ impl<'a> Resolver<'a> { false, None, None, + crate::cache::NonRegularFile::Read, )?; // NOTE: reshaped for borrowck — `mem::take` the contents (leaving // `Contents::Empty` behind) so `entry` stays whole for the close-guard. diff --git a/src/runtime/api/JSTranspiler.rs b/src/runtime/api/JSTranspiler.rs index e5c1d8c2df70..02b7b95aa599 100644 --- a/src/runtime/api/JSTranspiler.rs +++ b/src/runtime/api/JSTranspiler.rs @@ -771,6 +771,7 @@ impl TransformTask { macro_remappings: clone_macro_map(&self.macro_map), dirname_fd: bun_sys::Fd::INVALID, file_descriptor: None, + non_regular_file: bun_resolver::cache::NonRegularFile::Read, loader: self.loader, jsx, path: source.path, @@ -1226,6 +1227,7 @@ impl JSTranspiler { macro_remappings: clone_macro_map(&config.macro_map), dirname_fd: bun_sys::Fd::INVALID, file_descriptor: None, + non_regular_file: bun_resolver::cache::NonRegularFile::Read, loader: loader.unwrap_or(config.default_loader), jsx, path: source.path, diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index 5d1ff53722d5..6eabf9461848 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -1433,7 +1433,6 @@ mod vm_loader_ctx { use super::*; use crate::webcore::Blob; use bun_bundler::options::OpaqueBlob; - use bun_resolver::package_json::PackageJSON; /// Recover an [`OpaqueBlob`] as a shared `&Blob` (live until `blob_deinit`). /// @@ -1447,12 +1446,9 @@ mod vm_loader_ctx { // `this: *mut VirtualMachine`. Bodies use raw place projections — // `(*this).field` — so no `&VirtualMachine` retag is materialized for the - // simple field reads. This matters because `read_dir_info_package_json` - // holds a live `&mut transpiler.resolver` across a re-entrant `read_dir_info` - // that can call back into these hooks; a `&VirtualMachine` formed here would - // alias that `&mut` (SB/TB UB). The two accessors that call `&self` methods + // simple field reads. The two accessors that call `&self` methods // (`main`, `blob_loader`) form a transient `&VirtualMachine` scoped to the - // single call, which never spans the re-entrant path. + // single call. bun_bundler::link_impl_VmLoaderCtx! { Runtime for extern VirtualMachine => |this| { origin_host() => (*this).origin.host, @@ -1464,19 +1460,6 @@ mod vm_loader_ctx { .as_deref() .map(core::ptr::from_ref::), main() => &*core::ptr::from_ref::<[u8]>((*this).main()), - read_dir_info_package_json(dir) => { - // Short-lived `&mut Resolver` (not `&mut VirtualMachine`) for - // the call — narrows the borrow re-entrant JS could alias. - match (*this).transpiler.resolver.read_dir_info(dir) { - Ok(Some(dir_info)) => { - dir_info - .package_json() - .or(dir_info.enclosing_package_json) - .map(core::ptr::from_ref::) - } - _ => None, - } - }, is_blob_url(spec) => crate::webcore::object_url_registry::is_blob_url(spec), resolve_blob(spec) => { crate::webcore::object_url_registry::ObjectURLRegistry::singleton() @@ -2621,6 +2604,13 @@ fn transpile_source_code_inner( loader, dirname_fd: bun_sys::Fd::INVALID, file_descriptor: None, + // The error printer must not block on, or read from, what is + // no longer the regular file the loader read. + non_regular_file: if args.flags == FetchFlags::PrintSource { + bun_resolver::cache::NonRegularFile::Reject + } else { + bun_resolver::cache::NonRegularFile::Read + }, // SAFETY: `input_file_fd_ptr` points at this frame's // `input_file_fd`; reborrow through the raw pointer so the // `_fd_guard` scopeguard's tag is not invalidated by a @@ -2702,11 +2692,11 @@ fn transpile_source_code_inner( hash, package_json, ); + // Node compile cache: record the failed module so exit-time + // persist logs the "was not initialized" skip (Node parity). + note_compile_cache_parse_failure(path, loader, module_type); } arena_guard.2 = false; // give_back_arena = false - // Node compile cache: record the failed module so exit-time - // persist logs the "was not initialized" skip (Node parity). - note_compile_cache_parse_failure(path, loader, module_type); return Err(crate::Error::ParseError); }; diff --git a/src/sourcemap/Mapping.rs b/src/sourcemap/Mapping.rs index e2ba5b4e344a..7e1ca543abdb 100644 --- a/src/sourcemap/Mapping.rs +++ b/src/sourcemap/Mapping.rs @@ -398,7 +398,7 @@ impl Lookup { let normalized = bun_paths::resolve_path::join_abs_string_buf_z::< bun_paths::platform::Loose, >(dir, &mut buf, &[name]); - match bun_sys::File::read_from(bun_sys::Fd::cwd(), normalized) { + match bun_sys::File::read_regular_from(bun_sys::Fd::cwd(), normalized) { Ok(r) => break 'bytes r, Err(_) => return None, } diff --git a/src/sourcemap/lib.rs b/src/sourcemap/lib.rs index a2c098339941..cd0b10bfaa96 100644 --- a/src/sourcemap/lib.rs +++ b/src/sourcemap/lib.rs @@ -527,9 +527,9 @@ pub(crate) fn get_source_map_impl( let load_path = bun_core::ZStr::from_buf(&load_path_buf[..], source_filename.len() + 4); - // `bun_sys::File::read_from` returns an owned `Vec`, - // freed on scope exit by `Vec`'s Drop. - let data = match bun_sys::File::read_from(bun_core::Fd::cwd(), load_path) { + // Reached from `error.stack` and the error printer: a FIFO + // here must not block them. + let data = match bun_sys::File::read_regular_from(bun_core::Fd::cwd(), load_path) { Ok(data) => data, Err(_) => break 'try_external, }; diff --git a/src/sys/file.rs b/src/sys/file.rs index 23baec9e0f42..bdde938759f9 100644 --- a/src/sys/file.rs +++ b/src/sys/file.rs @@ -336,6 +336,33 @@ impl File { } // ── one-shot path helpers (open + io + close) ─────────────────────── + /// Open `path` for reading and return it with its size; `EISDIR` for a directory, `ENODEV` for any other non-regular file. + pub fn open_regular_at(dir: impl AsFd, path: &[u8]) -> Maybe<(Self, u64)> { + let dir = dir.as_fd(); + // On Windows `O_NONBLOCK` would make the handle overlapped; the fstat still rejects there. + #[cfg(unix)] + let flags = O::RDONLY | O::CLOEXEC | O::NONBLOCK; + #[cfg(not(unix))] + let flags = O::RDONLY | O::CLOEXEC; + let file = Self::openat(dir, path, flags, 0)?; + let size = file.ensure_regular(path)?; + Ok((file, size)) + } + /// The check of [`File::open_regular_at`] for a file opened with other flags: the size of a regular file, else its error. + pub fn ensure_regular(&self, path: &[u8]) -> Maybe { + let st = self.stat().map_err(|e| e.with_path(path))?; + let mode = st.st_mode as Mode; + if !S::ISREG(mode) { + let errno = if S::ISDIR(mode) { E::EISDIR } else { E::ENODEV }; + return Err(Error::new(errno, Tag::open).with_path(path)); + } + Ok(st.st_size.max(0) as u64) + } + /// [`File::read_from`] of a regular file only ([`File::open_regular_at`]). + pub fn read_regular_from(dir: impl AsFd, path: &[u8]) -> Maybe> { + let (file, _size) = Self::open_regular_at(dir, path)?; + file.read_to_end() + } /// Open + read + close. Accepts `&[u8]`; `&ZStr` callers deref-coerce. pub fn read_from(dir: impl AsFd, path: &[u8]) -> Maybe> { let dir = dir.as_fd(); diff --git a/test/js/bun/util/inspect-error.test.js b/test/js/bun/util/inspect-error.test.js index dcf0cc2b7617..c8cd075e60f1 100644 --- a/test/js/bun/util/inspect-error.test.js +++ b/test/js/bun/util/inspect-error.test.js @@ -1,5 +1,7 @@ -import { describe, expect, jest, test } from "bun:test"; -import { bunEnv, bunExe, tempDir } from "harness"; +import { afterAll, describe, expect, jest, test } from "bun:test"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; +import { mkfifo } from "mkfifo"; +import { closeSync, constants, mkdirSync, openSync, readSync, symlinkSync, writeSync } from "node:fs"; test("error.cause", () => { const err = new Error("error 1"); @@ -9,24 +11,25 @@ test("error.cause", () => { .replaceAll("\\", "/") .replaceAll(import.meta.dir.replaceAll("\\", "/"), "[dir]"), ).toMatchInlineSnapshot(` -"1 | import { describe, expect, jest, test } from "bun:test"; -2 | import { bunEnv, bunExe, tempDir } from "harness"; -3 | -4 | test("error.cause", () => { -5 | const err = new Error("error 1"); -6 | const err2 = new Error("error 2", { cause: err }); +"3 | import { mkfifo } from "mkfifo"; +4 | import { closeSync, constants, mkdirSync, openSync, readSync, symlinkSync, writeSync } from "node:fs"; +5 | +6 | test("error.cause", () => { +7 | const err = new Error("error 1"); +8 | const err2 = new Error("error 2", { cause: err }); ^ error: error 2 - at ([dir]/inspect-error.test.js:6:20) + at ([dir]/inspect-error.test.js:8:20) -1 | import { describe, expect, jest, test } from "bun:test"; -2 | import { bunEnv, bunExe, tempDir } from "harness"; -3 | -4 | test("error.cause", () => { -5 | const err = new Error("error 1"); +2 | import { bunEnv, bunExe, isWindows, tempDir } from "harness"; +3 | import { mkfifo } from "mkfifo"; +4 | import { closeSync, constants, mkdirSync, openSync, readSync, symlinkSync, writeSync } from "node:fs"; +5 | +6 | test("error.cause", () => { +7 | const err = new Error("error 1"); ^ error: error 1 - at ([dir]/inspect-error.test.js:5:19) + at ([dir]/inspect-error.test.js:7:19) " `); }); @@ -38,15 +41,15 @@ test("Error", () => { .replaceAll("\\", "/") .replaceAll(import.meta.dir.replaceAll("\\", "/"), "[dir]"), ).toMatchInlineSnapshot(` -"30 | " -31 | \`); -32 | }); -33 | -34 | test("Error", () => { -35 | const err = new Error("my message"); +"33 | " +34 | \`); +35 | }); +36 | +37 | test("Error", () => { +38 | const err = new Error("my message"); ^ error: my message - at ([dir]/inspect-error.test.js:35:19) + at ([dir]/inspect-error.test.js:38:19) " `); }); @@ -105,7 +108,7 @@ test("Error inside minified file (no color) ", () => { error: error inside long minified file! at ([dir]/inspect-error-fixture.min.js:26:2850) at ([dir]/inspect-error-fixture.min.js:26:2890) - at ([dir]/inspect-error.test.js:86:7)" + at ([dir]/inspect-error.test.js:89:7)" `); } }); @@ -134,7 +137,7 @@ test("Error inside minified file (color) ", () => { error: error inside long minified file! at ([dir]/inspect-error-fixture.min.js:26:2850) at ([dir]/inspect-error-fixture.min.js:26:2890) - at ([dir]/inspect-error.test.js:114:7)" + at ([dir]/inspect-error.test.js:117:7)" `); } }); @@ -148,7 +151,7 @@ test("Inserted originalLine and originalColumn do not appear in node:util.inspec .replaceAll(import.meta.path.replaceAll("\\", "/"), "[file]"), ).toMatchInlineSnapshot(` "Error: my message - at ([file]:143:19)" + at ([file]:146:19)" `); }); @@ -196,8 +199,17 @@ describe("source map remapping of the printed stack", () => { .map(line => line.replaceAll(prefix, "")); } - async function run(files) { + // A child that never exits fails its test by timeout. It must not outlive + // the run too. + const children = []; + afterAll(() => { + for (const child of children) child.kill("SIGKILL"); + }); + + // `prepare(dir)` runs once the files exist, for what a file tree can't express. + async function run(files, prepare = () => {}) { using dir = tempDir("inspect-error-sourcemap", files); + prepare(String(dir)); await using proc = Bun.spawn({ cmd: [bunExe(), "main.js"], cwd: String(dir), @@ -205,6 +217,7 @@ describe("source map remapping of the printed stack", () => { stdout: "pipe", stderr: "pipe", }); + children.push(proc); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); return { dir: String(dir), out: JSON.parse(stdout), stderr, exitCode }; } @@ -253,6 +266,163 @@ describe("source map remapping of the printed stack", () => { expect(exitCode).toBe(1); }); + // The error printer reads files by path: the file of the top frame for the + // code frame, the `.map` next to it, and an original source that map names. + // error.stack alone reads the `.map` too. Any of these paths can name a FIFO + // by then. Opening one blocks until a writer shows up, so the process used to + // stay in open() forever instead of printing the error and exiting. With a + // writer it took the bytes that were in the pipe and stayed in read(). Only a + // regular file is read now. What is printed is what a missing file gives. + describe.skipIf(isWindows)("a path the error printer reads names a FIFO", () => { + const swapped = { + "swapped.ts": [ + "type Padding1 = { a: number };", + "type Padding2 = { b: string };", + "export function thrower(): never {", + ' throw new Error("HOSTILE");', + "}", + "", + ].join("\n"), + "main.js": [ + 'import { renameSync } from "node:fs";', + 'import { thrower } from "./swapped.ts";', + 'renameSync(import.meta.dir + "/fifo", import.meta.dir + "/swapped.ts");', + "const out = {};", + "try { thrower(); } catch (e) { out.inspect = Bun.inspect(e); }", + "console.log(JSON.stringify(out));", + "thrower();", + "", + ].join("\n"), + }; + // The throw is on line 4 of the original module and on line 2 after type + // stripping. + const thrower = [expect.stringMatching(/^at thrower \(swapped\.ts:4:\d+\)$/)]; + const remapped = { inspect: thrower, uncaught: thrower }; + + test.concurrent("the file of a module bun transpiled, no writer", async () => { + const { dir, out, stderr, exitCode } = await run(swapped, cwd => mkfifo(`${cwd}/fifo`)); + expect({ + inspect: frames(out.inspect, dir, ["swapped.ts"]), + uncaught: frames(stderr, dir, ["swapped.ts"]), + }).toEqual(remapped); + expect(exitCode).toBe(1); + }); + + test.concurrent("the file of a module bun transpiled, a writer with bytes in the pipe", async () => { + let fd; + try { + const { dir, out, stderr, exitCode } = await run(swapped, cwd => { + mkfifo(`${cwd}/fifo`); + // Both ends: the child finds a writer, and the read below never blocks. + fd = openSync(`${cwd}/fifo`, constants.O_RDWR | constants.O_NONBLOCK); + writeSync(fd, "not source code\n"); + }); + expect({ + inspect: frames(out.inspect, dir, ["swapped.ts"]), + uncaught: frames(stderr, dir, ["swapped.ts"]), + }).toEqual(remapped); + const inPipe = Buffer.alloc(64); + expect(inPipe.toString("utf8", 0, readSync(fd, inPipe))).toBe("not source code\n"); + expect(exitCode).toBe(1); + } finally { + if (fd !== undefined) closeSync(fd); + } + }); + + // The frame of code run through node:vm names whatever its sourceURL says. + const vmScript = url => + [ + 'import vm from "node:vm";', + 'console.log("{}");', + `vm.runInNewContext(\`function thrower() { throw new Error("HOSTILE"); }\\nthrower();\\n//# sourceURL=\${import.meta.dir}/${url}\`);`, + "", + ].join("\n"); + + test.concurrent("the file a sourceURL names", async () => { + const { dir, stderr, exitCode } = await run({ "main.js": vmScript("fifo.js") }, cwd => mkfifo(`${cwd}/fifo.js`)); + expect(frames(stderr, dir, ["fifo.js"])).toEqual([expect.stringMatching(/^at thrower \(fifo\.js:1:\d+\)$/)]); + expect(exitCode).toBe(1); + }); + + // The printer used to look up the package.json of the frame's directory, + // for a module type it never used. In a directory nothing was loaded from, + // that read the directory and opened its package.json. + test.concurrent("the package.json of the directory a sourceURL names", async () => { + const { dir, stderr, exitCode } = await run({ "main.js": vmScript("cold/x.js") }, cwd => { + mkdirSync(`${cwd}/cold`); + mkfifo(`${cwd}/cold/fifo`); + symlinkSync("fifo", `${cwd}/cold/package.json`); + }); + expect(frames(stderr, dir, ["x.js"])).toEqual([expect.stringMatching(/^at thrower \(cold\/x\.js:1:\d+\)$/)]); + expect(exitCode).toBe(1); + }); + + // No code has to run under that name: the frames of a string assigned to + // error.stack are enough. + test.concurrent("the file a frame of an assigned error.stack names", async () => { + const { dir, stderr, exitCode } = await run( + { + "main.js": [ + 'const e = new Error("HOSTILE");', + 'e.stack = "Error: HOSTILE\\n at thrower (" + import.meta.dir + "/fifo.js:3:7)";', + 'console.log("{}");', + "throw e;", + "", + ].join("\n"), + }, + cwd => mkfifo(`${cwd}/fifo.js`), + ); + expect(frames(stderr, dir, ["fifo.js"])).toEqual(["at thrower (fifo.js:3:7)"]); + expect(exitCode).toBe(1); + }); + + const prebuilt = [ + "// @bun", + 'function thrower() { throw new Error("HOSTILE"); }', + "const out = {};", + "try { thrower(); } catch (e) { out.stack = e.stack; }", + "console.log(JSON.stringify(out));", + "thrower();", + "", + ].join("\n"); + + // Without a map the frames stay as they are. + test.concurrent("the .map next to a prebuilt file, which error.stack reads too", async () => { + const { dir, out, stderr, exitCode } = await run({ "main.js": prebuilt }, cwd => mkfifo(`${cwd}/main.js.map`)); + const unmapped = [expect.stringMatching(/^at thrower \(main\.js:2:\d+\)$/)]; + expect({ + stack: frames(out.stack, dir, ["main.js"]).slice(0, 1), + uncaught: frames(stderr, dir, ["main.js"]).slice(0, 1), + }).toEqual({ stack: unmapped, uncaught: unmapped }); + expect(exitCode).toBe(1); + }); + + // One segment at column 0 of generated lines 2, 4 and 6 (to orig.ts:11:5, + // 21:5 and 31:5), and no `sourcesContent`: the printer looks for orig.ts on + // disk. + test.concurrent("an original source the map names", async () => { + const map = { + version: 3, + sources: ["orig.ts"], + sourcesContent: [null], + names: [], + mappings: ";AAUI;;AAUA;;AAUA", + }; + const { dir, out, stderr, exitCode } = await run( + { "main.js": prebuilt, "main.js.map": JSON.stringify(map) }, + cwd => mkfifo(`${cwd}/orig.ts`), + ); + expect({ + stack: frames(out.stack, dir, ["orig.ts"]), + uncaught: frames(stderr, dir, ["orig.ts"]), + }).toEqual({ + stack: ["at thrower (orig.ts:11:5)", "at orig.ts:21:5"], + uncaught: ["at thrower (orig.ts:11:5)", "at orig.ts:31:5"], + }); + expect(exitCode).toBe(1); + }); + }); + // Modules bun transpiled itself. `present.ts` stays on disk; `deleted.ts` is // removed after it was loaded, so the code frame can no longer be read back. // Reading error.stack first makes the printer start from the already From 03fd5dd2ff33a0a6ba703a4f970f79f875e5fb8a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 11 Sep 2026 13:01:05 +0000 Subject: [PATCH 2/7] Select the non-regular file policy from disable_transpilying It is the same condition as the PrintSource comparison today, and it is the boolean the neighboring ParseOptions fields already use. --- src/runtime/jsc_hooks.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index 6eabf9461848..3916c8e7a99d 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -2606,7 +2606,7 @@ fn transpile_source_code_inner( file_descriptor: None, // The error printer must not block on, or read from, what is // no longer the regular file the loader read. - non_regular_file: if args.flags == FetchFlags::PrintSource { + non_regular_file: if disable_transpilying { bun_resolver::cache::NonRegularFile::Reject } else { bun_resolver::cache::NonRegularFile::Read From 94c7624c361ad7e815e5d00f06373ef1bab4f926 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 11 Sep 2026 13:05:35 +0000 Subject: [PATCH 3/7] Keep each added comment to one line The compile cache note goes back to its place, now under the same condition as the watcher registration above it. --- src/jsc/VirtualMachine.rs | 4 +--- src/resolver/lib.rs | 8 ++------ src/runtime/jsc_hooks.rs | 11 ++++++----- src/sourcemap/lib.rs | 3 +-- 4 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index d352d7962906..cd485b7c9de2 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -4458,9 +4458,7 @@ impl VirtualMachine { } else { bun_ast::Loader::File }), - // The one caller fetches with `PrintSource`, which returns the - // file as it is. A `package.json` lookup for the module type would - // scan a directory the error printer has no other reason to touch. + // Unused: the one caller fetches with `PrintSource`, which does not parse. module_type: bun_bundler::options::ModuleType::Unknown, source_code_printer: printer.as_ptr(), // `fetchWithoutOnLoadPlugins` forbids the async path. diff --git a/src/resolver/lib.rs b/src/resolver/lib.rs index 626bf89268b9..db8122535a22 100644 --- a/src/resolver/lib.rs +++ b/src/resolver/lib.rs @@ -2088,16 +2088,12 @@ pub mod cache { } } - /// What [`Fs::read_file_with_allocator`] does with a path that is not a - /// regular file. + /// What [`Fs::read_file_with_allocator`] does with a path that is not a regular file. #[derive(Clone, Copy, PartialEq, Eq)] pub enum NonRegularFile { /// Open and read it like a file. A FIFO blocks until it has a writer. Read, - /// Fail (`EISDIR` for a directory, `ENODEV` otherwise) without - /// blocking on it or reading a byte from it. For a read that happens - /// after the fact, such as the error printer's re-read of a module - /// that has already run. + /// Fail through [`bun_sys::File::open_regular_at`]: no blocking open, no byte read. Reject, } diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index 3916c8e7a99d..96ba9f4c12ab 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -2604,8 +2604,7 @@ fn transpile_source_code_inner( loader, dirname_fd: bun_sys::Fd::INVALID, file_descriptor: None, - // The error printer must not block on, or read from, what is - // no longer the regular file the loader read. + // The error printer reads after the fact: the path can name a FIFO by now. non_regular_file: if disable_transpilying { bun_resolver::cache::NonRegularFile::Reject } else { @@ -2692,11 +2691,13 @@ fn transpile_source_code_inner( hash, package_json, ); - // Node compile cache: record the failed module so exit-time - // persist logs the "was not initialized" skip (Node parity). - note_compile_cache_parse_failure(path, loader, module_type); } arena_guard.2 = false; // give_back_arena = false + // Node compile cache: record the failed module so exit-time + // persist logs the "was not initialized" skip (Node parity). + if !disable_transpilying { + note_compile_cache_parse_failure(path, loader, module_type); + } return Err(crate::Error::ParseError); }; diff --git a/src/sourcemap/lib.rs b/src/sourcemap/lib.rs index cd0b10bfaa96..dc7e6f6f9d42 100644 --- a/src/sourcemap/lib.rs +++ b/src/sourcemap/lib.rs @@ -527,8 +527,7 @@ pub(crate) fn get_source_map_impl( let load_path = bun_core::ZStr::from_buf(&load_path_buf[..], source_filename.len() + 4); - // Reached from `error.stack` and the error printer: a FIFO - // here must not block them. + // `error.stack` and the error printer get here: a FIFO must not block them. let data = match bun_sys::File::read_regular_from(bun_core::Fd::cwd(), load_path) { Ok(data) => data, Err(_) => break 'try_external, From 7fb60565df908e0aec99144e5eb29b14061f67bb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 1 Oct 2026 09:55:41 +0000 Subject: [PATCH 4/7] Delete Loader::is_js_like Its one caller was the package.json lookup in get_loader_and_virtual_source, which this branch removes. --- src/ast/loader.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/ast/loader.rs b/src/ast/loader.rs index cf748647a6d8..b08fe9084ad8 100644 --- a/src/ast/loader.rs +++ b/src/ast/loader.rs @@ -109,11 +109,6 @@ impl Loader { self == Loader::Css } - #[inline] - pub fn is_js_like(self) -> bool { - matches!(self, Loader::Jsx | Loader::Js | Loader::Ts | Loader::Tsx) - } - pub fn should_copy_for_bundling(self) -> bool { matches!( self, From cb18c76219846aaaa9136c9fbe0af92a00f09184 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 1 Oct 2026 09:55:44 +0000 Subject: [PATCH 5/7] test: say why the child cleanup is in afterAll --- test/js/bun/util/inspect-error.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/js/bun/util/inspect-error.test.js b/test/js/bun/util/inspect-error.test.js index 9b7e37f6db79..be3dc229afc3 100644 --- a/test/js/bun/util/inspect-error.test.js +++ b/test/js/bun/util/inspect-error.test.js @@ -253,8 +253,8 @@ describe("source map remapping of the printed stack", () => { .map(line => line.replaceAll(prefix, "")); } - // A child that never exits fails its test by timeout. It must not outlive - // the run too. + // A child that never exits fails its test by timeout and must not outlive the run. afterAll, not + // afterEach: the tests run concurrently, and a hook is not told which test it runs for. const children = []; afterAll(() => { for (const child of children) child.kill("SIGKILL"); From 899659eecf35a31476a0698d24b7b4e7d5324f66 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 1 Oct 2026 10:47:03 +0000 Subject: [PATCH 6/7] test: check only the first frame of the node:vm errors The script has a second frame under the same sourceURL, its top-level call. The printer does not print it today: a stack line with no function name ends its parse of error.stack. The two tests passed because of that, and must not depend on it. --- test/js/bun/util/inspect-error.test.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/js/bun/util/inspect-error.test.js b/test/js/bun/util/inspect-error.test.js index be3dc229afc3..2f1f3e954e32 100644 --- a/test/js/bun/util/inspect-error.test.js +++ b/test/js/bun/util/inspect-error.test.js @@ -383,7 +383,8 @@ describe("source map remapping of the printed stack", () => { } }); - // The frame of code run through node:vm names whatever its sourceURL says. + // The frames of code run through node:vm name whatever its sourceURL says. Whether the script's + // top-level frame is printed after `thrower` is not what these tests check: the first frame is. const vmScript = url => [ 'import vm from "node:vm";', @@ -394,7 +395,9 @@ describe("source map remapping of the printed stack", () => { test.concurrent("the file a sourceURL names", async () => { const { dir, stderr, exitCode } = await run({ "main.js": vmScript("fifo.js") }, cwd => mkfifo(`${cwd}/fifo.js`)); - expect(frames(stderr, dir, ["fifo.js"])).toEqual([expect.stringMatching(/^at thrower \(fifo\.js:1:\d+\)$/)]); + expect(frames(stderr, dir, ["fifo.js"]).slice(0, 1)).toEqual([ + expect.stringMatching(/^at thrower \(fifo\.js:1:\d+\)$/), + ]); expect(exitCode).toBe(1); }); @@ -407,7 +410,9 @@ describe("source map remapping of the printed stack", () => { mkfifo(`${cwd}/cold/fifo`); symlinkSync("fifo", `${cwd}/cold/package.json`); }); - expect(frames(stderr, dir, ["x.js"])).toEqual([expect.stringMatching(/^at thrower \(cold\/x\.js:1:\d+\)$/)]); + expect(frames(stderr, dir, ["x.js"]).slice(0, 1)).toEqual([ + expect.stringMatching(/^at thrower \(cold\/x\.js:1:\d+\)$/), + ]); expect(exitCode).toBe(1); }); From 7c2c08303c6f10ce2601c529f02f7a9864189248 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 1 Oct 2026 13:30:34 +0000 Subject: [PATCH 7/7] open_regular_at returns only the file No caller read the size it also returned. The check of a caller's handle in read_file_with_allocator goes too: no caller passes both a handle and NonRegularFile::Reject. --- src/resolver/lib.rs | 11 +---------- src/sys/file.rs | 17 +++++------------ 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/src/resolver/lib.rs b/src/resolver/lib.rs index 9c066598cd75..89b3787434be 100644 --- a/src/resolver/lib.rs +++ b/src/resolver/lib.rs @@ -2352,9 +2352,7 @@ pub mod cache { let open_at = |dir: Fd, path: &[u8], flags: i32| match non_regular_file { NonRegularFile::Read => bun_sys::File::openat(dir, path, flags, 0), - NonRegularFile::Reject => { - bun_sys::File::open_regular_at(dir, path).map(|(file, _size)| file) - } + NonRegularFile::Reject => bun_sys::File::open_regular_at(dir, path), }; let open_path = |path: &[u8]| open_at(Fd::cwd(), path, bun_sys::O::RDONLY | bun_sys::O::CLOEXEC); @@ -2392,13 +2390,6 @@ pub mod cache { }; let file_handle = bun_sys::File::borrow(&fd); - // A caller's handle did not go through `open_regular_at`. - if _file_handle.is_some() && non_regular_file == NonRegularFile::Reject { - file_handle - .ensure_regular(path) - .map_err(crate::Error::from)?; - } - #[cfg(not(windows))] // skip on Windows because NTCreateFile will do it. bun_core::scoped_log!( CacheFs, diff --git a/src/sys/file.rs b/src/sys/file.rs index 9c8e008109ce..8f56e4babd0e 100644 --- a/src/sys/file.rs +++ b/src/sys/file.rs @@ -335,8 +335,8 @@ impl File { } // ── one-shot path helpers (open + io + close) ─────────────────────── - /// Open `path` for reading and return it with its size; `EISDIR` for a directory, `ENODEV` for any other non-regular file. - pub fn open_regular_at(dir: impl AsFd, path: &[u8]) -> Maybe<(Self, u64)> { + /// Open `path` for reading; `EISDIR` for a directory, `ENODEV` for any other non-regular file. + pub fn open_regular_at(dir: impl AsFd, path: &[u8]) -> Maybe { let dir = dir.as_fd(); // On Windows `O_NONBLOCK` would make the handle overlapped; the fstat still rejects there. #[cfg(unix)] @@ -344,23 +344,16 @@ impl File { #[cfg(not(unix))] let flags = O::RDONLY | O::CLOEXEC; let file = Self::openat(dir, path, flags, 0)?; - let size = file.ensure_regular(path)?; - Ok((file, size)) - } - /// The check of [`File::open_regular_at`] for a file opened with other flags: the size of a regular file, else its error. - pub fn ensure_regular(&self, path: &[u8]) -> Maybe { - let st = self.stat().map_err(|e| e.with_path(path))?; - let mode = st.st_mode as Mode; + let mode = file.stat().map_err(|e| e.with_path(path))?.st_mode as Mode; if !S::ISREG(mode) { let errno = if S::ISDIR(mode) { E::EISDIR } else { E::ENODEV }; return Err(Error::new(errno, Tag::open).with_path(path)); } - Ok(st.st_size.max(0) as u64) + Ok(file) } /// [`File::read_from`] of a regular file only ([`File::open_regular_at`]). pub fn read_regular_from(dir: impl AsFd, path: &[u8]) -> Maybe> { - let (file, _size) = Self::open_regular_at(dir, path)?; - file.read_to_end() + Self::open_regular_at(dir, path)?.read_to_end() } /// Open + read + close. Accepts `&[u8]`; `&ZStr` callers deref-coerce. pub fn read_from(dir: impl AsFd, path: &[u8]) -> Maybe> {