Skip to content

Commit f7c7d1d

Browse files
committed
fix(ipfs): listen on 'close' so stderr drains before _spawnAsync rejects
Pulled the Promise wiring out of _spawnAsync into an exported _gatherChildOutput so the race is unit-testable. Listen on 'close' (not 'exit'): Node guarantees 'close' fires after all stdio streams have closed, so every stderr 'data' event has been delivered by the time we build the rejection Error. Why this matters: a fast-exiting child can deliver its exit signal before its stderr drains. With the old 'exit' listener, _spawnAsync rejected with errorMessage="". In startKuboNode the empty message defeated the `error.message.includes("ipfs configuration file already exists!")` suppression, turned a benign already-initialised repo into "Failed to call ipfs init" — and because startKuboNode wraps an async executor in `new Promise(...)`, that throw became an unhandledRejection instead of propagating, hanging keepKuboUp() and letting the daemon exit silently with code 0. That's the macos-latest CI failure on PR #47. The regression test in test/kubo/gather-child-output.test.ts feeds a fake EventEmitter the macOS event ordering (exit → data → close) and asserts the rejection captures the stderr text. Cross-platform deterministic, fails on the old 'exit' listener, passes on 'close'.
1 parent 5094658 commit f7c7d1d

2 files changed

Lines changed: 61 additions & 14 deletions

File tree

src/ipfs/startIpfs.ts

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -75,32 +75,38 @@ export async function mergeCliDefaultsIntoIpfsConfig(log: any, ipfsConfigPath: s
7575
// use this custom function instead of spawnSync for better logging
7676
// also spawnSync might have been causing crash on start on windows
7777

78-
function _spawnAsync(log: any, ...args: any[]) {
78+
// Listens on 'close' (not 'exit') so all stderr 'data' events have been delivered
79+
// before we read errorMessage — otherwise on macOS a fast-exiting child can deliver
80+
// its exit signal before its stderr drains, producing a rejection with an empty
81+
// message and breaking the "configuration file already exists" suppression upstream.
82+
export function _gatherChildOutput(log: any, child: ChildProcessWithoutNullStreams): Promise<null> {
7983
return new Promise((resolve, reject) => {
80-
//@ts-ignore
81-
const spawedProcess: ChildProcessWithoutNullStreams = spawn(...args);
8284
let errorMessage = "";
83-
spawedProcess.on("exit", (exitCode, signal) => {
84-
if (exitCode === 0) resolve(null);
85-
else {
86-
const error = new Error(errorMessage);
87-
Object.assign(error, { exitCode, pid: spawedProcess.pid, signal });
88-
reject(error);
89-
}
85+
child.on("close", (exitCode, signal) => {
86+
if (exitCode === 0) return resolve(null);
87+
const error = new Error(errorMessage);
88+
Object.assign(error, { exitCode, pid: child.pid, signal });
89+
reject(error);
9090
});
91-
spawedProcess.stderr.on("data", (data) => {
91+
child.stderr.on("data", (data) => {
9292
log.trace(data.toString());
9393
errorMessage += data.toString();
9494
});
95-
spawedProcess.stdin.on("data", (data) => log.trace(data.toString()));
96-
spawedProcess.stdout.on("data", (data) => log.trace(data.toString()));
97-
spawedProcess.on("error", (data) => {
95+
child.stdin.on("data", (data) => log.trace(data.toString()));
96+
child.stdout.on("data", (data) => log.trace(data.toString()));
97+
child.on("error", (data) => {
9898
errorMessage += data.toString();
9999
log.error(data.toString());
100100
});
101101
});
102102
}
103103

104+
function _spawnAsync(log: any, ...args: any[]) {
105+
//@ts-ignore
106+
const child: ChildProcessWithoutNullStreams = spawn(...args);
107+
return _gatherChildOutput(log, child);
108+
}
109+
104110
type MultiaddrComponent = { name: string; value?: string };
105111
type MultiaddrModule = {
106112
multiaddr: (multiAddr: string) => {
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import { describe, it, expect } from "vitest";
2+
import { EventEmitter } from "node:events";
3+
import { _gatherChildOutput } from "../../dist/ipfs/startIpfs.js";
4+
5+
const noopLog = Object.assign(() => {}, { trace: () => {}, error: () => {} });
6+
7+
const makeFakeChild = () =>
8+
Object.assign(new EventEmitter(), {
9+
stdout: new EventEmitter(),
10+
stderr: new EventEmitter(),
11+
stdin: new EventEmitter(),
12+
pid: 12345
13+
}) as any;
14+
15+
describe("_gatherChildOutput", () => {
16+
it("captures stderr even when the child's 'exit' event fires before stderr 'data' (macOS race)", async () => {
17+
const child = makeFakeChild();
18+
const promise = _gatherChildOutput(noopLog, child);
19+
20+
// Simulate the macOS-style ordering: process exits first, stderr drains
21+
// afterwards, 'close' fires last. The previous 'exit'-based listener would
22+
// settle the promise with an empty errorMessage at the first emit and miss
23+
// the stderr payload entirely.
24+
queueMicrotask(() => {
25+
child.emit("exit", 1, null);
26+
setImmediate(() => {
27+
child.stderr.emit("data", Buffer.from("Error: ipfs configuration file already exists!\n"));
28+
child.emit("close", 1, null);
29+
});
30+
});
31+
32+
await expect(promise).rejects.toThrow(/ipfs configuration file already exists!/);
33+
});
34+
35+
it("resolves cleanly when the child exits with code 0", async () => {
36+
const child = makeFakeChild();
37+
const promise = _gatherChildOutput(noopLog, child);
38+
queueMicrotask(() => child.emit("close", 0, null));
39+
await expect(promise).resolves.toBeNull();
40+
});
41+
});

0 commit comments

Comments
 (0)