Skip to content

Commit e3d709a

Browse files
committed
fix(polyfill): add missing Bun API polyfills for npm distribution
The Node.js polyfill layer was missing several Bun APIs used by source code reachable from normal npm usage, causing silent failures: - Bun.file().size and .lastModified were undefined, silently breaking DSN auto-detection (size guard bypassed, cache invalidation broken) - Bun.spawnSync() was missing, crashing `sentry init` git checks - Bun.which() ignored the { PATH } options parameter, breaking custom PATH lookups in shell completion detection Also fixes an incorrect JSDoc in oauth.ts that referenced the wrong esbuild define key name (SENTRY_CLIENT_ID vs SENTRY_CLIENT_ID_BUILD).
1 parent adbd907 commit e3d709a

File tree

3 files changed

+287
-8
lines changed

3 files changed

+287
-8
lines changed

script/node-polyfills.ts

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
/**
22
* Node.js polyfills for Bun APIs. Injected at bundle time via esbuild.
33
*/
4-
import { execSync, spawn as nodeSpawn } from "node:child_process";
4+
import {
5+
execSync,
6+
spawn as nodeSpawn,
7+
spawnSync as nodeSpawnSync,
8+
} from "node:child_process";
9+
import { statSync } from "node:fs";
510
import { access, readFile, writeFile } from "node:fs/promises";
611
// node:sqlite is imported lazily inside NodeDatabasePolyfill to avoid
712
// crashing on Node.js versions without node:sqlite support when the
@@ -98,6 +103,14 @@ const bunSqlitePolyfill = { Database: NodeDatabasePolyfill };
98103
const BunPolyfill = {
99104
file(path: string) {
100105
return {
106+
/** File size in bytes (synchronous, like Bun.file().size). */
107+
get size(): number {
108+
return statSync(path).size;
109+
},
110+
/** Last-modified time in ms since epoch (like Bun.file().lastModified). */
111+
get lastModified(): number {
112+
return statSync(path).mtimeMs;
113+
},
101114
async exists(): Promise<boolean> {
102115
try {
103116
await access(path);
@@ -132,12 +145,18 @@ const BunPolyfill = {
132145
}
133146
},
134147

135-
which(command: string): string | null {
148+
which(command: string, opts?: { PATH?: string }): string | null {
136149
try {
137150
const isWindows = process.platform === "win32";
138151
const cmd = isWindows ? `where ${command}` : `which ${command}`;
152+
// If a custom PATH is provided, override it in the subprocess env
153+
const env = opts?.PATH ? { ...process.env, PATH: opts.PATH } : undefined;
139154
return (
140-
execSync(cmd, { encoding: "utf-8", stdio: ["pipe", "pipe", "ignore"] })
155+
execSync(cmd, {
156+
encoding: "utf-8",
157+
stdio: ["pipe", "pipe", "ignore"],
158+
env,
159+
})
141160
.trim()
142161
.split("\n")[0] || null
143162
);
@@ -146,6 +165,31 @@ const BunPolyfill = {
146165
}
147166
},
148167

168+
/**
169+
* Synchronously spawn a subprocess. Matches Bun.spawnSync() used by
170+
* git.ts for pre-flight checks in `sentry init`.
171+
*/
172+
spawnSync(
173+
cmd: string[],
174+
opts?: {
175+
stdout?: "pipe" | "ignore" | "inherit";
176+
stderr?: "pipe" | "ignore" | "inherit";
177+
cwd?: string;
178+
}
179+
) {
180+
const [command, ...args] = cmd;
181+
const result = nodeSpawnSync(command, args, {
182+
stdio: ["ignore", opts?.stdout ?? "ignore", opts?.stderr ?? "ignore"],
183+
cwd: opts?.cwd,
184+
});
185+
return {
186+
success: result.status === 0,
187+
exitCode: result.status ?? 1,
188+
stdout: result.stdout,
189+
stderr: result.stderr,
190+
};
191+
},
192+
149193
spawn(
150194
cmd: string[],
151195
opts?: {

src/lib/oauth.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ function getSentryUrl(): string {
3131
/**
3232
* OAuth client ID
3333
*
34-
* Build-time: Injected via Bun.build({ define: { SENTRY_CLIENT_ID: "..." } })
34+
* Build-time: Injected via esbuild define: { SENTRY_CLIENT_ID_BUILD: "..." }
3535
* Runtime: Can be overridden via SENTRY_CLIENT_ID env var (for self-hosted)
3636
*
3737
* Read at call time (not module load time) so tests can override SENTRY_CLIENT_ID

test/script/node-polyfills.test.ts

Lines changed: 239 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
/**
2-
* Node.js Polyfill Tests — Bun.spawn and Bun.Glob
2+
* Node.js Polyfill Tests — Bun.spawn, Bun.spawnSync, Bun.which, Bun.file, and Bun.Glob
33
*
4-
* Tests the spawn and glob logic used by the Node.js polyfill in
5-
* script/node-polyfills.ts.
4+
* Tests the logic used by the Node.js polyfill in script/node-polyfills.ts.
65
*
76
* We can't import the polyfill directly (it overwrites globalThis.Bun and has
87
* side effects), so we reproduce the exact implementation and verify its
@@ -15,7 +14,14 @@
1514
*/
1615

1716
import { describe, expect, test } from "bun:test";
18-
import { spawn as nodeSpawn } from "node:child_process";
17+
import {
18+
execSync,
19+
spawn as nodeSpawn,
20+
spawnSync as nodeSpawnSync,
21+
} from "node:child_process";
22+
import { mkdtempSync, rmSync, statSync, writeFileSync } from "node:fs";
23+
import { tmpdir } from "node:os";
24+
import { join } from "node:path";
1925

2026
/**
2127
* Reproduces the exact spawn logic from script/node-polyfills.ts.
@@ -226,3 +232,232 @@ describe("Glob polyfill match()", () => {
226232
}
227233
});
228234
});
235+
236+
/**
237+
* Reproduces the exact spawnSync logic from script/node-polyfills.ts.
238+
* Kept in sync manually — if the polyfill changes, update this too.
239+
*/
240+
function polyfillSpawnSync(
241+
cmd: string[],
242+
opts?: {
243+
stdout?: "pipe" | "ignore" | "inherit";
244+
stderr?: "pipe" | "ignore" | "inherit";
245+
cwd?: string;
246+
}
247+
) {
248+
const [command, ...args] = cmd;
249+
const result = nodeSpawnSync(command, args, {
250+
stdio: ["ignore", opts?.stdout ?? "ignore", opts?.stderr ?? "ignore"],
251+
cwd: opts?.cwd,
252+
});
253+
return {
254+
success: result.status === 0,
255+
exitCode: result.status ?? 1,
256+
stdout: result.stdout,
257+
stderr: result.stderr,
258+
};
259+
}
260+
261+
describe("spawnSync polyfill", () => {
262+
test("returns success: true for exit code 0", () => {
263+
const result = polyfillSpawnSync(["node", "-e", "process.exit(0)"]);
264+
expect(result.success).toBe(true);
265+
expect(result.exitCode).toBe(0);
266+
});
267+
268+
test("returns success: false for non-zero exit code", () => {
269+
const result = polyfillSpawnSync(["node", "-e", "process.exit(42)"]);
270+
expect(result.success).toBe(false);
271+
expect(result.exitCode).toBe(42);
272+
});
273+
274+
test("captures stdout when stdout: pipe", () => {
275+
const result = polyfillSpawnSync(
276+
["node", "-e", 'process.stdout.write("hello")'],
277+
{ stdout: "pipe" }
278+
);
279+
expect(result.success).toBe(true);
280+
expect(result.stdout.toString()).toBe("hello");
281+
});
282+
283+
test("respects cwd option", () => {
284+
const tmpDir = mkdtempSync(join(tmpdir(), "polyfill-cwd-"));
285+
try {
286+
const result = polyfillSpawnSync(
287+
["node", "-e", "process.stdout.write(process.cwd())"],
288+
{
289+
stdout: "pipe",
290+
cwd: tmpDir,
291+
}
292+
);
293+
expect(result.success).toBe(true);
294+
// Resolve symlinks (macOS /tmp → /private/tmp)
295+
const expected = Bun.which("realpath")
296+
? execSync(`realpath "${tmpDir}"`, { encoding: "utf-8" }).trim()
297+
: tmpDir;
298+
const actual = Bun.which("realpath")
299+
? execSync(`realpath "${result.stdout.toString()}"`, {
300+
encoding: "utf-8",
301+
}).trim()
302+
: result.stdout.toString();
303+
expect(actual).toBe(expected);
304+
} finally {
305+
rmSync(tmpDir, { recursive: true });
306+
}
307+
});
308+
309+
test("is consistent with Bun.spawnSync for git --version", () => {
310+
const polyfill = polyfillSpawnSync(["git", "--version"], {
311+
stdout: "pipe",
312+
});
313+
const bun = Bun.spawnSync(["git", "--version"], { stdout: "pipe" });
314+
expect(polyfill.success).toBe(bun.success);
315+
expect(polyfill.exitCode).toBe(bun.exitCode);
316+
// Both should output something starting with "git version"
317+
expect(polyfill.stdout.toString()).toStartWith("git version");
318+
expect(bun.stdout.toString()).toStartWith("git version");
319+
});
320+
});
321+
322+
/**
323+
* Reproduces the exact file() polyfill logic from script/node-polyfills.ts.
324+
* Kept in sync manually — if the polyfill changes, update this too.
325+
*/
326+
function polyfillFile(path: string) {
327+
return {
328+
get size(): number {
329+
return statSync(path).size;
330+
},
331+
get lastModified(): number {
332+
return statSync(path).mtimeMs;
333+
},
334+
};
335+
}
336+
337+
describe("file polyfill size and lastModified", () => {
338+
test("size returns correct byte length", () => {
339+
const tmpDir = mkdtempSync(join(tmpdir(), "polyfill-file-"));
340+
const filePath = join(tmpDir, "test.txt");
341+
try {
342+
writeFileSync(filePath, "hello world"); // 11 bytes
343+
const pf = polyfillFile(filePath);
344+
expect(pf.size).toBe(11);
345+
346+
// Verify consistency with Bun.file().size
347+
expect(pf.size).toBe(Bun.file(filePath).size);
348+
} finally {
349+
rmSync(tmpDir, { recursive: true });
350+
}
351+
});
352+
353+
test("size returns 0 for empty file", () => {
354+
const tmpDir = mkdtempSync(join(tmpdir(), "polyfill-file-"));
355+
const filePath = join(tmpDir, "empty.txt");
356+
try {
357+
writeFileSync(filePath, "");
358+
const pf = polyfillFile(filePath);
359+
expect(pf.size).toBe(0);
360+
} finally {
361+
rmSync(tmpDir, { recursive: true });
362+
}
363+
});
364+
365+
test("lastModified returns a recent timestamp in milliseconds", () => {
366+
const before = Date.now();
367+
const tmpDir = mkdtempSync(join(tmpdir(), "polyfill-file-"));
368+
const filePath = join(tmpDir, "test.txt");
369+
try {
370+
writeFileSync(filePath, "data");
371+
const after = Date.now();
372+
const pf = polyfillFile(filePath);
373+
374+
// Should be between before and after (with 1s tolerance for slow CI)
375+
expect(pf.lastModified).toBeGreaterThanOrEqual(before - 1000);
376+
expect(pf.lastModified).toBeLessThanOrEqual(after + 1000);
377+
} finally {
378+
rmSync(tmpDir, { recursive: true });
379+
}
380+
});
381+
382+
test("lastModified is consistent with Bun.file().lastModified", () => {
383+
const tmpDir = mkdtempSync(join(tmpdir(), "polyfill-file-"));
384+
const filePath = join(tmpDir, "test.txt");
385+
try {
386+
writeFileSync(filePath, "data");
387+
const pf = polyfillFile(filePath);
388+
const bunMtime = Bun.file(filePath).lastModified;
389+
// Both should be within 1ms of each other
390+
expect(Math.abs(pf.lastModified - bunMtime)).toBeLessThanOrEqual(1);
391+
} finally {
392+
rmSync(tmpDir, { recursive: true });
393+
}
394+
});
395+
396+
test("size throws for non-existent file", () => {
397+
const pf = polyfillFile("/tmp/__nonexistent_file_polyfill_test__");
398+
expect(() => pf.size).toThrow();
399+
});
400+
});
401+
402+
/**
403+
* Reproduces the exact which() polyfill logic from script/node-polyfills.ts
404+
* with PATH option support.
405+
* Kept in sync manually — if the polyfill changes, update this too.
406+
*/
407+
function polyfillWhich(
408+
command: string,
409+
opts?: { PATH?: string }
410+
): string | null {
411+
try {
412+
const isWindows = process.platform === "win32";
413+
const cmd = isWindows ? `where ${command}` : `which ${command}`;
414+
const env = opts?.PATH ? { ...process.env, PATH: opts.PATH } : undefined;
415+
return (
416+
execSync(cmd, {
417+
encoding: "utf-8",
418+
stdio: ["pipe", "pipe", "ignore"],
419+
env,
420+
})
421+
.trim()
422+
.split("\n")[0] || null
423+
);
424+
} catch {
425+
return null;
426+
}
427+
}
428+
429+
describe("which polyfill with PATH option", () => {
430+
test("finds command on default PATH", () => {
431+
const result = polyfillWhich("node");
432+
expect(result).not.toBeNull();
433+
// Should match Bun.which
434+
expect(result).toBe(Bun.which("node"));
435+
});
436+
437+
test("returns null for nonexistent command", () => {
438+
const result = polyfillWhich("__nonexistent_command_polyfill_test__");
439+
expect(result).toBeNull();
440+
});
441+
442+
test("returns null when PATH excludes command directory", () => {
443+
// Use a valid but irrelevant directory so which finds nothing
444+
const result = polyfillWhich("__nonexistent_command__", { PATH: "/tmp" });
445+
expect(result).toBeNull();
446+
});
447+
448+
test("passes env with custom PATH to execSync", () => {
449+
// Verify the polyfill constructs the env correctly when PATH is provided
450+
const withPath = polyfillWhich("node", { PATH: process.env.PATH });
451+
const withoutPath = polyfillWhich("node");
452+
// Both should find node when given the same PATH
453+
expect(withPath).toBe(withoutPath);
454+
});
455+
456+
test("PATH option changes search scope", () => {
457+
// A nonexistent command should not be found regardless of PATH
458+
const result = polyfillWhich("__does_not_exist_anywhere__", {
459+
PATH: "/usr/bin:/usr/local/bin",
460+
});
461+
expect(result).toBeNull();
462+
});
463+
});

0 commit comments

Comments
 (0)