-
Notifications
You must be signed in to change notification settings - Fork 5k
bundler: report unknown node: builtins as No such built-in module #39744
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,11 +4,11 @@ use core::fmt; | |
|
|
||
| use bun_core::Output; | ||
| use bun_jsc::{ | ||
| CallFrame, JSGlobalObject, JSValue, JsError, JsResult, | ||
| CallFrame, CallerSrcLoc, JSGlobalObject, JSValue, JsError, JsResult, | ||
| ConsoleObject, JSFunction, JSPropertyIterator, JSString, | ||
| }; | ||
| use bun_jsc::{JsClass as _, StringJsc as _}; | ||
| use bun_core::ZigString; | ||
| use bun_core::{ZStr, ZigString}; | ||
| use bun_jsc::js_promise; | ||
| use bun_jsc::virtual_machine::VirtualMachine; | ||
| use bun_core::strings; | ||
|
|
@@ -41,6 +41,10 @@ pub struct Expect { | |
| pub(crate) flags: Cell<Flags>, | ||
| pub(crate) parent: Option<bun_test::RefDataPtr>, | ||
| pub(crate) custom_label: bun_core::String, | ||
| /// Caller source location captured when `expect()` was called. Not in | ||
| /// tail position (its result feeds the matcher member access), so it is | ||
| /// intact even when JSC elides the caller frame for a tail-called matcher. | ||
| pub(crate) caller_src_loc: CallerSrcLoc, | ||
| } | ||
|
|
||
|
|
||
|
|
@@ -684,6 +688,7 @@ impl Expect { | |
| #[allow(clippy::boxed_local)] | ||
| pub fn finalize(mut self: Box<Self>) { | ||
| self.custom_label.deref(); | ||
| self.caller_src_loc.str.deref(); | ||
| // RefDataPtr = RefPtr<RefData> has NO `Drop` impl (src/ptr/ref_count.rs) | ||
| // so the Box drop below would leak the +1 — release explicitly. | ||
| if let Some(parent) = self.parent.take() { | ||
|
|
@@ -719,9 +724,16 @@ impl Expect { | |
| // error path between ref creation and the wrapper taking ownership; from | ||
| // then on `Expect::finalize` derefs `parent` (RefDataPtr has no Drop). | ||
|
|
||
| // Capture the caller src loc now: `expect()` is never in tail position | ||
| // (its result feeds the matcher member access), so the caller frame is | ||
| // intact here even when the matcher call itself is tail-called and JSC | ||
| // elides the caller frame (see `inline_snapshot` fallback). | ||
| let caller_src_loc = callframe.get_caller_src_loc(global_this); | ||
|
|
||
| let expect = Expect { | ||
| flags: Cell::new(Flags::default()), | ||
| custom_label, | ||
| caller_src_loc, | ||
| parent: active_execution_entry_ref, | ||
| }; | ||
| // `JsClass::to_js` boxes `self` and hands the pointer to `${T}__create`. | ||
|
|
@@ -1120,7 +1132,25 @@ impl Expect { | |
| let buntest = buntest_strong.get(); | ||
|
|
||
| // 1. find the src loc of the snapshot | ||
| let srcloc = call_frame.get_caller_src_loc(global_this); | ||
| let mut srcloc = call_frame.get_caller_src_loc(global_this); | ||
| // When the matcher is the last expression of a function body, | ||
| // JavaScriptCore applies tail-call optimization and elides the | ||
| // caller frame, so the walk above finds no location. `expect()` | ||
| // itself is never in tail position (its result feeds the matcher | ||
| // member access), so fall back to the location captured when the | ||
| // `Expect` was created — it points at the `expect(` call, and the | ||
| // matcher call site is relocated from the test file below. | ||
| let mut relocated_from_expect = false; | ||
| if srcloc.str.is_empty() { | ||
| // release the +1 captured by the failed walk above | ||
| bun_core::OwnedString::new(srcloc.str); | ||
| srcloc = CallerSrcLoc { | ||
| str: this.caller_src_loc.str.clone_ref(), | ||
| line: this.caller_src_loc.line, | ||
| column: this.caller_src_loc.column, | ||
| }; | ||
| relocated_from_expect = true; | ||
| } | ||
| // bun_core::String is Copy | ||
| // with no Drop, so wrap in the RAII guard to release the +1 on | ||
| // every exit path (including the early returns below). | ||
|
|
@@ -1143,6 +1173,20 @@ impl Expect { | |
| } | ||
|
|
||
| // 2. save to write later | ||
| if relocated_from_expect { | ||
| // The fallback location points at the `expect(` call, but the | ||
| // writer needs the matcher call site — read the test file and | ||
| // find the `fn_name(` call at/after the `expect()` position. | ||
| if let Some((line, column)) = Self::locate_matcher_call_in_test_file( | ||
| fget_source_path_text, | ||
| u64::from(srcloc.line), | ||
| u64::from(srcloc.column), | ||
| fn_name.as_bytes(), | ||
| ) { | ||
| srcloc.line = u32::try_from(line).unwrap_or(srcloc.line); | ||
| srcloc.column = u32::try_from(column).unwrap_or(srcloc.column); | ||
| } | ||
| } | ||
| runner.snapshots.add_inline_snapshot_to_write(file_id, super::snapshot::InlineSnapshotToWrite { | ||
| line: core::ffi::c_ulong::from(srcloc.line), | ||
| col: core::ffi::c_ulong::from(srcloc.column), | ||
|
|
@@ -1196,6 +1240,73 @@ impl Expect { | |
| Ok(()) | ||
| } | ||
|
|
||
| /// Read the test file and find the first `fn_name(` call site at/after the | ||
| /// given `expect()` position (1-based line/column). Used to relocate the | ||
| /// [`Expect::caller_src_loc`] fallback — which points at `expect(` — to the | ||
| /// actual matcher call site when JSC elided the caller frame (tail call). | ||
| fn locate_matcher_call_in_test_file( | ||
| test_file_path: &ZigString, | ||
| expect_line: u64, | ||
| expect_col: u64, | ||
| fn_name: &[u8], | ||
| ) -> Option<(u64, u64)> { | ||
| let mut path_buf = test_file_path.to_vec(); | ||
| path_buf.push(0); | ||
| // SAFETY: NUL appended above | ||
| let path_z = ZStr::from_slice_with_nul(&path_buf); | ||
| let fd = match bun_sys::open(path_z, bun_sys::O::RDONLY, 0) { | ||
| bun_sys::Result::Ok(r) => r, | ||
| bun_sys::Result::Err(_) => return None, | ||
| }; | ||
| let file_text: Vec<u8> = match bun_sys::File::from_fd(fd).read_to_end() { | ||
| bun_sys::Result::Ok(t) => t, | ||
| bun_sys::Result::Err(_) => return None, | ||
| }; | ||
| Self::locate_matcher_call(&file_text, expect_line, expect_col, fn_name) | ||
| } | ||
|
|
||
| /// Search `file_text` for the first `fn_name(` call at/after the given | ||
| /// 1-based line/column and return its line/column. | ||
| fn locate_matcher_call( | ||
| file_text: &[u8], | ||
| expect_line: u64, | ||
| expect_col: u64, | ||
| fn_name: &[u8], | ||
| ) -> Option<(u64, u64)> { | ||
| let start = bun_ast::Source::line_col_to_byte_offset(file_text, 1, 1, expect_line, expect_col)?; | ||
| let mut i = start; | ||
| while i < file_text.len() { | ||
| if file_text[i..].starts_with(fn_name) { | ||
| let after = i + fn_name.len(); | ||
| let mut j = after; | ||
| while j < file_text.len() && matches!(file_text[j], b' ' | b'\t') { | ||
| j += 1; | ||
| } | ||
| if j < file_text.len() | ||
| && file_text[j] == b'(' | ||
| // the name must be a real property access / call token, not | ||
| // a substring of an identifier or of a string/comment | ||
| && (i == 0 | ||
| || !matches!( | ||
| file_text[i - 1], | ||
| b'a'..=b'z' | ||
| | b'A'..=b'Z' | ||
| | b'0'..=b'9' | ||
| | b'_' | ||
| | b'$' | ||
| | b'"' | ||
| | b'\'' | ||
| | b'`' | ||
| )) | ||
| { | ||
| return line_col_of_byte(file_text, i); | ||
|
Comment on lines
+1276
to
+1302
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift Use a JavaScript parser to locate the matcher call. The byte scan accepts Parse the test source and select the matcher call AST node after the captured As per coding guidelines, “use real parsers instead of prefix stripping or regex heuristics for user input.” 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| } | ||
| } | ||
| i += 1; | ||
| } | ||
| None | ||
| } | ||
|
|
||
| pub(crate) fn snapshot( | ||
| &self, | ||
| global_this: &JSGlobalObject, | ||
|
|
@@ -3268,6 +3379,51 @@ unsafe extern "C" { | |
| // Exports: handled by #[unsafe(no_mangle)] on: | ||
| // ExpectMatcherUtils_createSigleton, Expect_readFlagsAndProcessPromise, ExpectCustomAsymmetricMatcher__execute | ||
|
|
||
| /// 1-based line/column of the codepoint starting at `byte` in `text`, using the | ||
| /// same newline/column semantics as `Source::line_col_to_byte_offset` (so the | ||
| /// round trip `byte → line/col → byte` is exact). | ||
| fn line_col_of_byte(text: &[u8], byte: usize) -> Option<(u64, u64)> { | ||
| use bun_core::strings::{CodepointIterator, Cursor}; | ||
| let iter_ = CodepointIterator::init(text); | ||
| let mut iter = Cursor::default(); | ||
| let _ = iter_.next(&mut iter); | ||
| let mut line: u64 = 1; | ||
| let mut column: u64 = 1; | ||
| loop { | ||
| let c = iter.c; | ||
| let start = iter.i as usize; | ||
| if start == byte { | ||
| return Some((line, column)); | ||
| } | ||
| if start > byte { | ||
| return None; | ||
| } | ||
| if !iter_.next(&mut iter) { | ||
| break; | ||
| } | ||
| match c { | ||
| 0x0A => { | ||
| column = 1; | ||
| line += 1; | ||
| } | ||
| 0x0D => { | ||
| column = 1; | ||
| line += 1; | ||
| if iter.c == ('\n' as i32) { | ||
| let _ = iter_.next(&mut iter); | ||
| } | ||
| } | ||
| 0x2028 | 0x2029 => { | ||
| line += 1; | ||
| column = 1; | ||
| } | ||
| _ => { | ||
| column += if c > 0xFFFF { 2 } else { 1 }; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
Comment on lines
+3382
to
+3426
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path("src/runtime/test_runner/expect.rs")
text = path.read_text()
start = text.index("fn line_col_of_byte")
body = text[start:text.index("\n}", start) + 2]
assert "None" not in body.rsplit("}", 1)[0], (
"line_col_of_byte has no terminal Option return after its loop"
)
PYRepository: oven-sh/bun Length of output: 300 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
file="src/runtime/test_runner/expect.rs"
rg -n -A75 -B8 'fn line_col_of_byte' "$file"
printf '\nFunction boundaries:\n'
python3 - <<'PY'
from pathlib import Path
path = Path("src/runtime/test_runner/expect.rs")
text = path.read_text()
start = text.index("fn line_col_of_byte")
brace = text.index("{", start)
depth = 0
end = None
for i in range(brace, len(text)):
if text[i] == "{":
depth += 1
elif text[i] == "}":
depth -= 1
if depth == 0:
end = i + 1
break
assert end is not None
body = text[start:end]
print(body)
print("\nTerminal return tokens:", body.rstrip().splitlines()[-3:])
print("Contains terminal None:", body.rstrip().endswith("None"))
PYRepository: oven-sh/bun Length of output: 4433 Return The loop can reach 🤖 Prompt for AI Agents |
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -91,6 +91,21 @@ describe("ResolveMessage", () => { | |
| expect(err.message).toContain(referrer); | ||
| }); | ||
|
|
||
| it("reports an unknown node: builtin as No such built-in module (bundler)", async () => { | ||
| const spec = "node:definitely-not-a-real-module"; | ||
| const lineText = `import "${spec}";`; | ||
| const fileName = "entry-unknown-node-builtin.js"; | ||
| using dir = tempDir("resolve-unknown-node-builtin", { | ||
| [fileName]: lineText + "\n", | ||
| }); | ||
| const result = await Bun.build({ entrypoints: [path.join(String(dir), fileName)], throw: false }); | ||
| expect(result.success).toBe(false); | ||
| const log: any = result.logs.find(l => l.name === "ResolveMessage"); | ||
| expect(log).toBeDefined(); | ||
| expect(log.specifier).toBe(spec); | ||
| expect(log.message).toBe(`No such built-in module: ${spec}`); | ||
| }); | ||
|
|
||
|
Comment on lines
+94
to
+108
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Assert 📍 Affects 2 files
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| it("preserves non-ASCII in position.lineText and position.file", async () => { | ||
| const lineText = `const caf\u00e9 = 1; import "./na\u00efve-missing.js"; // \u{1F389}`; | ||
| const fileName = "entry-caf\u00e9-\u{1F389}.js"; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Propagate test-file read errors.
Lines 1257-1264 convert an open or read failure into
None. The caller then writes a snapshot using theexpect()location. This can modify the wrong source location after an I/O failure.Return the
bun_sys::Errorthrough a typed error path. Include the test-file path and preserve the underlying error.As per coding guidelines, “Never swallow failures or signal success after failure; propagate I/O, syscall, cleanup, and requested-operation errors explicitly.”
🤖 Prompt for AI Agents
Source: Coding guidelines