diff --git a/src/audio-manager.ts b/src/audio-manager.ts index 23b5e9b..cf9b8e7 100644 --- a/src/audio-manager.ts +++ b/src/audio-manager.ts @@ -82,21 +82,27 @@ export default class AudioManager { this.clearRenewTimer(); this.playbackState = 'connecting'; this.connection?.destroy(); - this.connection = joinVoiceChannel({ + const connection = joinVoiceChannel({ guildId: this.connectionOptions.guildId, channelId: this.connectionOptions.channelId, adapterCreator: this.connectionOptions.adapterCreator, }); - this.connection.subscribe(this.audioPlayer); + this.connection = connection; + connection.subscribe(this.audioPlayer); try { - await entersState(this.connection, VoiceConnectionStatus.Ready, this.options.connectTimeoutMs); + await entersState(connection, VoiceConnectionStatus.Ready, this.options.connectTimeoutMs); } catch (error) { - this.connection.destroy(); - this.connection = undefined; + connection.destroy(); + if (this.connection === connection) { + this.connection = undefined; + } this.playbackState = 'stopped'; throw error; } + if (this.connection !== connection) { + throw new AudioManagerStateError('Voice connection was stopped before it became ready.'); + } this.playbackState = 'ready'; this.scheduleRenewal(); } @@ -115,23 +121,35 @@ export default class AudioManager { const resolvedSource = this.resolveSource(); this.stopCurrentPlayback(); this.ffmpeg = startFfmpeg(resolvedSource.input, this.options.ffmpeg); + const ffmpeg = this.ffmpeg; try { - await this.ffmpeg.ready; + await ffmpeg.ready; + if (this.ffmpeg !== ffmpeg) { + throw new AudioManagerStateError('Playback was stopped before ffmpeg became ready.'); + } + this.resource = createAudioResource(ffmpeg.process.stdout, { + inputType: StreamType.Raw, + inlineVolume: this.options.volume?.enabled === true, + }); } catch (error) { - this.stopCurrentPlayback(); + if (this.ffmpeg === ffmpeg) { + this.stopCurrentPlayback(); + } throw error; } - this.resource = createAudioResource(this.ffmpeg.process.stdout, { - inputType: StreamType.Raw, - inlineVolume: this.options.volume?.enabled === true, - }); if (this.options.volume?.enabled === true && this.options.volume.initialPercent !== undefined) { this.setVolume(this.options.volume.initialPercent); } - this.audioPlayer.play(this.resource); - this.playbackState = 'playing'; + try { + this.audioPlayer.play(this.resource); + this.playbackState = 'playing'; + } catch (error) { + this.stopCurrentPlayback(); + this.playbackState = 'ready'; + throw error; + } } public async start(): Promise { diff --git a/tests/audio-manager.test.ts b/tests/audio-manager.test.ts index 9f5badc..129ec06 100644 --- a/tests/audio-manager.test.ts +++ b/tests/audio-manager.test.ts @@ -1,12 +1,14 @@ import { jest, describe, beforeEach, afterEach, it, expect } from '@jest/globals'; -import { entersState } from '@discordjs/voice'; +import { createAudioResource, entersState, joinVoiceChannel } from '@discordjs/voice'; import { resolve } from 'node:path'; import { PassThrough } from 'node:stream'; import AudioManager from '../src/audio-manager'; import { AudioManagerConfigError, AudioManagerStateError, FfmpegProcessError } from '../src'; import { startFfmpeg } from '../src/ffmpeg'; +import type { FfmpegProcessHandle } from '../src/ffmpeg'; import type { AudioSource, VoiceConnectionOptions } from '../src'; +import type { VoiceConnection } from '@discordjs/voice'; const mockAudioPlayer = { play: jest.fn(), @@ -19,6 +21,13 @@ const mockConnection = { disconnect: jest.fn(), destroy: jest.fn(), }; +const mockSecondConnection = { + subscribe: jest.fn(), + disconnect: jest.fn(), + destroy: jest.fn(), +}; +const mockVoiceConnection = mockConnection as unknown as VoiceConnection; +const mockSecondVoiceConnection = mockSecondConnection as unknown as VoiceConnection; const mockAudioResource = { playStream: { destroy: jest.fn(), @@ -27,10 +36,10 @@ const mockAudioResource = { setVolume: jest.fn(), }, }; -const mockFfmpegHandle = { +const mockFfmpegHandle: FfmpegProcessHandle = { process: { stdout: new PassThrough(), - }, + } as unknown as FfmpegProcessHandle['process'], ready: Promise.resolve(), stop: jest.fn(), }; @@ -47,8 +56,8 @@ jest.mock('@discordjs/voice', () => ({ }, createAudioPlayer: jest.fn(() => mockAudioPlayer), createAudioResource: jest.fn(() => mockAudioResource), - entersState: jest.fn(() => Promise.resolve(mockConnection)), - joinVoiceChannel: jest.fn(() => mockConnection), + entersState: jest.fn(() => Promise.resolve(mockVoiceConnection)), + joinVoiceChannel: jest.fn(() => mockVoiceConnection), })); jest.mock('../src/ffmpeg', () => ({ @@ -68,6 +77,26 @@ const liveStreamSource: AudioSource = { const fileSourcePath = 'tests/audio.mp3'; const resolvedFileSourcePath = resolve(process.cwd(), fileSourcePath); +type MockedEntersStateReturn = ReturnType; + +function deferred(): { promise: Promise; resolve: (value: T) => void } { + let resolve!: (value: T) => void; + const promise = new Promise((promiseResolve) => { + resolve = promiseResolve; + }); + + return { promise, resolve }; +} + +function createMockFfmpegHandle(): FfmpegProcessHandle { + return { + process: { + stdout: new PassThrough(), + } as unknown as FfmpegProcessHandle['process'], + ready: Promise.resolve(), + stop: jest.fn(), + }; +} describe('AudioManager', () => { beforeEach(() => { @@ -119,6 +148,73 @@ describe('AudioManager', () => { expect(jest.getTimerCount()).toBe(0); }); + it('does not become ready when stopped during connection startup', async () => { + jest.useFakeTimers(); + const ready = deferred(); + jest.mocked(entersState).mockReturnValueOnce(ready.promise as unknown as MockedEntersStateReturn); + const manager = new AudioManager({ + connection: connectionOptions, + renewIntervalMs: 10_000, + }); + + const connectPromise = manager.connect(); + await manager.stop(); + ready.resolve(mockVoiceConnection); + + await expect(connectPromise).rejects.toThrow(AudioManagerStateError); + + expect(manager.state).toBe('stopped'); + expect(manager.isConnected).toBe(false); + expect(jest.getTimerCount()).toBe(0); + }); + + it('keeps the latest connection when concurrent connects resolve out of order', async () => { + const firstReady = deferred(); + const secondReady = deferred(); + jest.mocked(joinVoiceChannel) + .mockReturnValueOnce(mockVoiceConnection) + .mockReturnValueOnce(mockSecondVoiceConnection); + jest.mocked(entersState) + .mockReturnValueOnce(firstReady.promise as unknown as MockedEntersStateReturn) + .mockReturnValueOnce(secondReady.promise as unknown as MockedEntersStateReturn); + const manager = new AudioManager({ + connection: connectionOptions, + renewIntervalMs: false, + }); + + const firstConnect = manager.connect(); + const secondConnect = manager.connect(); + firstReady.resolve(mockVoiceConnection); + secondReady.resolve(mockSecondVoiceConnection); + + await expect(firstConnect).rejects.toThrow(AudioManagerStateError); + await expect(secondConnect).resolves.toBeUndefined(); + + expect(manager.state).toBe('ready'); + expect(manager.isConnected).toBe(true); + expect(mockConnection.destroy).toHaveBeenCalledTimes(1); + expect(mockSecondConnection.destroy).not.toHaveBeenCalled(); + }); + + it('does not become ready when disposed during connection startup', async () => { + const ready = deferred(); + jest.mocked(entersState).mockReturnValueOnce(ready.promise as unknown as MockedEntersStateReturn); + const manager = new AudioManager({ + connection: connectionOptions, + renewIntervalMs: false, + }); + + const connectPromise = manager.connect(); + manager.dispose(); + ready.resolve(mockVoiceConnection); + + await expect(connectPromise).rejects.toThrow(AudioManagerStateError); + + expect(manager.state).toBe('disposed'); + expect(manager.isConnected).toBe(false); + expect(mockConnection.destroy).toHaveBeenCalledTimes(1); + }); + it('starts playback with the committed mp3 test file', async () => { const manager = new AudioManager({ connection: connectionOptions, @@ -184,6 +280,118 @@ describe('AudioManager', () => { expect(mockFfmpegHandle.stop).toHaveBeenCalledTimes(1); }); + it('cleans up when audio resource creation fails', async () => { + const resourceError = new Error('resource failed'); + jest.mocked(createAudioResource).mockImplementationOnce(() => { + throw resourceError; + }); + const manager = new AudioManager({ + connection: connectionOptions, + source: liveStreamSource, + renewIntervalMs: false, + }); + + await manager.connect(); + await expect(manager.play()).rejects.toBe(resourceError); + + expect(manager.state).toBe('ready'); + expect(mockFfmpegHandle.stop).toHaveBeenCalledTimes(1); + expect(mockAudioPlayer.play).not.toHaveBeenCalled(); + }); + + it('cleans up when the audio player rejects playback', async () => { + const playerError = new Error('player failed'); + mockAudioPlayer.play.mockImplementationOnce(() => { + throw playerError; + }); + const manager = new AudioManager({ + connection: connectionOptions, + source: liveStreamSource, + renewIntervalMs: false, + }); + + await manager.connect(); + await expect(manager.play()).rejects.toBe(playerError); + + expect(manager.state).toBe('ready'); + expect(mockFfmpegHandle.stop).toHaveBeenCalledTimes(1); + expect(mockAudioResource.playStream.destroy).toHaveBeenCalledTimes(1); + }); + + it('does not report playback when stopped before ffmpeg is ready', async () => { + const ready = deferred(); + jest.mocked(startFfmpeg).mockReturnValueOnce({ + ...mockFfmpegHandle, + ready: ready.promise, + }); + const manager = new AudioManager({ + connection: connectionOptions, + source: liveStreamSource, + renewIntervalMs: false, + }); + + await manager.connect(); + const playPromise = manager.play(); + await manager.stop(); + ready.resolve(); + + await expect(playPromise).rejects.toThrow(AudioManagerStateError); + + expect(manager.state).toBe('stopped'); + expect(mockAudioPlayer.play).not.toHaveBeenCalled(); + }); + + it('does not report playback when disposed before ffmpeg is ready', async () => { + const ready = deferred(); + jest.mocked(startFfmpeg).mockReturnValueOnce({ + ...mockFfmpegHandle, + ready: ready.promise, + }); + const manager = new AudioManager({ + connection: connectionOptions, + source: liveStreamSource, + renewIntervalMs: false, + }); + + await manager.connect(); + const playPromise = manager.play(); + manager.dispose(); + ready.resolve(); + + await expect(playPromise).rejects.toThrow(AudioManagerStateError); + + expect(manager.state).toBe('disposed'); + expect(mockAudioPlayer.play).not.toHaveBeenCalled(); + }); + + it('keeps only the latest concurrent playback', async () => { + const firstReady = deferred(); + const firstHandle = { + ...createMockFfmpegHandle(), + ready: firstReady.promise, + }; + const secondHandle = createMockFfmpegHandle(); + jest.mocked(startFfmpeg).mockReturnValueOnce(firstHandle).mockReturnValueOnce(secondHandle); + const manager = new AudioManager({ + connection: connectionOptions, + source: liveStreamSource, + renewIntervalMs: false, + }); + + await manager.connect(); + const firstPlay = manager.play(); + const secondPlay = manager.play({ type: 'file', path: fileSourcePath }); + firstReady.resolve(); + + await expect(firstPlay).rejects.toThrow(AudioManagerStateError); + await expect(secondPlay).resolves.toBeUndefined(); + + expect(manager.state).toBe('playing'); + expect(mockAudioPlayer.play).toHaveBeenCalledTimes(1); + expect(firstHandle.stop).toHaveBeenCalledTimes(1); + expect(secondHandle.stop).not.toHaveBeenCalled(); + }); + it('replaces active playback when a new source is played', async () => { const manager = new AudioManager({ connection: connectionOptions, @@ -277,6 +485,7 @@ describe('AudioManager', () => { expect(() => manager.setVolume(Number.POSITIVE_INFINITY)).toThrow(AudioManagerConfigError); expect(() => manager.setVolume(Number.NEGATIVE_INFINITY)).toThrow(AudioManagerConfigError); expect(() => manager.setVolume(0)).not.toThrow(); + expect(() => manager.setVolume(42)).not.toThrow(); expect(() => manager.setVolume(100)).not.toThrow(); }); @@ -326,6 +535,20 @@ describe('AudioManager', () => { expect(manager.state).toBe('stopped'); }); + it('rejects pause and resume after stop', async () => { + const manager = new AudioManager({ + connection: connectionOptions, + source: liveStreamSource, + renewIntervalMs: false, + }); + + await manager.start(); + await manager.stop(); + + expect(() => manager.pause()).toThrow(AudioManagerStateError); + expect(() => manager.resume()).toThrow(AudioManagerStateError); + }); + it('restarts playback when the renewal timer fires', async () => { jest.useFakeTimers(); diff --git a/tests/ffmpeg.test.ts b/tests/ffmpeg.test.ts index d54e867..9db9d11 100644 --- a/tests/ffmpeg.test.ts +++ b/tests/ffmpeg.test.ts @@ -1,5 +1,6 @@ import { jest, describe, beforeEach, afterEach, it, expect } from '@jest/globals'; import { spawn } from 'node:child_process'; +import type { ChildProcess } from 'node:child_process'; import { FfmpegProcessError } from '../src'; import { resolveFfmpegExecutable, startFfmpeg } from '../src/ffmpeg'; @@ -52,6 +53,26 @@ function createMockChildProcess(): MockChildProcess { }; } +function mockSpawnReturn(childProcess: MockChildProcess): void { + mockSpawn.mockReturnValue(childProcess as unknown as ChildProcess); +} + +function getProcessHandler(childProcess: MockChildProcess, eventName: string): (...args: unknown[]) => void { + return childProcess.once.mock.calls.find(([event]) => event === eventName)?.[1] as (...args: unknown[]) => void; +} + +function getStdoutHandler(childProcess: MockChildProcess, eventName: string): (...args: unknown[]) => void { + return childProcess.stdout.once.mock.calls.find(([event]) => event === eventName)?.[1] as ( + ...args: unknown[] + ) => void; +} + +function getStderrHandler(childProcess: MockChildProcess, eventName: string): (...args: unknown[]) => void { + return childProcess.stderr.on.mock.calls.find(([event]) => event === eventName)?.[1] as ( + ...args: unknown[] + ) => void; +} + describe('ffmpeg helpers', () => { beforeEach(() => { jest.clearAllMocks(); @@ -72,7 +93,7 @@ describe('ffmpeg helpers', () => { it('spawns ffmpeg with the default argument set', () => { const childProcess = createMockChildProcess(); - mockSpawn.mockReturnValue(childProcess); + mockSpawnReturn(childProcess); startFfmpeg('https://synradiode.stream.laut.fm/synradiode'); @@ -105,10 +126,10 @@ describe('ffmpeg helpers', () => { it('rejects readiness when ffmpeg startup emits an error', async () => { const childProcess = createMockChildProcess(); - mockSpawn.mockReturnValue(childProcess); + mockSpawnReturn(childProcess); const handle = startFfmpeg('tests/audio.mp3'); - const errorHandler = childProcess.once.mock.calls.find(([event]) => event === 'error')?.[1]; + const errorHandler = getProcessHandler(childProcess, 'error'); errorHandler(new Error('spawn ENOENT')); @@ -118,11 +139,11 @@ describe('ffmpeg helpers', () => { it('includes stderr when ffmpeg exits before producing audio', async () => { const childProcess = createMockChildProcess(); - mockSpawn.mockReturnValue(childProcess); + mockSpawnReturn(childProcess); const handle = startFfmpeg('tests/audio.mp3'); - const stderrHandler = childProcess.stderr.on.mock.calls.find(([event]) => event === 'data')?.[1]; - const exitHandler = childProcess.once.mock.calls.find(([event]) => event === 'exit')?.[1]; + const stderrHandler = getStderrHandler(childProcess, 'data'); + const exitHandler = getProcessHandler(childProcess, 'exit'); stderrHandler('invalid input'); exitHandler(1, null); @@ -130,9 +151,37 @@ describe('ffmpeg helpers', () => { await expect(handle.ready).rejects.toThrow('invalid input'); }); + it('keeps the useful tail of long ffmpeg stderr output', async () => { + const childProcess = createMockChildProcess(); + mockSpawnReturn(childProcess); + + const handle = startFfmpeg('tests/audio.mp3'); + const stderrHandler = getStderrHandler(childProcess, 'data'); + const exitHandler = getProcessHandler(childProcess, 'exit'); + + stderrHandler(`${'x'.repeat(5_000)}final diagnostic`); + exitHandler(1, null); + + await expect(handle.ready).rejects.toThrow('final diagnostic'); + }); + + it('keeps readiness resolved when ffmpeg exits after producing audio', async () => { + const childProcess = createMockChildProcess(); + mockSpawnReturn(childProcess); + + const handle = startFfmpeg('tests/audio.mp3'); + const readableHandler = getStdoutHandler(childProcess, 'readable'); + const exitHandler = getProcessHandler(childProcess, 'exit'); + + readableHandler(); + exitHandler(1, null); + + await expect(handle.ready).resolves.toBeUndefined(); + }); + it('uses custom executable and argument overrides when provided', () => { const childProcess = createMockChildProcess(); - mockSpawn.mockReturnValue(childProcess); + mockSpawnReturn(childProcess); startFfmpeg('tests/audio.mp3', { executablePath: '/custom/ffmpeg', @@ -150,7 +199,7 @@ describe('ffmpeg helpers', () => { it('stops a running child process and schedules a force kill fallback', () => { jest.useFakeTimers(); const childProcess = createMockChildProcess(); - mockSpawn.mockReturnValue(childProcess); + mockSpawnReturn(childProcess); const handle = startFfmpeg('tests/audio.mp3'); handle.stop(); @@ -168,7 +217,7 @@ describe('ffmpeg helpers', () => { it('does not signal a process that already exited', () => { const childProcess = createMockChildProcess(); childProcess.exitCode = 0; - mockSpawn.mockReturnValue(childProcess); + mockSpawnReturn(childProcess); const handle = startFfmpeg('tests/audio.mp3'); handle.stop();