Skip to content
Merged
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
4 changes: 2 additions & 2 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6618,7 +6618,7 @@ impl VirtualMachine {
if let Some(frame) = top_frame {
if !frame.position.is_invalid() {
let source_url = frame.source_url.to_utf8();
let file = bun_paths::resolve_path::relative(dir, source_url.slice());
let file = crate::ZigStackFrame::relative_source_url(dir, source_url.slice());
let _ = write!(
writer,
"\n::error file={},line={},col={},title=",
Expand Down Expand Up @@ -6680,7 +6680,7 @@ impl VirtualMachine {
};
for frame in frames {
let source_url = frame.source_url.to_utf8();
let file = bun_paths::resolve_path::relative(dir, source_url.slice());
let file = crate::ZigStackFrame::relative_source_url(dir, source_url.slice());
let func = frame.function_name.to_utf8();
if file.is_empty() && func.slice().is_empty() {
continue;
Expand Down
17 changes: 17 additions & 0 deletions src/jsc/ZigStackFrame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,23 @@ impl ZigStackFrame {
jsc_stack_frame_index: -1,
};

/// The frame's source as a report that lists files relative to `dir` (the JUnit
/// reporter, the GitHub Actions annotation) prints it.
///
/// Only an absolute path is made relative. A source URL that is not a path (a
/// `data:` or `blob:` URL, `node:fs`, the name from a `//# sourceURL=` comment)
/// is printed as-is, like [`SourceURLFormatter`] prints it. `relative` normalizes
/// its operands in fixed path buffers, so a source URL that is too long to be a
/// path is printed as-is too.
///
/// The relative form lives in `relative`'s thread-local buffer until the next call.
pub fn relative_source_url<'a>(dir: &[u8], source_url: &'a [u8]) -> &'a [u8] {
if !bun_paths::is_absolute(source_url) || source_url.len() >= bun_paths::MAX_PATH_BYTES {
return source_url;
}
bun_paths::resolve_path::relative(dir, source_url)
}

pub fn name_formatter(&self, enable_color: bool) -> NameFormatter {
NameFormatter {
function_name: self.function_name,
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/cli/test_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ impl JunitReporter {
let dir = FileSystem::instance().top_level_dir;
for frame in exception.stack.frames() {
let source_url = frame.source_url.to_utf8();
let file = resolve_path::relative(dir, source_url.slice());
let file = jsc::ZigStackFrame::relative_source_url(dir, source_url.slice());
let func = frame.function_name.to_utf8();
if file.is_empty() && func.slice().is_empty() {
continue;
Expand Down
64 changes: 63 additions & 1 deletion test/cli/test/bun-test.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { spawnSync } from "bun";
import { beforeAll, describe, expect, it, test } from "bun:test";
import { bunEnv, bunExe, isLinux, isWindows, tempDir, tempDirWithFiles, tmpdirSync } from "harness";
import { mkdirSync, rmSync, writeFileSync } from "node:fs";
import { dirname, join, resolve } from "node:path";
import { basename, dirname, join, resolve, sep } from "node:path";

describe("bun test", () => {
test("running a non-existent absolute file path is a 1 exit code", () => {
Expand Down Expand Up @@ -719,6 +719,68 @@ describe("bun test", () => {
});
expect(stderr).toMatch(/::error title=error: Test \"time out\" timed out after \d+ms::/);
});
test("should annotate an error thrown from a source whose URL is longer than a path buffer", () => {
// Longer than a path buffer on every platform (98302 bytes on Windows).
const padding = 100_000;
const dataUrlModule = 'export default function fromDataUrl() { throw new Error("boom"); }//';
const base64 = btoa(dataUrlModule + Buffer.alloc(padding, "x").toString());
const longPath = "/" + Buffer.alloc(padding, "y").toString();
const stderr = runTest({
input: `
import { test } from "bun:test";
test("data url", async () => {
const source = ${JSON.stringify(dataUrlModule)} + Buffer.alloc(${padding}, "x").toString();
const m = await import("data:text/javascript;base64," + btoa(source));
m.default();
});
test("long sourceURL", () => {
const sourceURL = "/" + Buffer.alloc(${padding}, "y").toString();
(0, eval)("(function fromLongPath() { throw new Error('boom'); })\\n//# sourceURL=" + sourceURL)();
});
`,
env: {
GITHUB_ACTIONS: "true",
},
expectExitCode: 1,
});
const annotations = stderr.split("\n").filter(line => line.startsWith("::error"));
expect(annotations).toHaveLength(2);
const [dataUrl, longSourceUrl] = annotations;
expect(dataUrl).toStartWith(`::error file=data%3Atext/javascript;base64%2C${base64},line=1,col=`);
expect(dataUrl).toContain(`%0A at fromDataUrl (data:text/javascript;base64,${base64}:1:`);
expect(longSourceUrl).toStartWith(`::error file=${longPath},line=1,col=`);
expect(longSourceUrl).toContain(`%0A at fromLongPath (${longPath}:1:`);
});
test("should make the annotation file relative to GITHUB_WORKSPACE only when it is a path", () => {
const cwd = createTest([
{
filename: "workspace.test.ts",
contents: `
import { test } from "bun:test";
test("in the test file", () => {
throw new Error("boom");
});
test("in a sourceURL that is not a path", () => {
(0, eval)("(function fromSourceUrl() { throw new Error('boom'); })\\n//# sourceURL=webpack://app/./src/x.ts")();
});
`,
},
]);
const stderr = runTest({
cwd,
env: {
GITHUB_ACTIONS: "true",
GITHUB_WORKSPACE: dirname(cwd),
},
expectExitCode: 1,
});
const annotations = stderr.split("\n").filter(line => line.startsWith("::error"));
expect(annotations).toHaveLength(2);
const [testFile, sourceUrl] = annotations;
expect(testFile).toStartWith(`::error file=${basename(cwd)}${sep}workspace.test.ts,line=4,col=`);
expect(sourceUrl).toStartWith("::error file=webpack%3A//app/./src/x.ts,line=1,col=");
expect(sourceUrl).toContain("%0A at fromSourceUrl (webpack://app/./src/x.ts:1:");
});
});
describe(".each", () => {
test("should run tests with test.each", () => {
Expand Down
26 changes: 26 additions & 0 deletions test/js/bun/test/stack.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,32 @@ test("throwing inside an error suppresses the error and prints the stack", async
expect(exitCode).toBe(1);
});

test("uncaught error thrown from a data: URL module longer than a path buffer is annotated for GitHub Actions", async () => {
// Longer than a path buffer on every platform (98302 bytes on Windows).
const padding = 100_000;
const dataUrlModule = 'export default function fromDataUrl() { throw new Error("boom"); }//';
const base64 = btoa(dataUrlModule + Buffer.alloc(padding, "x").toString());

await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`const source = ${JSON.stringify(dataUrlModule)} + Buffer.alloc(${padding}, "x").toString();
const m = await import("data:text/javascript;base64," + btoa(source));
m.default();`,
],
env: { ...bunEnv, GITHUB_ACTIONS: "true" },
stdout: "pipe",
stderr: "pipe",
});
const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);

const annotation = stderr.split("\n").find(line => line.startsWith("::error"));
expect(annotation).toStartWith(`::error file=data%3Atext/javascript;base64%2C${base64},line=1,col=`);
expect(annotation).toContain(`%0A at fromDataUrl (data:text/javascript;base64,${base64}:1:`);
expect(exitCode).toBe(1);
});

test("throwing inside an error suppresses the error and continues printing properties on the object", async () => {
$.throws(false);
$.env(bunEnv);
Expand Down
54 changes: 54 additions & 0 deletions test/js/junit-reporter/junit.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,60 @@ describe("junit reporter", () => {
expect(xmlContent).not.toMatch(/\[[\d;]*[A-HJKSTfm]/);
expect(exitCode).toBe(1);
});

it("prints a stack frame whose source is not a file path as-is in <failure>", async () => {
// Longer than a path buffer on every platform (98302 bytes on Windows).
const padding = 100_000;
const dataUrlModule = 'export default function fromDataUrl() { throw new Error("boom"); }//';
const dataUrl = "data:text/javascript;base64," + btoa(dataUrlModule + Buffer.alloc(padding, "x").toString());
const longPath = "/" + Buffer.alloc(padding, "y").toString();

await using tmpDir = tempDir("junit-source-url", {
"package.json": "{}",
"source-url.test.js": `
import { test } from "bun:test";
const thrower = (name, sourceURL) =>
(0, eval)("(function " + name + "() { throw new Error('boom'); })\\n//# sourceURL=" + sourceURL);
test("data url", async function dataUrlTest() {
const source = ${JSON.stringify(dataUrlModule)} + Buffer.alloc(${padding}, "x").toString();
const m = await import("data:text/javascript;base64," + btoa(source));
m.default();
});
test("sourceURL that is not a path", () => {
thrower("fromSourceUrl", "webpack://app/./src/x.ts")();
});
test("sourceURL longer than a path", () => {
thrower("fromLongPath", "/" + Buffer.alloc(${padding}, "y").toString())();
});
test("sourceURL that is a path", () => {
thrower("fromPath", import.meta.dir.replaceAll("\\\\", "/") + "/virtual/../generated.js")();
});
`,
});

const junitPath = join(tmpDir, "junit.xml");
await using proc = spawn([bunExe(), "test", "--reporter=junit", "--reporter-outfile", junitPath], {
cwd: tmpDir,
env: { ...bunEnv, BUN_DEBUG_QUIET_LOGS: "1" },
stdout: "pipe",
stderr: "pipe",
});
const [, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(exitCode).toBe(1);

const xmlContent = await file(junitPath).text();
const result = await new Promise((resolve, reject) => {
xml2js.parseString(xmlContent, { strict: true }, (err, r) => (err ? reject(err) : resolve(r)));
});
const [dataUrlCase, sourceUrlCase, longPathCase, pathCase] = result.testsuites.testsuite[0].testcase;

expect(dataUrlCase.failure[0]._).toContain(`at fromDataUrl (${dataUrl}:1:`);
// The frame in the test file itself is still relative to the cwd.
expect(dataUrlCase.failure[0]._).toContain("at dataUrlTest (source-url.test.js:");
expect(sourceUrlCase.failure[0]._).toContain("at fromSourceUrl (webpack://app/./src/x.ts:1:");
expect(longPathCase.failure[0]._).toContain(`at fromLongPath (${longPath}:1:`);
expect(pathCase.failure[0]._).toContain("at fromPath (generated.js:1:");
});
});

function filterJunitXmlOutput(xmlContent) {
Expand Down
Loading