Skip to content

Commit 805804e

Browse files
authored
fix(polyfill): add exited promise and stdin to Bun.spawn Node.js polyfill (#248)
## Summary Fixes CLI-68: `UpgradeError: Setup failed with exit code undefined` The `Bun.spawn()` Node.js polyfill (used in the npm bundle) only returned `{ unref() }` — it was designed for fire-and-forget background processes. The upgrade command and clipboard module need to **wait** on the spawned process via `await proc.exited`, which resolved to `undefined` and always threw. ### Changes **`script/node-polyfills.ts`** — Enhanced spawn polyfill: - `exited` promise resolving with the numeric exit code - `stdin` passthrough from the underlying child process - `env` option support for passing environment variables - `"inherit"` stdio type support - Removed unnecessary `detached: true` default **`test/script/node-polyfills.test.ts`** — 7 new tests covering exit codes, stdin piping, env passthrough, inherit stdio, error paths, and unref. ### Affected code paths (all fixed by the polyfill change) | Location | Feature | Prior behavior | |----------|---------|----------------| | `upgrade.ts:113-118` | `runSetupOnNewBinary` | Always threw on npm installs | | `clipboard.ts:55-65` | `copyToClipboard` | Silently failed (try/catch) | Fixes CLI-68
1 parent 405da88 commit 805804e

File tree

2 files changed

+153
-8
lines changed

2 files changed

+153
-8
lines changed

script/node-polyfills.ts

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -135,19 +135,34 @@ const BunPolyfill = {
135135

136136
spawn(
137137
cmd: string[],
138-
opts?: { stdout?: "pipe" | "ignore"; stderr?: "pipe" | "ignore" }
138+
opts?: {
139+
stdin?: "pipe" | "ignore" | "inherit";
140+
stdout?: "pipe" | "ignore" | "inherit";
141+
stderr?: "pipe" | "ignore" | "inherit";
142+
env?: Record<string, string | undefined>;
143+
}
139144
) {
140145
const [command, ...args] = cmd;
141-
const stdio: ("pipe" | "ignore")[] = [
142-
"ignore", // stdin
143-
opts?.stdout ?? "ignore",
144-
opts?.stderr ?? "ignore",
145-
];
146146
const proc = nodeSpawn(command, args, {
147-
detached: true,
148-
stdio,
147+
stdio: [
148+
opts?.stdin ?? "ignore",
149+
opts?.stdout ?? "ignore",
150+
opts?.stderr ?? "ignore",
151+
],
152+
env: opts?.env,
153+
});
154+
155+
// Promise that resolves with the exit code when the process exits.
156+
// Bun's proc.exited resolves to the numeric exit code; we match that
157+
// contract, falling back to 1 on signal-only termination.
158+
const exited = new Promise<number>((resolve) => {
159+
proc.on("close", (code) => resolve(code ?? 1));
160+
proc.on("error", () => resolve(1));
149161
});
162+
150163
return {
164+
stdin: proc.stdin,
165+
exited,
151166
unref() {
152167
proc.unref();
153168
},

test/script/node-polyfills.test.ts

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
/**
2+
* Node.js Polyfill Tests — Bun.spawn
3+
*
4+
* Tests the spawn logic used by the Node.js polyfill in script/node-polyfills.ts.
5+
*
6+
* We can't import the polyfill directly (it overwrites globalThis.Bun and has
7+
* side effects), so we reproduce the exact spawn implementation and verify its
8+
* contract: exited promise, stdin piping, env passthrough, and inherit stdio.
9+
*
10+
* Fixes CLI-68: the original polyfill returned no `exited` property, causing
11+
* `await proc.exited` to resolve to `undefined` and the upgrade command to
12+
* throw "Setup failed with exit code undefined".
13+
*/
14+
15+
import { describe, expect, test } from "bun:test";
16+
import { spawn as nodeSpawn } from "node:child_process";
17+
18+
/**
19+
* Reproduces the exact spawn logic from script/node-polyfills.ts.
20+
* Kept in sync manually — if the polyfill changes, update this too.
21+
*/
22+
function polyfillSpawn(
23+
cmd: string[],
24+
opts?: {
25+
stdin?: "pipe" | "ignore" | "inherit";
26+
stdout?: "pipe" | "ignore" | "inherit";
27+
stderr?: "pipe" | "ignore" | "inherit";
28+
env?: Record<string, string | undefined>;
29+
}
30+
) {
31+
const [command, ...args] = cmd;
32+
const proc = nodeSpawn(command, args, {
33+
stdio: [
34+
opts?.stdin ?? "ignore",
35+
opts?.stdout ?? "ignore",
36+
opts?.stderr ?? "ignore",
37+
],
38+
env: opts?.env,
39+
});
40+
41+
const exited = new Promise<number>((resolve) => {
42+
proc.on("close", (code) => resolve(code ?? 1));
43+
proc.on("error", () => resolve(1));
44+
});
45+
46+
return {
47+
stdin: proc.stdin,
48+
exited,
49+
unref() {
50+
proc.unref();
51+
},
52+
};
53+
}
54+
55+
describe("spawn polyfill", () => {
56+
test("exited resolves with exit code 0 for successful command", async () => {
57+
const proc = polyfillSpawn(["node", "-e", "process.exit(0)"]);
58+
const exitCode = await proc.exited;
59+
expect(exitCode).toBe(0);
60+
});
61+
62+
test("exited resolves with non-zero exit code for failed command", async () => {
63+
const proc = polyfillSpawn(["node", "-e", "process.exit(42)"]);
64+
const exitCode = await proc.exited;
65+
expect(exitCode).toBe(42);
66+
});
67+
68+
test("stdin is writable when stdin: pipe", async () => {
69+
// Pipe text through cat, verify it exits cleanly
70+
const proc = polyfillSpawn(
71+
[
72+
"node",
73+
"-e",
74+
"process.stdin.resume(); process.stdin.on('end', () => process.exit(0));",
75+
],
76+
{
77+
stdin: "pipe",
78+
}
79+
);
80+
81+
proc.stdin!.write("hello");
82+
proc.stdin!.end();
83+
84+
const exitCode = await proc.exited;
85+
expect(exitCode).toBe(0);
86+
});
87+
88+
test("env is passed to child process", async () => {
89+
const proc = polyfillSpawn(
90+
[
91+
"node",
92+
"-e",
93+
"process.exit(process.env.POLYFILL_TEST === 'works' ? 0 : 1)",
94+
],
95+
{
96+
env: { ...process.env, POLYFILL_TEST: "works" },
97+
}
98+
);
99+
100+
const exitCode = await proc.exited;
101+
expect(exitCode).toBe(0);
102+
});
103+
104+
test("inherit stdio does not throw", async () => {
105+
const proc = polyfillSpawn(["node", "-e", "process.exit(0)"], {
106+
stdout: "inherit",
107+
stderr: "inherit",
108+
});
109+
110+
const exitCode = await proc.exited;
111+
expect(exitCode).toBe(0);
112+
});
113+
114+
test("exited resolves to 1 for non-existent command", async () => {
115+
const proc = polyfillSpawn(["__nonexistent_command_polyfill_test__"]);
116+
const exitCode = await proc.exited;
117+
// error event fires → resolves to 1
118+
expect(exitCode).toBe(1);
119+
});
120+
121+
test("unref is callable", () => {
122+
const proc = polyfillSpawn(["node", "-e", "setTimeout(() => {}, 5000)"], {
123+
stdout: "ignore",
124+
stderr: "ignore",
125+
});
126+
127+
// Should not throw
128+
expect(() => proc.unref()).not.toThrow();
129+
});
130+
});

0 commit comments

Comments
 (0)