diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 7f58442e3754..8268ae4320ed 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -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=", @@ -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; diff --git a/src/jsc/ZigStackFrame.rs b/src/jsc/ZigStackFrame.rs index 4de668b7810d..e77d7bda1004 100644 --- a/src/jsc/ZigStackFrame.rs +++ b/src/jsc/ZigStackFrame.rs @@ -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, diff --git a/src/runtime/cli/test_command.rs b/src/runtime/cli/test_command.rs index afabe9d4128e..97e471a8cf5b 100644 --- a/src/runtime/cli/test_command.rs +++ b/src/runtime/cli/test_command.rs @@ -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; diff --git a/test/cli/test/bun-test.test.ts b/test/cli/test/bun-test.test.ts index 566279d3e877..21ec163324af 100644 --- a/test/cli/test/bun-test.test.ts +++ b/test/cli/test/bun-test.test.ts @@ -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", () => { @@ -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", () => { diff --git a/test/js/bun/test/stack.test.ts b/test/js/bun/test/stack.test.ts index 63b28630a3f6..5445e109bf8e 100644 --- a/test/js/bun/test/stack.test.ts +++ b/test/js/bun/test/stack.test.ts @@ -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); diff --git a/test/js/junit-reporter/junit.test.js b/test/js/junit-reporter/junit.test.js index d5a2ebe87d8b..5cf40e2d295d 100644 --- a/test/js/junit-reporter/junit.test.js +++ b/test/js/junit-reporter/junit.test.js @@ -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 ", 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) {