diff --git a/browse/src/bun-polyfill.cjs b/browse/src/bun-polyfill.cjs index ce95b6dce..275d92a7e 100644 --- a/browse/src/bun-polyfill.cjs +++ b/browse/src/bun-polyfill.cjs @@ -12,7 +12,6 @@ const http = require('http'); const { spawnSync, spawn } = require('child_process'); -const { Readable } = require('stream'); globalThis.Bun = { serve(options) { @@ -94,34 +93,70 @@ globalThis.Bun = { cwd: options.cwd, }); + // Drain stdout/stderr eagerly into in-memory buffers. Bun's spawn buffers + // these for the consumer; Node's Readables are pull-based, so if the caller + // awaits `proc.exited` before reading, anything past the OS pipe buffer + // (~16-64 KB) back-pressures the child until it blocks in write() and + // `exit` never fires. Eager draining keeps the pipes flowing regardless + // of read order; replay below is via fresh Web ReadableStreams. + const drain = (stream) => { + if (!stream) return { done: Promise.resolve(), chunks: [] }; + const chunks = []; + const done = new Promise((resolve) => { + stream.on('data', (chunk) => chunks.push(chunk)); + stream.once('end', resolve); + stream.once('error', resolve); // exit event carries the real status + }); + return { done, chunks }; + }; + const stdoutDrain = drain(proc.stdout); + const stderrDrain = drain(proc.stderr); + // Bun's spawn exposes `proc.exited` as a Promise resolving to the exit // code; several call sites — DPAPI decryption, isBrowserRunning, // browser-skill-commands — `await proc.exited` directly or via // Promise.race with a timeout. Without this, those awaits resolve to // `undefined` immediately and the operation looks like a silent failure. + // Resolve only after both pipes have finished draining so consumers that + // read stdout AFTER awaiting exit see the full output, not a partial buffer. const exited = new Promise((resolveExited) => { + let exitStatus; proc.once('exit', (code, signal) => { // Match Bun: exit code on normal exit; 128 + signal number on signal; // 0 if neither was reported. - if (code !== null) resolveExited(code); - else if (signal) resolveExited(128 + (require('os').constants.signals[signal] || 0)); - else resolveExited(0); + if (code !== null) exitStatus = code; + else if (signal) exitStatus = 128 + (require('os').constants.signals[signal] || 0); + else exitStatus = 0; }); - proc.once('error', () => resolveExited(1)); + proc.once('error', () => { + if (exitStatus === undefined) exitStatus = 1; + }); + Promise.all([ + new Promise((r) => proc.once('exit', r)), + stdoutDrain.done, + stderrDrain.done, + ]).then(() => resolveExited(exitStatus !== undefined ? exitStatus : 0)); }); - // Bun gives consumers a Web ReadableStream so `new Response(proc.stdout)` - // works regardless of read order. With Node's Readable, the stream auto- - // drains once the child exits, so `await proc.exited` followed by - // `new Response(proc.stdout).text()` throws "body disturbed or locked". - // Readable.toWeb hands the consumer a fresh ReadableStream that buffers - // until it's read. Falls back to the raw Node stream on Node < 18. - const toWeb = (s) => (s && typeof Readable.toWeb === 'function' ? Readable.toWeb(s) : s); + // Replay buffered output as a fresh Web ReadableStream. `start()` awaits + // the drain before enqueueing so `new Response(proc.stdout).text()` yields + // the complete output regardless of whether the consumer reads before or + // after awaiting `proc.exited`. Stream is single-shot (locked after one + // read), matching Bun's behavior. + const replay = (d) => new ReadableStream({ + async start(controller) { + await d.done; + for (const chunk of d.chunks) { + controller.enqueue(chunk instanceof Uint8Array ? chunk : new Uint8Array(chunk)); + } + controller.close(); + }, + }); return { pid: proc.pid, - stdout: toWeb(proc.stdout), - stderr: toWeb(proc.stderr), + stdout: replay(stdoutDrain), + stderr: replay(stderrDrain), stdin: proc.stdin, exited, unref() { proc.unref(); }, diff --git a/browse/test/bun-polyfill.test.ts b/browse/test/bun-polyfill.test.ts index 57b2981cb..c9309208b 100644 --- a/browse/test/bun-polyfill.test.ts +++ b/browse/test/bun-polyfill.test.ts @@ -97,6 +97,31 @@ describe('bun-polyfill', () => { expect(result.stdout.toString().trim()).toBe('ready:0'); }); + // Regression for the pipe-blocking case: if the child writes more than the + // OS pipe buffer (~16-64 KB) and the polyfill doesn't drain eagerly, the + // child blocks in write() and `exit` never fires. 1 MB is well past every + // OS pipe buffer size. Pre-fix this test hangs forever; post-fix it returns + // in <500ms. Bun's default per-test timeout is 5s — generous here. + test('Bun.spawn drains large stdout so proc.exited still resolves', async () => { + const result = Bun.spawnSync(['node', '-e', ` + require('${polyfillPath}'); + (async () => { + const ONE_MB = 1024 * 1024; + const p = Bun.spawn( + ['node', '-e', 'process.stdout.write("x".repeat(' + ONE_MB + ')); process.exit(0)'], + { stdio: ['ignore', 'pipe', 'ignore'] } + ); + const code = await Promise.race([ + p.exited, + new Promise((_, r) => setTimeout(() => r(new Error('timeout')), 10000)) + ]).catch(e => 'TIMEOUT'); + const out = await new Response(p.stdout).text(); + console.log(out.length + ':' + code); + })().catch((e) => { console.log('THREW:' + e.message); }); + `], { stdout: 'pipe', stderr: 'pipe' }); + expect(result.stdout.toString().trim()).toBe('1048576:0'); + }, 15000); + test('Bun.serve creates an HTTP server that responds', async () => { const result = Bun.spawnSync(['node', '-e', ` require('${polyfillPath}');