diff --git a/src/music/spotify/go-librespot.test.ts b/src/music/spotify/go-librespot.test.ts index aaa6ae1..0d198d0 100644 --- a/src/music/spotify/go-librespot.test.ts +++ b/src/music/spotify/go-librespot.test.ts @@ -219,3 +219,43 @@ describe("GoLibrespotBackend.stop", () => { expect(h.backend.isReady()).toBe(false); }); }); + +describe("GoLibrespotBackend.start failure cleanup", () => { + it("tears down ffmpeg, go-librespot, and the FIFO when readiness polling never succeeds", async () => { + const h = makeHarness(); + // ping() never returns true → waitUntilReady() times out → start() rejects + // AFTER both processes were spawned and the FIFO was created. + h.rest.ping.mockResolvedValue(false); + // FIFO present so the cleanup path unlinks it. + h.existsSync.mockReturnValue(true); + + await expect(h.backend.start()).rejects.toThrow(/did not become ready/); + + expect(h.ffmpegChild.kill).toHaveBeenCalled(); // ffmpeg killed on failed startup + expect(h.gliChild.kill).toHaveBeenCalled(); // go-librespot killed on failed startup + expect(h.unlinkSync).toHaveBeenCalledWith("/tmp/work/go-librespot.fifo"); // FIFO removed + expect(h.backend.isReady()).toBe(false); + }); +}); + +describe("GoLibrespotBackend child-process error handling", () => { + it("does not throw when a child 'error' is emitted with no backend 'error' listener attached", async () => { + const h = makeHarness(); + await h.backend.start(); + // No "error" listener on the backend: an unhandled 'error' event would crash + // Node, so the backend must swallow+log it instead of re-emitting. + expect(h.backend.listenerCount("error")).toBe(0); + expect(() => h.ffmpegChild.emit("error", new Error("ffmpeg boom"))).not.toThrow(); + expect(() => h.gliChild.emit("error", new Error("gli boom"))).not.toThrow(); + }); + + it("re-emits a child 'error' to an attached backend 'error' listener", async () => { + const h = makeHarness(); + await h.backend.start(); + const onErr = vi.fn(); + h.backend.on("error", onErr); + const err = new Error("ffmpeg boom"); + h.ffmpegChild.emit("error", err); + expect(onErr).toHaveBeenCalledWith(err); + }); +}); diff --git a/src/music/spotify/go-librespot.ts b/src/music/spotify/go-librespot.ts index c2f35df..cdc1f0c 100644 --- a/src/music/spotify/go-librespot.ts +++ b/src/music/spotify/go-librespot.ts @@ -105,66 +105,89 @@ export class GoLibrespotBackend extends EventEmitter implements SpotifyAudioBack if (existsSync(this.fifoPath)) unlinkSync(this.fifoPath); execFileSync("mkfifo", [this.fifoPath]); - // 3. Spawn ffmpeg FIRST so the PCM reader is attached to the FIFO before - // go-librespot (the writer) starts pushing raw 44.1k s16le into it. - // Opening the FIFO for writing before a reader exists errors with ENXIO. - this.ffmpeg = spawn( - ffmpegCommand, - [ - "-hide_banner", "-loglevel", "error", - "-f", "s16le", "-ar", "44100", "-ac", "2", "-i", this.fifoPath, - "-f", "s16le", "-ar", "48000", "-ac", "2", "-acodec", "pcm_s16le", "pipe:1", - ], - { stdio: ["ignore", "pipe", "pipe"] }, - ); - this.ffmpeg.stderr?.on("data", (b: Buffer) => - this.log.debug({ ffmpeg: b.toString().trim() }, "ffmpeg"), - ); - this.ffmpeg.on("error", (err) => this.emit("error", err)); + // Everything past this point spawns processes / opens sockets. If any step + // throws (e.g. the readiness poll times out), tear down whatever was already + // created via stop() (kill ffmpeg + go-librespot, close WS, remove FIFO) so + // we don't leak child processes or leave the FIFO on disk, then rethrow. + try { + // 3. Spawn ffmpeg FIRST so the PCM reader is attached to the FIFO before + // go-librespot (the writer) starts pushing raw 44.1k s16le into it. + // Opening the FIFO for writing before a reader exists errors with ENXIO. + this.ffmpeg = spawn( + ffmpegCommand, + [ + "-hide_banner", "-loglevel", "error", + "-f", "s16le", "-ar", "44100", "-ac", "2", "-i", this.fifoPath, + "-f", "s16le", "-ar", "48000", "-ac", "2", "-acodec", "pcm_s16le", "pipe:1", + ], + { stdio: ["ignore", "pipe", "pipe"] }, + ); + this.ffmpeg.stderr?.on("data", (b: Buffer) => + this.log.debug({ ffmpeg: b.toString().trim() }, "ffmpeg"), + ); + this.ffmpeg.on("error", (err) => this.emitError(err)); - // 4. Render + write config.yml into the config dir. - const yml = renderConfigYml({ - deviceName: this.opts.deviceName, - bitrate: this.opts.bitrate, - fifoPath: this.fifoPath, - apiAddress: "0.0.0.0", - apiPort: this.apiPort, - callbackPort: DEFAULT_CALLBACK_PORT, - }); - writeFileSync(posixPath.join(this.opts.configDir, "config.yml"), yml, "utf8"); + // 4. Render + write config.yml into the config dir. + const yml = renderConfigYml({ + deviceName: this.opts.deviceName, + bitrate: this.opts.bitrate, + fifoPath: this.fifoPath, + apiAddress: "0.0.0.0", + apiPort: this.apiPort, + callbackPort: DEFAULT_CALLBACK_PORT, + }); + writeFileSync(posixPath.join(this.opts.configDir, "config.yml"), yml, "utf8"); - // 5. Spawn go-librespot AFTER ffmpeg is listening on the FIFO. Its stdout/ - // stderr carry the interactive OAuth URL on first run — surface via logger. - const bin = findBinary(); - this.proc = spawn(bin, ["--config_dir", this.opts.configDir], { - stdio: ["ignore", "pipe", "pipe"], - }); - const onLog = (b: Buffer) => this.log.info({ golibrespot: b.toString().trim() }, "go-librespot"); - this.proc.stdout?.on("data", onLog); - this.proc.stderr?.on("data", onLog); - this.proc.on("error", (err) => this.emit("error", err)); - this.proc.on("exit", (code, signal) => { - this.ready = false; - this.log.warn({ code, signal }, "go-librespot exited"); - }); + // 5. Spawn go-librespot AFTER ffmpeg is listening on the FIFO. Its stdout/ + // stderr carry the interactive OAuth URL on first run — surface via logger. + const bin = findBinary(); + this.proc = spawn(bin, ["--config_dir", this.opts.configDir], { + stdio: ["ignore", "pipe", "pipe"], + }); + const onLog = (b: Buffer) => this.log.info({ golibrespot: b.toString().trim() }, "go-librespot"); + this.proc.stdout?.on("data", onLog); + this.proc.stderr?.on("data", onLog); + this.proc.on("error", (err) => this.emitError(err)); + this.proc.on("exit", (code, signal) => { + this.ready = false; + this.log.warn({ code, signal }, "go-librespot exited"); + }); - // 6. REST client, then poll GET / until the HTTP server answers. - const baseUrl = `http://127.0.0.1:${this.apiPort}`; - this.rest = this.deps.makeRest - ? this.deps.makeRest(baseUrl) - : new GoLibrespotRestClient(baseUrl); - await this.waitUntilReady(); + // 6. REST client, then poll GET / until the HTTP server answers. + const baseUrl = `http://127.0.0.1:${this.apiPort}`; + this.rest = this.deps.makeRest + ? this.deps.makeRest(baseUrl) + : new GoLibrespotRestClient(baseUrl); + await this.waitUntilReady(); - // 7. Connect the WebSocket event stream and wire event mapping. - const wsUrl = `ws://127.0.0.1:${this.apiPort}/events`; - this.events = this.deps.makeEvents - ? this.deps.makeEvents(wsUrl) - : new GoLibrespotEventClient(wsUrl); - this.wireEvents(this.events); - this.events.start(); + // 7. Connect the WebSocket event stream and wire event mapping. + const wsUrl = `ws://127.0.0.1:${this.apiPort}/events`; + this.events = this.deps.makeEvents + ? this.deps.makeEvents(wsUrl) + : new GoLibrespotEventClient(wsUrl); + this.wireEvents(this.events); + this.events.start(); - this.ready = true; - this.emit("ready"); + this.ready = true; + this.emit("ready"); + } catch (e) { + this.stop(); + throw e; + } + } + + /** + * Re-emit a child-process "error" only when a consumer is listening; Node + * throws on an unhandled "error" event (can crash the process), so with no + * listener we log via the injected logger instead. Mirrors the WS client's + * listenerCount("error") gate in go-librespot-api.ts. + */ + private emitError(err: unknown): void { + if (this.listenerCount("error") > 0) { + this.emit("error", err); + } else { + this.log.error({ err }, "go-librespot backend error (no listener)"); + } } private async waitUntilReady(): Promise {