From 179e7c248a72e168b67d3217296912aa55ee3b02 Mon Sep 17 00:00:00 2001
From: saopig1 <4x7sw862st@gmail.com>
Date: Thu, 2 Jul 2026 21:30:13 +0800
Subject: [PATCH] fix(spotify): tear down errored backend in SpotifyController
 (no leak/cross-talk)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/music/spotify/controller.test.ts | 68 ++++++++++++++++++++++++++++
 src/music/spotify/controller.ts      | 19 ++++++++
 2 files changed, 87 insertions(+)

diff --git a/src/music/spotify/controller.test.ts b/src/music/spotify/controller.test.ts
index 901fc1c..dfe897e 100644
--- a/src/music/spotify/controller.test.ts
+++ b/src/music/spotify/controller.test.ts
@@ -310,6 +310,74 @@ describe("SpotifyController backend error handling (C3)", () => {
     expect(built).toBe(2);
     expect(be2.startCalls).toBe(1);
   });
+
+  it("tears down the errored backend (stop + detach + null) so it is not orphaned", async () => {
+    const be1 = new FakeBackend();
+    const be2 = new FakeBackend();
+    const backends = [be1, be2];
+    let built = 0;
+    const { ctrl } = makeCtrl({
+      backendFactory: () => {
+        built++;
+        return backends.shift()!;
+      },
+    });
+    await ctrl.ensureStarted();
+    expect(built).toBe(1);
+
+    // On "error" the controller must stop() the errored backend (cleans its
+    // ffmpeg/go-librespot children + FIFO) and detach ALL its listeners.
+    expect(() => be1.emit("error", new Error("sidecar boom"))).not.toThrow();
+    expect(be1.stopCalls).toBe(1);
+    expect(be1.listenerCount("error")).toBe(0);
+    expect(be1.listenerCount("trackEnded")).toBe(0);
+    expect(be1.listenerCount("metadata")).toBe(0);
+
+    // Internal backend is null: a transport call no-ops (delegates to nothing)
+    // and getPcmStream throws until a rebuild.
+    await ctrl.pause();
+    expect(be1.pauseCalls).toBe(0);
+    expect(() => ctrl.getPcmStream()).toThrow();
+
+    // A fresh ensureStarted builds a NEW backend instance.
+    expect(await ctrl.ensureStarted()).toBe(true);
+    expect(built).toBe(2);
+    expect(be2.startCalls).toBe(1);
+    expect(ctrl.getPcmStream()).toBe(be2.pcm);
+  });
+
+  it("does not cross-talk: a later error from the ORPHANED backend leaves the healthy rebuilt controller ready", async () => {
+    const be1 = new FakeBackend();
+    const be2 = new FakeBackend();
+    const backends = [be1, be2];
+    let built = 0;
+    const { ctrl } = makeCtrl({
+      backendFactory: () => {
+        built++;
+        return backends.shift()!;
+      },
+    });
+    await ctrl.ensureStarted();
+
+    // First error tears down be1 and the controller rebuilds onto healthy be2.
+    be1.emit("error", new Error("sidecar boom"));
+    expect(await ctrl.ensureStarted()).toBe(true);
+    expect(built).toBe(2);
+    await ctrl.pause();
+    expect(be2.pauseCalls).toBe(1);
+
+    // A SECOND error emitted by the OLD (orphaned) backend must NOT reach the
+    // controller: the healthy be2 stays owned, ready, and untouched.
+    be1.on("error", () => {}); // controller detached; re-arm to avoid unhandled throw
+    expect(() => be1.emit("error", new Error("orphan boom"))).not.toThrow();
+    expect(be2.stopCalls).toBe(0); // healthy backend not torn down
+    // Still ready: ensureStarted returns true without rebuilding a third time.
+    expect(await ctrl.ensureStarted()).toBe(true);
+    expect(built).toBe(2);
+    await ctrl.pause();
+    expect(be2.pauseCalls).toBe(2);
+    expect(ctrl.getPcmStream()).toBe(be2.pcm);
+  });
 });
 
 describe("SpotifyController.stop", () => {
diff --git a/src/music/spotify/controller.ts b/src/music/spotify/controller.ts
index 3005244..f378cf2 100644
--- a/src/music/spotify/controller.ts
+++ b/src/music/spotify/controller.ts
@@ -114,6 +114,25 @@ export class SpotifyController extends EventEmitter {
    */
   private handleBackendError(err: unknown): void {
     this.logger.error({ err }, "Spotify backend error; marking not-ready");
+    // Tear down the errored backend BEFORE resetting flags so ensureStarted()
+    // does not orphan it: stop() cleans its ffmpeg/go-librespot children + FIFO,
+    // removeAllListeners() detaches its "error" handler so a later error from
+    // this now-orphaned backend cannot flip a healthy rebuilt controller
+    // back to not-ready (state cross-talk). Teardown must never mask the
+    // original error, so guard stop() which may throw.
+    try {
+      this.backend?.stop();
+    } catch (stopErr) {
+      this.logger.error(
+        { err: stopErr },
+        "Spotify backend stop() threw during error teardown",
+      );
+    }
+    // SpotifyAudioBackend's type contract exposes on() but not
+    // removeAllListeners(); every concrete backend extends EventEmitter, so
+    // detach through it to drop this controller's listeners from the orphan.
+    (this.backend as unknown as EventEmitter | null)?.removeAllListeners();
+    this.backend = null;
     this.started = false;
     this.startPromise = null;
   }