mirror of
https://github.com/ZHANGTIANYAO1/teamspeak-music-bot.git
synced 2026-10-02 04:52:50 +08:00
fix(spotify): tear down errored backend in SpotifyController (no leak/cross-talk)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
3a9504500b
commit
179e7c248a
2 files changed
+87
No files matched your search
@@ -310,6 +310,74 @@ describe("SpotifyController backend error handling (C3)", () => {
|
|||||||
expect(built).toBe(2);
|
expect(built).toBe(2);
|
||||||
expect(be2.startCalls).toBe(1);
|
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", () => {
|
describe("SpotifyController.stop", () => {
|
||||||
|
|||||||
@@ -114,6 +114,25 @@ export class SpotifyController extends EventEmitter {
|
|||||||
*/
|
*/
|
||||||
private handleBackendError(err: unknown): void {
|
private handleBackendError(err: unknown): void {
|
||||||
this.logger.error({ err }, "Spotify backend error; marking not-ready");
|
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.started = false;
|
||||||
this.startPromise = null;
|
this.startPromise = null;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user