Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5724,7 +5724,7 @@ impl VirtualMachine {
};

if enable_source_code_preview.get() && code.slice().is_empty() {
exception.collect_source_lines(error_instance, global);
exception.collect_source_lines(error_instance, global, top as u8);
}

// Direct copy; both sides are `bun_core::Ordinal`.
Expand Down Expand Up @@ -5768,7 +5768,9 @@ impl VirtualMachine {
*source_code_slice = Some(code);
}
} else if enable_source_code_preview.get() {
exception.collect_source_lines(error_instance, global);
// Nothing to remap through (node:vm script, eval, new Function):
// excerpt the frame picked above straight from its JSC source.
exception.collect_source_lines(error_instance, global, top as u8);
}

drop(top_source_url);
Expand Down
19 changes: 16 additions & 3 deletions src/jsc/ZigException.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@ use crate::{JSErrorCode, JSGlobalObject, JSRuntimeType, JSValue, ZigStackFrame,

// SAFETY (safe fn): `JSValue` is a by-value scalar; `JSGlobalObject` is an
// opaque `UnsafeCell`-backed handle (`&` is ABI-identical to non-null `*mut`);
// `ZigException` is a `#[repr(C)]` out-param the C++ side fills in-place.
// `ZigException` is a `#[repr(C)]` out-param the C++ side fills in-place;
// `frame_index` is a by-value scalar that C++ bounds-checks against
// `stack.frames_len`.
unsafe extern "C" {
pub(crate) safe fn ZigException__collectSourceLines(
js_value: JSValue,
global: &JSGlobalObject,
exception: &mut ZigException,
frame_index: u8,
);
}

Expand Down Expand Up @@ -50,8 +53,18 @@ pub struct ZigException {
}

impl ZigException {
pub(crate) fn collect_source_lines(&mut self, value: JSValue, global: &JSGlobalObject) {
ZigException__collectSourceLines(value, global, self);
/// Fills `stack.source_lines_*` with the source around
/// `stack.frames()[frame_index]`, read from that frame's JSC source
/// provider. For sources bun transpiled, `remap_zig_exception` reads the
/// original file instead; this is the path for everything else (`node:vm`
/// scripts, `eval`, `new Function`).
pub(crate) fn collect_source_lines(
&mut self,
value: JSValue,
global: &JSGlobalObject,
frame_index: u8,
) {
ZigException__collectSourceLines(value, global, self, frame_index);
}

// Kept as explicit `deinit` (not `Drop`) — this is a #[repr(C)] FFI
Expand Down
36 changes: 21 additions & 15 deletions src/jsc/bindings/ZigException.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,17 +225,17 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr
}

static void populateStackFrame(JSC::VM& vm, ZigStackTrace& trace, const JSC::StackFrame& stackFrame,
ZigStackFrame& frame, bool is_top, JSC::SourceProvider** referenced_source_provider, JSC::JSGlobalObject* globalObject, PopulateStackTraceFlags flags, FinalizerSafety finalizerSafety)
ZigStackFrame& frame, JSC::SourceProvider** referenced_source_provider, JSC::JSGlobalObject* globalObject, PopulateStackTraceFlags flags, FinalizerSafety finalizerSafety)
{
if (flags == PopulateStackTraceFlags::OnlyPosition) {
populateStackFrameMetadata(vm, globalObject, stackFrame, frame, finalizerSafety);
populateStackFramePosition(stackFrame, nullptr,
nullptr,
0, frame.position, referenced_source_provider, flags);
} else if (flags == PopulateStackTraceFlags::OnlySourceLines) {
populateStackFramePosition(stackFrame, is_top ? trace.source_lines_ptr : nullptr,
is_top ? trace.source_lines_numbers : nullptr,
is_top ? trace.source_lines_to_collect : 0, frame.position, referenced_source_provider, flags);
populateStackFramePosition(stackFrame, trace.source_lines_ptr,
trace.source_lines_numbers,
trace.source_lines_to_collect, frame.position, referenced_source_provider, flags);
}
}

Expand Down Expand Up @@ -421,7 +421,11 @@ class V8StackTraceIterator {
}
};

