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
11 changes: 3 additions & 8 deletions src/jsc/bindings/FormatStackTraceForJS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -188,20 +188,15 @@ WTF::String formatStackTrace(
memset(&remappedFrame, 0, sizeof(ZigStackFrame));

remappedFrame.position.line_zero_based = originalLine.zeroBasedInt();
remappedFrame.position.column_zero_based = 0;

String sourceURLForFrame = err->sourceURL();

// If it's not a Zig::GlobalObject, don't bother source-mapping it.
if (globalObject && !sourceURLForFrame.isEmpty()) {
// https://github.com/oven-sh/bun/issues/3595
if (!sourceURLForFrame.isEmpty()) {
remappedFrame.source_url = Bun::toStringRef(sourceURLForFrame);
// This ensures the lifetime of the sourceURL is accounted for correctly
Bun__remapStackFramePositions(getBunVM(), &remappedFrame, 1);

sourceURLForFrame = remappedFrame.source_url.toWTFString();
}
remappedFrame.source_url = Bun::toStringRef(sourceURLForFrame);
Bun__remapParseErrorFrame(getBunVM(), &remappedFrame, err->line(), err->column());
sourceURLForFrame = remappedFrame.source_url.toWTFString();
}

// there is always a newline before each stack frame line, ensuring that the name + message
Expand Down
18 changes: 18 additions & 0 deletions src/jsc/bindings/ZigException.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,24 @@ static void fromErrorInstance(ZigException& except, JSC::JSGlobalObject* global,
}

if (except.stack.frames_len == 0 && getFromSourceURL) {
// Parse-time SyntaxErrors have no stack; addErrorInfo() still set the C++ fields (#5192).
const String& nativeSourceURL = err->sourceURL();
if (!nativeSourceURL.isEmpty()) {
auto& frame = except.stack.frames_ptr[0];
frame.source_url.deref();
frame.source_url = Bun::toStringRef(nativeSourceURL);
if (err->line() > 0) {
if (auto* zigGlobal = dynamicDowncast<Zig::GlobalObject>(global))
Bun__remapParseErrorFrame(zigGlobal->bunVM(), &frame, err->line(), err->column());
else
frame.position.line_zero_based = OrdinalNumber::fromOneBasedInt(err->line()).zeroBasedInt();
}
except.stack.frames_len = 1;
frame.remapped = true;
except.remapped = true;
return;
}

JSC::JSValue sourceURL = getNonObservable(vm, global, obj, vm.propertyNames->sourceURL);
if (!scope.clearExceptionExceptTermination()) [[unlikely]]
return;
Expand Down
11 changes: 11 additions & 0 deletions src/jsc/bindings/headers-handwritten.h
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,17 @@ bool Bun__deepMatch(

extern "C" void Bun__remapStackFramePositions(void*, ZigStackFrame*, size_t);

ALWAYS_INLINE void Bun__remapParseErrorFrame(void* bunVM, ZigStackFrame* frame, unsigned line, unsigned column)
{
frame->position.line_zero_based = static_cast<int32_t>(line) - 1;
bool columnKnown = column > 0;
// addErrorInfo() column is 0; a lookup there lands on the previous line's tail, so query past end-of-line.
frame->position.column_zero_based = columnKnown ? static_cast<int32_t>(column) - 1 : INT32_MAX;
Bun__remapStackFramePositions(bunVM, frame, 1);
if (!columnKnown)
frame->position.column_zero_based = 0;
}

namespace Inspector {
class ScriptArguments;
}
Expand Down
153 changes: 153 additions & 0 deletions test/regression/issue/05192.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
import { expect, test } from "bun:test";
Comment thread
robobun marked this conversation as resolved.
import { bunEnv, bunExe, tempDir } from "harness";

// https://github.com/oven-sh/bun/issues/5192
//
// When JSC (not Bun's own parser) rejects a module with a SyntaxError, the
// ErrorInstance carries line/sourceURL on its C++ fields but those were never
// surfaced because the error has no JS stack at module-parse time. The error
// printed as a bare "SyntaxError: ..." with no file, line, or code frame.

async function run(files: Record<string, string>, entry: string) {
using dir = tempDir("issue-5192", files);
await using proc = Bun.spawn({
cmd: [bunExe(), entry],
env: { ...bunEnv, NO_COLOR: "1" },
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
return { stdout, stderr, exitCode };
}

// `with` in strict mode is caught by JSC's parser, not Bun's, so it exercises
// the JSC ParserError path directly.
const withInStrict = `"use strict";
export const x = 1;
function foo() {
with ({ a: 1 }) {
console.log(a);
}
}
foo();
`;

test.concurrent("JSC parse SyntaxError in the entry module prints file and line", async () => {
const { stderr, exitCode } = await run({ "entry.mjs": withInStrict }, "entry.mjs");
expect(stderr).toContain("SyntaxError: 'with' statements are not valid in strict mode.");
// File path with a line number must appear somewhere in the output.
expect(stderr).toMatch(/entry\.mjs:\d+/);
// A code frame with the `with` line must appear.
expect(stderr).toMatch(/\bwith\b.*\{ a: 1 \}/);
expect(exitCode).toBe(1);
});

test.concurrent("JSC parse SyntaxError in an imported ESM module prints file and line", async () => {
const { stderr, exitCode } = await run(
{
"module.mjs": withInStrict,
"entry.ts": `import "./module.mjs";`,
},
"entry.ts",
);
expect(stderr).toContain("SyntaxError: 'with' statements are not valid in strict mode.");
expect(stderr).toMatch(/module\.mjs:\d+/);
expect(stderr).toMatch(/\bwith\b.*\{ a: 1 \}/);
expect(exitCode).toBe(1);
});

test.concurrent("JSC parse SyntaxError in an imported .ts module prints file and line", async () => {
const { stderr, exitCode } = await run(
{
"module.ts": withInStrict,
"entry.ts": `import "./module.ts";`,
},
"entry.ts",
);
expect(stderr).toContain("SyntaxError: 'with' statements are not valid in strict mode.");
expect(stderr).toMatch(/module\.ts:\d+/);
expect(stderr).toMatch(/\bwith\b.*\{ a: 1 \}/);
expect(exitCode).toBe(1);
});

test.concurrent("JSC parse SyntaxError: 'delete x' in strict mode prints file and line", async () => {
const { stderr, exitCode } = await run(
{
"module.mjs": `export const x = 1;
function fn() {
delete someVar;
}
fn();
`,
"entry.mjs": `import "./module.mjs";`,
},
"entry.mjs",
);
expect(stderr).toContain("SyntaxError: Cannot delete unqualified property 'someVar' in strict mode.");
expect(stderr).toMatch(/module\.mjs:\d+/);
expect(exitCode).toBe(1);
});

test.concurrent("JSC parse SyntaxError from dynamic import() prints file and line when re-thrown", async () => {
const { stderr, exitCode } = await run(
{
"module.mjs": withInStrict,
"entry.mjs": `try {
await import("./module.mjs");
} catch (e) {
console.error(e);
}
`,
},
"entry.mjs",
);
expect(stderr).toContain("SyntaxError: 'with' statements are not valid in strict mode.");
expect(stderr).toMatch(/module\.mjs:\d+/);
expect(exitCode).toBe(0);
});

test.concurrent("JSC parse SyntaxError in a require()'d CJS module prints the offending line", async () => {
// require() has a JS stack so the `<parse>` frame comes from the .stack
// formatter rather than the error printer's synthetic frame; both paths had
// the same column-0 source-map lookup bug that resolved to the previous line.
const { stderr, exitCode } = await run(
{
"mod.cjs": `"use strict";
with ({ a: 1 }) { console.log(a); }
`,
"main.cjs": `require("./mod.cjs");`,
},
"main.cjs",
);
expect(stderr).toContain("SyntaxError: 'with' statements are not valid in strict mode.");
expect(stderr).toMatch(/mod\.cjs:2\b/);
expect(stderr).toMatch(/2 \|.*\bwith\b/);
expect(exitCode).toBe(1);
});

test.concurrent(
"JSC parse SyntaxError reported line points at the offending statement, not the line before",
async () => {
// addErrorInfo() discards the parser-error column; a naive source-map lookup
// at column 0 resolves to bun's own start-of-line mapping for the *previous*
// source line when the offending statement is indented.
const { stderr, exitCode } = await run(
{
"module.mjs": `export const x = 1;
function foo() {
with ({ a: 1 }) {
console.log(a);
}
}
foo();
`,
},
"module.mjs",
);
expect(stderr).toContain("SyntaxError: 'with' statements are not valid in strict mode.");
expect(stderr).toMatch(/module\.mjs:3\b/);
expect(stderr).toMatch(/3 \|.*\bwith\b/);
expect(exitCode).toBe(1);
},
);
Loading