From 79fb8443be6c3eae7db16a3c0441934d6f59f92c Mon Sep 17 00:00:00 2001 From: TIANYAO ZHANG <88520881+ZHANGTIANYAO1@users.noreply.github.com> Date: Sat, 3 Oct 2026 16:55:34 +0800 Subject: [PATCH] fix: fence stream recovery and EOF advancement by playback session --- src/audio/player.ts | 2 + src/bot/instance.test.ts | 90 ++++++++++++++++++++++++++++++++++++++++ src/bot/instance.ts | 17 +++++++- 3 files changed, 108 insertions(+), 1 deletion(-) diff --git a/src/audio/player.ts b/src/audio/player.ts index 18cc644..19d5397 100644 --- a/src/audio/player.ts +++ b/src/audio/player.ts @@ -834,6 +834,8 @@ export class AudioPlayer extends EventEmitter { } getDuckingGain(): number { return this.duckingGainAt(performance.now()); } getState(): PlayerState { return this.state; } + /** Changes on stop or a new play/seek, so asynchronous recovery can be fenced. */ + getPlaybackSessionId(): number { return this.sessionId; } // True only while attached to an external (Spotify sidecar) PCM stream. Used // by the orchestrator to decide whether to re-attach: stop() detaches (sets // externalMode=false) so this is false after any player.stop(). diff --git a/src/bot/instance.test.ts b/src/bot/instance.test.ts index 80e4feb..2154010 100644 --- a/src/bot/instance.test.ts +++ b/src/bot/instance.test.ts @@ -1683,6 +1683,7 @@ describe("resumeInterruptedStream — long B站 streams dying mid-play (#161)", player: { getElapsed: vi.fn(() => elapsed), getState: vi.fn(() => state), + getPlaybackSessionId: vi.fn(() => 1), play: vi.fn(() => { state = "playing"; }), }, getProviderFor: vi.fn(() => provider), @@ -1751,3 +1752,92 @@ describe("resumeInterruptedStream — long B站 streams dying mid-play (#161)", expect(ctx.player.play).not.toHaveBeenCalled(); }); }); + +describe("BotInstance trackEnd — stale playback sessions", () => { + function makeEndedCtx(platform = "bilibili", duration = 10_000) { + const song = { + id: "ended", name: "Ended", artist: "A", album: "", coverUrl: "", + platform, duration, url: "old", + }; + let current: any = song; + let state = "idle"; + let session = 1; + const player = new EventEmitter() as any; + player.getState = () => state; + player.getElapsed = () => 1000; + player.getPlaybackSessionId = () => session; + player.play = vi.fn(() => { session++; state = "playing"; }); + const provider = { getSongUrl: vi.fn(async () => ({ url: "fresh" })) }; + const advances: string[] = []; + const ctx: any = { + song, provider, player, connected: true, effectiveDuration: duration, + streamRecovery: null, queue: { current: () => current }, + spotifyController: new EventEmitter(), tsClient: { sendVoiceData: vi.fn() }, + logger: { warn: vi.fn(), debug: vi.fn(), error: vi.fn() }, emit: vi.fn(), + getProviderFor: () => provider, + playNext: vi.fn(async () => { advances.push(current?.id ?? "empty"); return true; }), + replace: () => { current = { ...song, id: "replacement" }; session++; state = "playing"; }, + stop: () => { current = null; session++; state = "idle"; }, + restartSameSong: () => { session++; state = "idle"; }, + pause: () => { state = "paused"; }, + advances, + }; + ctx.resumeInterruptedStream = (BotInstance.prototype as any).resumeInterruptedStream.bind(ctx); + setupPlayerEvents.call(ctx); + return ctx; + } + + async function flushEvents() { + await new Promise(resolve => setImmediate(resolve)); + } + + it.each(["netease", "bilibili"])("an old normal %s EOF never skips a pending replacement", async platform => { + const ctx = makeEndedCtx(platform, 1000); + const replacement = Promise.resolve().then(() => ctx.replace()); + ctx.player.emit("trackEnd"); + await replacement; + await flushEvents(); + expect(ctx.advances).not.toContain("replacement"); + }); + + it("normal EOF still advances the ending track when no replacement arrives", async () => { + const ctx = makeEndedCtx("netease", 1000); + ctx.player.emit("trackEnd"); + await flushEvents(); + expect(ctx.advances).toEqual(["ended"]); + }); + + it("a failed recovery never advances a replacement", async () => { + const ctx = makeEndedCtx(); + const lookup = deferred<{ url: string }>(); + ctx.provider.getSongUrl.mockReturnValue(lookup.promise); + ctx.player.emit("trackEnd"); + ctx.replace(); + lookup.reject(new Error("temporary lookup failure")); + await flushEvents(); + expect(ctx.advances).toEqual([]); + }); + + it.each(["stop", "restartSameSong", "pause"])("recovery does not overwrite playback after %s", async action => { + const ctx = makeEndedCtx(); + const lookup = deferred<{ url: string }>(); + ctx.provider.getSongUrl.mockReturnValue(lookup.promise); + ctx.player.emit("trackEnd"); + ctx[action](); + lookup.resolve({ url: "fresh" }); + await flushEvents(); + expect(ctx.player.play).not.toHaveBeenCalled(); + expect(ctx.advances).toEqual([]); + }); + + it("a failed recovery cannot advance a newer session of the same queue song", async () => { + const ctx = makeEndedCtx(); + const lookup = deferred<{ url: string }>(); + ctx.provider.getSongUrl.mockReturnValue(lookup.promise); + ctx.player.emit("trackEnd"); + ctx.restartSameSong(); + lookup.reject(new Error("temporary lookup failure")); + await flushEvents(); + expect(ctx.advances).toEqual([]); + }); +}); diff --git a/src/bot/instance.ts b/src/bot/instance.ts index 1489faf..2d91ed8 100755 --- a/src/bot/instance.ts +++ b/src/bot/instance.ts @@ -330,6 +330,8 @@ export class BotInstance extends EventEmitter { }); this.player.on("trackEnd", () => { + const endedSong = this.queue.current(); + const endedSession = this.player.getPlaybackSessionId(); this.resumeInterruptedStream() .catch((err) => { this.logger.warn({ err }, "Stream resume failed"); @@ -337,6 +339,14 @@ export class BotInstance extends EventEmitter { }) .then((resumed) => { if (resumed) return; + // A pending command may replace, stop, or restart the same queue + // song before this continuation. Only advance the session that ended. + if ( + !this.connected || + this.queue.current() !== endedSong || + this.player.getPlaybackSessionId() !== endedSession || + this.player.getState() !== "idle" + ) return; this.logger.debug("Track ended, advancing queue"); return this.playNext(); }) @@ -1161,6 +1171,7 @@ export class BotInstance extends EventEmitter { } const duration = this.effectiveDuration ?? song.duration; const position = Math.floor(this.player.getElapsed()); + const endedSession = this.player.getPlaybackSessionId(); if (!(duration > 0) || duration - position <= BotInstance.STREAM_END_TOLERANCE_S) { return false; } @@ -1192,7 +1203,11 @@ export class BotInstance extends EventEmitter { const result = await this.getProviderFor(song.platform).getSongUrl(song.id); // The user may have skipped/stopped while we were resolving; never // clobber whatever is playing now. - if (this.queue.current() !== song || this.player.getState() !== "idle") return true; + if ( + this.queue.current() !== song || + this.player.getPlaybackSessionId() !== endedSession || + this.player.getState() !== "idle" + ) return true; if (!result?.url || !this.connected) return false; song.url = result.url;