static void populateStackTrace(JSC::VM& vm, const WTF::Vector<JSC::StackFrame>& frames, ZigStackTrace& trace, JSC::JSGlobalObject* globalObject, PopulateStackTraceFlags flags, FinalizerSafety finalizerSafety = FinalizerSafety::NotInFinalizer)
// OnlyPosition fills `trace.frames_ptr` from `frames`. OnlySourceLines runs
// later, after Rust has filtered the frames and picked the one whose source
// to excerpt (`remap_zig_exception`), and collects the source lines of
// `trace.frames_ptr[source_lines_frame_index]` only.
static void populateStackTrace(JSC::VM& vm, const WTF::Vector<JSC::StackFrame>& frames, ZigStackTrace& trace, JSC::JSGlobalObject* globalObject, PopulateStackTraceFlags flags, FinalizerSafety finalizerSafety = FinalizerSafety::NotInFinalizer, uint8_t source_lines_frame_index = 0)
{
if (flags == PopulateStackTraceFlags::OnlyPosition) {
uint8_t frame_i = 0;
Expand All @@ -439,18 +443,19 @@ static void populateStackTrace(JSC::VM& vm, const WTF::Vector<JSC::StackFrame>&

ZigStackFrame& frame = trace.frames_ptr[frame_i];
frame.jsc_stack_frame_index = static_cast<int32_t>(stack_frame_i);
populateStackFrame(vm, trace, frames[stack_frame_i], frame, frame_i == 0, &trace.referenced_source_provider, globalObject, flags, finalizerSafety);
populateStackFrame(vm, trace, frames[stack_frame_i], frame, &trace.referenced_source_provider, globalObject, flags, finalizerSafety);
stack_frame_i++;
frame_i++;
}
trace.frames_len = frame_i;
} else if (flags == PopulateStackTraceFlags::OnlySourceLines) {
for (uint8_t i = 0; i < trace.frames_len; i++) {
ZigStackFrame& frame = trace.frames_ptr[i];
if (frame.jsc_stack_frame_index < 0 || static_cast<size_t>(frame.jsc_stack_frame_index) >= frames.size())
continue;
populateStackFrame(vm, trace, frames[frame.jsc_stack_frame_index], frame, i == 0, &trace.referenced_source_provider, globalObject, flags, finalizerSafety);
}
if (source_lines_frame_index >= trace.frames_len)
return;
ZigStackFrame& frame = trace.frames_ptr[source_lines_frame_index];
// -1 when the frames were parsed out of an `error.stack` string.
if (frame.jsc_stack_frame_index < 0 || static_cast<size_t>(frame.jsc_stack_frame_index) >= frames.size())
return;
populateStackFrame(vm, trace, frames[frame.jsc_stack_frame_index], frame, &trace.referenced_source_provider, globalObject, flags, finalizerSafety);
}
}

Expand Down Expand Up @@ -866,7 +871,8 @@ extern "C" [[ZIG_EXPORT(check_slow)]] void JSC__JSValue__toZigException(JSC::Enc
exceptionFromString(*exception, value, global);
}

