Skip to content

Commit ffdfe27

Browse files
authored
fix(polyfill): add missing Bun API polyfills for npm distribution (#637)
## Summary - Add `Bun.file().size` and `.lastModified` polyfills — fixes DSN auto-detection silently failing on npm (size guard bypassed, cache invalidation broken) - Add `Bun.spawnSync()` polyfill — fixes `sentry init` crashing on npm when running git status checks - Add `{ PATH }` option support to `Bun.which()` polyfill — fixes custom PATH lookups being silently ignored in shell completion detection - Fix incorrect JSDoc in `oauth.ts` referencing wrong esbuild define key name ## Context Audit of build-time replacement issues (similar to #627) found no remaining `define` replacement bugs, but uncovered these silent failures in the Node.js polyfill layer where code works on Bun but silently breaks on the npm distribution.
1 parent e378841 commit ffdfe27

File tree

3 files changed

+325
-8
lines changed

3 files changed

+325
-8
lines changed

script/node-polyfills.ts

Lines changed: 59 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,22 @@ const bunSqlitePolyfill = { Database: NodeDatabasePolyfill };
98103
const BunPolyfill = {
99104
file(path: string) {
100105
return {
106+
/** File size in bytes (synchronous, like Bun.file().size). Returns 0 for non-existent files. */
107+
get size(): number {
108+
try {
109+
return statSync(path).size;
110+
} catch {
111+
return 0;
112+
}
113+
},
114+
/** Last-modified time in ms since epoch (like Bun.file().lastModified). Returns 0 for non-existent files. */
115+
get lastModified(): number {
116+
try {
117+
return statSync(path).mtimeMs;
118+
} catch {
119+
return 0;
120+
}
121+
},
101122
async exists(): Promise<boolean> {
102123
try {
103124
await access(path);
@@ -132,12 +153,22 @@ const BunPolyfill = {
132153
}
133154
},
134155

135-
which(command: string): string | null {
156+
which(command: string, opts?: { PATH?: string }): string | null {
136157
try {
137158
const isWindows = process.platform === "win32";
138159
const cmd = isWindows ? `where ${command}` : `which ${command}`;
160+
// If a custom PATH is provided, override it in the subprocess env.
161+
// Use !== undefined (not truthy) so empty-string PATH is respected.
162+
const env =
163+
opts?.PATH !== undefined
164+
? { ...process.env, PATH: opts.PATH }
165+
: undefined;
139166
return (
140-
execSync(cmd, { encoding: "utf-8", stdio: ["pipe", "pipe", "ignore"] })
167+
execSync(cmd, {
168+
encoding: "utf-8",
169+
stdio: ["pipe", "pipe", "ignore"],
170+
env,
171+
})
141172
.trim()
142173
.split("\n")[0] || null
143174
);
@@ -146,6 +177,31 @@ const BunPolyfill = {
146177
}
147178
},
148179

180+
/**
181+
* Synchronously spawn a subprocess. Matches Bun.spawnSync() used by
182+
* git.ts for pre-flight checks in `sentry init`.
183+
*/
184+
spawnSync(
185+
cmd: string[],
186+
opts?: {
187+
stdout?: "pipe" | "ignore" | "inherit";
188+
stderr?: "pipe" | "ignore" | "inherit";
189+
cwd?: string;
190+
}
191+
) {
192+
const [command, ...args] = cmd;
193+
const result = nodeSpawnSync(command, args, {
194+
stdio: ["ignore", opts?.stdout ?? "ignore", opts?.stderr ?? "ignore"],
195+
cwd: opts?.cwd,
196+
});
197+
return {
198+
success: result.status === 0,
199+
exitCode: result.status ?? 1,
200+
stdout: result.stdout,
201+
stderr: result.stderr,
202+
};
203+
},
204+
149205
spawn(
150206
cmd: string[],
151207
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: 265 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,258 @@ 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+
try {
330+
return statSync(path).size;
331+
} catch {
332+
return 0;
333+
}
334+
},
335+
get lastModified(): number {
336+
try {
337+
return statSync(path).mtimeMs;
338+
} catch {
339+
return 0;
340+
}
341+
},
342+
};
343+
}
344+
345+
describe("file polyfill size and lastModified", () => {
346+
test("size returns correct byte length", () => {
347+
const tmpDir = mkdtempSync(join(tmpdir(), "polyfill-file-"));
348+
const filePath = join(tmpDir, "test.txt");
349+
try {
350+
writeFileSync(filePath, "hello world"); // 11 bytes
351+
const pf = polyfillFile(filePath);
352+
expect(pf.size).toBe(11);
353+
354+
// Verify consistency with Bun.file().size
355+
expect(pf.size).toBe(Bun.file(filePath).size);
356+
} finally {
357+
rmSync(tmpDir, { recursive: true });
358+
}
359+
});
360+
361+
test("size returns 0 for empty file", () => {
362+
const tmpDir = mkdtempSync(join(tmpdir(), "polyfill-file-"));
363+
const filePath = join(tmpDir, "empty.txt");
364+
try {
365+
writeFileSync(filePath, "");
366+
const pf = polyfillFile(filePath);
367+
expect(pf.size).toBe(0);
368+
} finally {
369+
rmSync(tmpDir, { recursive: true });
370+
}
371+
});
372+
373+
test("lastModified returns a recent timestamp in milliseconds", () => {
374+
const before = Date.now();
375+
const tmpDir = mkdtempSync(join(tmpdir(), "polyfill-file-"));
376+
const filePath = join(tmpDir, "test.txt");
377+
try {
378+
writeFileSync(filePath, "data");
379+
const after = Date.now();
380+
const pf = polyfillFile(filePath);
381+
382+
// Should be between before and after (with 1s tolerance for slow CI)
383+
expect(pf.lastModified).toBeGreaterThanOrEqual(before - 1000);
384+
expect(pf.lastModified).toBeLessThanOrEqual(after + 1000);
385+
} finally {
386+
rmSync(tmpDir, { recursive: true });
387+
}
388+
});
389+
390+
test("lastModified is consistent with Bun.file().lastModified", () => {
391+
const tmpDir = mkdtempSync(join(tmpdir(), "polyfill-file-"));
392+
const filePath = join(tmpDir, "test.txt");
393+
try {
394+
writeFileSync(filePath, "data");
395+
const pf = polyfillFile(filePath);
396+
const bunMtime = Bun.file(filePath).lastModified;
397+
// Both should be within 1ms of each other
398+
expect(Math.abs(pf.lastModified - bunMtime)).toBeLessThanOrEqual(1);
399+
} finally {
400+
rmSync(tmpDir, { recursive: true });
401+
}
402+
});
403+
404+
test("size returns 0 for non-existent file (matches Bun behavior)", () => {
405+
const pf = polyfillFile("/tmp/__nonexistent_file_polyfill_test__");
406+
expect(pf.size).toBe(0);
407+
expect(pf.size).toBe(
408+
Bun.file("/tmp/__nonexistent_file_polyfill_test__").size
409+
);
410+
});
411+
412+
test("lastModified returns 0 for non-existent file", () => {
413+
const pf = polyfillFile("/tmp/__nonexistent_file_polyfill_test__");
414+
expect(pf.lastModified).toBe(0);
415+
});
416+
});
417+
418+
/**
419+
* Reproduces the exact which() polyfill logic from script/node-polyfills.ts
420+
* with PATH option support.
421+
* Kept in sync manually — if the polyfill changes, update this too.
422+
*/
423+
function polyfillWhich(
424+
command: string,
425+
opts?: { PATH?: string }
426+
): string | null {
427+
try {
428+
const isWindows = process.platform === "win32";
429+
const cmd = isWindows ? `where ${command}` : `which ${command}`;
430+
const env =
431+
opts?.PATH !== undefined
432+
? { ...process.env, PATH: opts.PATH }
433+
: undefined;
434+
return (
435+
execSync(cmd, {
436+
encoding: "utf-8",
437+
stdio: ["pipe", "pipe", "ignore"],
438+
env,
439+
})
440+
.trim()
441+
.split("\n")[0] || null
442+
);
443+
} catch {
444+
return null;
445+
}
446+
}
447+
448+
describe("which polyfill with PATH option", () => {
449+
test("finds command on default PATH", () => {
450+
const result = polyfillWhich("node");
451+
expect(result).not.toBeNull();
452+
// Should match Bun.which
453+
expect(result).toBe(Bun.which("node"));
454+
});
455+
456+
test("returns null for nonexistent command", () => {
457+
const result = polyfillWhich("__nonexistent_command_polyfill_test__");
458+
expect(result).toBeNull();
459+
});
460+
461+
test("returns null when PATH is empty string (matches Bun behavior)", () => {
462+
// Empty-string PATH should be respected, not ignored as falsy
463+
const result = polyfillWhich("node", { PATH: "" });
464+
expect(result).toBeNull();
465+
expect(result).toBe(Bun.which("node", { PATH: "" }));
466+
});
467+
468+
test("returns null when PATH excludes command directory", () => {
469+
// Use a valid but irrelevant directory so which finds nothing
470+
const result = polyfillWhich("__nonexistent_command__", { PATH: "/tmp" });
471+
expect(result).toBeNull();
472+
});
473+
474+
test("passes env with custom PATH to execSync", () => {
475+
// Verify the polyfill constructs the env correctly when PATH is provided
476+
const withPath = polyfillWhich("node", { PATH: process.env.PATH });
477+
const withoutPath = polyfillWhich("node");
478+
// Both should find node when given the same PATH
479+
expect(withPath).toBe(withoutPath);
480+
});
481+
482+
test("PATH option changes search scope", () => {
483+
// A nonexistent command should not be found regardless of PATH
484+
const result = polyfillWhich("__does_not_exist_anywhere__", {
485+
PATH: "/usr/bin:/usr/local/bin",
486+
});
487+
expect(result).toBeNull();
488+
});
489+
});

0 commit comments

Comments
 (0)