extern "C" void ZigException__collectSourceLines(JSC::EncodedJSValue jsException, JSC::JSGlobalObject* global, ZigException* exception)
// `frame_index` indexes `exception->stack.frames_ptr` (after Rust's frame filtering).
extern "C" void ZigException__collectSourceLines(JSC::EncodedJSValue jsException, JSC::JSGlobalObject* global, ZigException* exception, uint8_t frame_index)
{
JSC::JSValue value = JSC::JSValue::decode(jsException);
if (value == JSC::JSValue {}) {
Expand All @@ -878,7 +884,7 @@ extern "C" void ZigException__collectSourceLines(JSC::EncodedJSValue jsException
JSValue unwrapped = jscException->value();

if (jscException->stack().size() > 0) {
populateStackTrace(global->vm(), jscException->stack(), exception->stack, global, PopulateStackTraceFlags::OnlySourceLines);
populateStackTrace(global->vm(), jscException->stack(), exception->stack, global, PopulateStackTraceFlags::OnlySourceLines, FinalizerSafety::NotInFinalizer, frame_index);
}

exceptionFromString(*exception, unwrapped, global);
Expand All @@ -887,7 +893,7 @@ extern "C" void ZigException__collectSourceLines(JSC::EncodedJSValue jsException

if (JSC::ErrorInstance* error = dynamicDowncast<JSC::ErrorInstance>(value)) {
if (error->stackTrace() != nullptr && error->stackTrace()->size() > 0) {
populateStackTrace(global->vm(), *error->stackTrace(), exception->stack, global, PopulateStackTraceFlags::OnlySourceLines, FinalizerSafety::MustNotTriggerGC);
populateStackTrace(global->vm(), *error->stackTrace(), exception->stack, global, PopulateStackTraceFlags::OnlySourceLines, FinalizerSafety::MustNotTriggerGC, frame_index);
}
return;
}
Expand Down
113 changes: 113 additions & 0 deletions test/js/node/vm/vm.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, normalizeBunSnapshot } from "harness";
import { EventEmitter } from "node:events";
import {
compileFunction,
constants,
Expand Down Expand Up @@ -463,6 +464,118 @@ describe("Script", () => {
});
});

// The code frame bun prints above an error is the line of the first frame that
// is in one of the user's sources. A vm script (like eval and new Function) has
// no source map, so that line is cut out of the source JSC compiled. When the
// error is thrown inside a JS builtin or one of bun's own modules, the user's
// frame is not frame 0; the excerpt used to be cut out of frame 0's source
// regardless, i.e. it showed the builtin's own text and line number.
describe("code frame of an error thrown inside a builtin called from a source without a source map", () => {
type Expected = {
// Names the user's source in the stack; the excerpt has to be its line.
filename: string;
// The `name: message` line printed under the code frame.
header: string;
// The text of the line that calls into the builtin.
excerpt: string;
};

function expectCodeFrame(printed: string, { filename, header, excerpt }: Expected) {
const lines = printed.split("\n");
const headerIndex = lines.indexOf(header);
expect(headerIndex).toBeGreaterThanOrEqual(2);
const userFrame = printed.match(new RegExp(`${RegExp.escape(filename)}:(\\d+):\\d+`));
expect(userFrame).not.toBeNull();
// Directly above the header are the excerpt and the caret. The excerpt is
// labelled with the line the user's frame reports (line 2 of the source
// below; for new Function, 2 plus its `function anonymous(\n) {` wrapper).
expect(lines.slice(headerIndex - 2, headerIndex)).toEqual([
`${userFrame![1]} | ${excerpt}`,
expect.stringMatching(/^ +\^$/),
]);
}

// The builtin is called from line 2 so that the excerpt's line number has to
// come from the user's frame as well as its text.
const source = '"line 1";\n[].reduce((a, b) => a);';
const reduce = {
excerpt: "[].reduce((a, b) => a);",
header: "TypeError: reduce of empty array with no initial value",
};
// displayErrors: false keeps node:vm from rewriting err.stack; the code frame
// under test is the one bun's error printer builds from the error's frames.
const displayErrors = false;

const cases: (Expected & { name: string; run(filename: string): unknown })[] = [
{
name: "vm.Script",
filename: "/virtual/script.js",
...reduce,
run: filename => new Script(source, { filename }).runInThisContext({ displayErrors }),
},
{
name: "runInNewContext",
filename: "/virtual/new-context.js",
...reduce,
run: filename => runInNewContext(source, {}, { filename, displayErrors }),
},
{
name: "eval",
filename: "/virtual/eval.js",
...reduce,
run: filename => (0, eval)(`${source}\n//# sourceURL=${filename}`),
},
{
name: "new Function",
filename: "/virtual/function.js",
...reduce,
run: filename => new Function(`${source}\n//# sourceURL=${filename}`)(),
},
{
// emit() with no "error" listener throws from inside bun's node:events,
// so the frame on top is a node:events frame rather than a JS builtin's.
name: "node: module on top of the stack",
filename: "/virtual/events.js",
header: "error: Unhandled error. (undefined)",
excerpt: 'emitter.emit("error");',
run: filename =>
runInNewContext(
'"line 1";\nemitter.emit("error");',
{ emitter: new EventEmitter() },
{ filename, displayErrors },
),
},
];

test.each(cases)("Bun.inspect: $name", testCase => {
let thrown: unknown;
try {
testCase.run(testCase.filename);
} catch (e) {
thrown = e;
}
expectCodeFrame(Bun.inspect(thrown), testCase);
});

test("uncaught error", async () => {
const filename = "/virtual/uncaught.js";
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`require("node:vm").runInThisContext(${JSON.stringify(source)}, ${JSON.stringify({ filename, displayErrors })})`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stdout).toBe("");
expectCodeFrame(stderr, { filename, ...reduce });
expect(exitCode).toBe(1);
});
});

type TestRunInContextArg =
| { fn: typeof runInContext; isIsolated: true; isNew?: boolean }
| { fn: typeof runInThisContext; isIsolated?: false; isNew?: boolean };
Expand Down