From 2be42257ab06436012764efa5fc7754b3b492d7a Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Wed, 10 Jun 2026 12:06:13 +0200 Subject: [PATCH] fix(playback): harden embedded MPV polling loop and IPC error visibility (#1041) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(playback): harden embedded MPV polling loop and IPC error visibility The 500ms session polling interval called refreshSession() unguarded: a throwing addon call (getAddon/getSessionSnapshot) escaped the interval callback as an uncaughtException in the main process on every tick. Wrap each refresh in try-catch, log the first failure only, and resume session updates once the addon recovers. Embedded MPV IPC handlers also forwarded service calls without any error handling, unlike every other events module. The renderer swallows these rejections by design (guardIpc), so addon errors were completely invisible. Route all registrations through a wrapper that logs the failing channel in the main process before rethrowing. Co-Authored-By: Claude Fable 5 * fix(playback): track poll-failure log suppression per session A healthy session in the same poll tick reset the shared pollFailureLogged flag before the failing session was processed, so a mixed healthy/failing session set logged the failure on every 500ms tick — the flooding the flag was meant to prevent. Track logged failures per session id instead and clean entries up on dispose. Addresses Greptile/Codex review feedback on #1041. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: 4gray Co-authored-by: Claude Fable 5 --- .../app/events/embedded-mpv.events.spec.ts | 82 +++++++++++++++++ .../src/app/events/embedded-mpv.events.ts | 88 +++++++++++-------- .../embedded-mpv-native.service.spec.ts | 46 ++++++++++ .../services/embedded-mpv-native.service.ts | 21 ++++- 4 files changed, 200 insertions(+), 37 deletions(-) create mode 100644 apps/electron-backend/src/app/events/embedded-mpv.events.spec.ts diff --git a/apps/electron-backend/src/app/events/embedded-mpv.events.spec.ts b/apps/electron-backend/src/app/events/embedded-mpv.events.spec.ts new file mode 100644 index 000000000..cd086ee7c --- /dev/null +++ b/apps/electron-backend/src/app/events/embedded-mpv.events.spec.ts @@ -0,0 +1,82 @@ +jest.mock('electron', () => ({ + ipcMain: { + handle: jest.fn(), + }, +})); + +const mockEmbeddedMpvService = { + getSupport: jest.fn(), + setPaused: jest.fn(), +}; + +jest.mock('../services/embedded-mpv-native.service', () => ({ + EmbeddedMpvNativeService: class {}, + embeddedMpvNativeService: mockEmbeddedMpvService, +})); + +import { ipcMain } from 'electron'; +import { + EMBEDDED_MPV_SET_PAUSED, + EMBEDDED_MPV_SUPPORT, +} from '@iptvnator/shared/interfaces'; +import './embedded-mpv.events'; + +function getIpcMainHandler( + channel: string +): (...args: unknown[]) => Promise { + const handleMock = ipcMain.handle as unknown as jest.Mock; + const calls = handleMock.mock.calls as Array< + [string, (...args: unknown[]) => Promise] + >; + const match = calls.find( + ([registeredChannel]) => registeredChannel === channel + ); + + if (!match) { + throw new Error(`Missing ipcMain handler for ${channel}`); + } + + return match[1]; +} + +describe('EmbeddedMpvEvents IPC handlers', () => { + beforeEach(() => { + mockEmbeddedMpvService.getSupport.mockReset(); + mockEmbeddedMpvService.setPaused.mockReset(); + }); + + it('forwards arguments to the native service and returns its result', async () => { + const session = { id: 'session-1', status: 'paused' }; + mockEmbeddedMpvService.setPaused.mockReturnValue(session); + + const handler = getIpcMainHandler(EMBEDDED_MPV_SET_PAUSED); + + await expect(handler({}, 'session-1', true)).resolves.toEqual(session); + expect(mockEmbeddedMpvService.setPaused).toHaveBeenCalledWith( + 'session-1', + true + ); + }); + + it('logs in the main process and rethrows when the native service throws', async () => { + const consoleErrorSpy = jest + .spyOn(console, 'error') + .mockImplementation(); + + try { + mockEmbeddedMpvService.getSupport.mockImplementation(() => { + throw new Error('addon failed to load'); + }); + + const handler = getIpcMainHandler(EMBEDDED_MPV_SUPPORT); + + await expect(handler({})).rejects.toThrow('addon failed to load'); + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.stringContaining(EMBEDDED_MPV_SUPPORT), + expect.any(Error) + ); + } finally { + consoleErrorSpy.mockRestore(); + } + }); +}); diff --git a/apps/electron-backend/src/app/events/embedded-mpv.events.ts b/apps/electron-backend/src/app/events/embedded-mpv.events.ts index 35f6e2cca..307544024 100644 --- a/apps/electron-backend/src/app/events/embedded-mpv.events.ts +++ b/apps/electron-backend/src/app/events/embedded-mpv.events.ts @@ -36,93 +36,109 @@ function getService(): EmbeddedMpvNativeService { return embeddedMpvNativeService; } -ipcMain.handle(EMBEDDED_MPV_SUPPORT, () => getService().getSupport()); +/** + * Registers an embedded-MPV IPC handler that logs failures in the main + * process before rethrowing them to the renderer. The renderer swallows + * these rejections (the next session snapshot resyncs its state), so + * without main-side logging addon errors would be invisible. + */ +function handleEmbeddedMpv( + channel: string, + handler: (...args: Args) => unknown +): void { + ipcMain.handle(channel, async (_event, ...args: unknown[]) => { + try { + return await handler(...(args as Args)); + } catch (error) { + console.error( + `[Embedded MPV] ${channel} handler failed:`, + error + ); + throw error; + } + }); +} -ipcMain.handle(EMBEDDED_MPV_PREPARE, () => getService().prepareAddon()); +handleEmbeddedMpv(EMBEDDED_MPV_SUPPORT, () => getService().getSupport()); -ipcMain.handle( +handleEmbeddedMpv(EMBEDDED_MPV_PREPARE, () => getService().prepareAddon()); + +handleEmbeddedMpv( EMBEDDED_MPV_CREATE_SESSION, - ( - _event, - bounds: EmbeddedMpvBounds, - title?: string, - initialVolume?: number - ) => getService().createSession(bounds, title, initialVolume) + (bounds: EmbeddedMpvBounds, title?: string, initialVolume?: number) => + getService().createSession(bounds, title, initialVolume) ); -ipcMain.handle( +handleEmbeddedMpv( EMBEDDED_MPV_LOAD_PLAYBACK, - (_event, sessionId: string, playback: ResolvedPortalPlayback) => + (sessionId: string, playback: ResolvedPortalPlayback) => getService().loadPlayback(sessionId, playback) ); -ipcMain.handle( +handleEmbeddedMpv( EMBEDDED_MPV_SET_BOUNDS, - (_event, sessionId: string, bounds: EmbeddedMpvBounds) => + (sessionId: string, bounds: EmbeddedMpvBounds) => getService().setBounds(sessionId, bounds) ); -ipcMain.handle( +handleEmbeddedMpv( EMBEDDED_MPV_SET_PAUSED, - (_event, sessionId: string, paused: boolean) => + (sessionId: string, paused: boolean) => getService().setPaused(sessionId, paused) ); -ipcMain.handle( - EMBEDDED_MPV_SEEK, - (_event, sessionId: string, seconds: number) => - getService().seek(sessionId, seconds) +handleEmbeddedMpv(EMBEDDED_MPV_SEEK, (sessionId: string, seconds: number) => + getService().seek(sessionId, seconds) ); -ipcMain.handle( +handleEmbeddedMpv( EMBEDDED_MPV_SET_VOLUME, - (_event, sessionId: string, volume: number) => + (sessionId: string, volume: number) => getService().setVolume(sessionId, volume) ); -ipcMain.handle( +handleEmbeddedMpv( EMBEDDED_MPV_SET_AUDIO_TRACK, - (_event, sessionId: string, trackId: number) => + (sessionId: string, trackId: number) => getService().setAudioTrack(sessionId, trackId) ); -ipcMain.handle( +handleEmbeddedMpv( EMBEDDED_MPV_SET_SUBTITLE_TRACK, - (_event, sessionId: string, trackId: number) => + (sessionId: string, trackId: number) => getService().setSubtitleTrack(sessionId, trackId) ); -ipcMain.handle( +handleEmbeddedMpv( EMBEDDED_MPV_SET_SPEED, - (_event, sessionId: string, speed: number) => - getService().setSpeed(sessionId, speed) + (sessionId: string, speed: number) => getService().setSpeed(sessionId, speed) ); -ipcMain.handle( +handleEmbeddedMpv( EMBEDDED_MPV_SET_ASPECT, - (_event, sessionId: string, aspect: string) => + (sessionId: string, aspect: string) => getService().setAspect(sessionId, aspect) ); -ipcMain.handle( +handleEmbeddedMpv( EMBEDDED_MPV_START_RECORDING, - (_event, sessionId: string, options: EmbeddedMpvRecordingStartOptions) => + (sessionId: string, options: EmbeddedMpvRecordingStartOptions) => getService().startRecording(sessionId, options) ); -ipcMain.handle(EMBEDDED_MPV_STOP_RECORDING, (_event, sessionId: string) => +handleEmbeddedMpv(EMBEDDED_MPV_STOP_RECORDING, (sessionId: string) => getService().stopRecording(sessionId) ); -ipcMain.handle(EMBEDDED_MPV_GET_DEFAULT_RECORDING_FOLDER, () => +handleEmbeddedMpv(EMBEDDED_MPV_GET_DEFAULT_RECORDING_FOLDER, () => getService().getDefaultRecordingFolder() ); -ipcMain.handle(EMBEDDED_MPV_SELECT_RECORDING_FOLDER, () => +handleEmbeddedMpv(EMBEDDED_MPV_SELECT_RECORDING_FOLDER, () => getService().selectRecordingFolder() ); -ipcMain.handle(EMBEDDED_MPV_DISPOSE_SESSION, (_event, sessionId: string) => +handleEmbeddedMpv(EMBEDDED_MPV_DISPOSE_SESSION, (sessionId: string) => getService().disposeSession(sessionId) ); diff --git a/apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts b/apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts index 7acbf57a2..8dff12386 100644 --- a/apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts +++ b/apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts @@ -169,6 +169,52 @@ describe('EmbeddedMpvNativeService power blocker', () => { expect(powerSaveBlockerMock.start).not.toHaveBeenCalled(); }); + it('keeps the polling timer alive when refreshing a session throws', () => { + jest.useFakeTimers(); + const consoleErrorSpy = jest + .spyOn(console, 'error') + .mockImplementation(); + + try { + startSession('s1', snapshot('playing')); + startSession('s2', snapshot('playing')); + + // s1 keeps failing while s2 stays healthy: the healthy session + // must not reset the log suppression for the failing one. + addon.getSessionSnapshot.mockImplementation( + (sessionId: string) => { + if (sessionId === 's1') { + throw new Error('addon crashed'); + } + return snapshot('playing'); + } + ); + + // Three poll ticks: nothing may escape the interval callback, + // and the failure is logged once instead of at poll rate. + expect(() => jest.advanceTimersByTime(1500)).not.toThrow(); + expect(consoleErrorSpy).toHaveBeenCalledTimes(1); + + // Once the addon recovers, session updates flow again. + addon.getSessionSnapshot.mockImplementation(() => + snapshot('playing', { positionSeconds: 42 }) + ); + mainWindowSendMock.mockClear(); + jest.advanceTimersByTime(500); + expect(mainWindowSendMock).toHaveBeenCalled(); + + // A recovered session that fails again logs once more (one line + // per session per failure streak, not one per service lifetime). + addon.getSessionSnapshot.mockImplementation(() => { + throw new Error('addon crashed again'); + }); + jest.advanceTimersByTime(1000); + expect(consoleErrorSpy).toHaveBeenCalledTimes(3); + } finally { + consoleErrorSpy.mockRestore(); + } + }); + it('acquires a single prevent-display-sleep blocker once a session is playing', () => { startSession('s1', snapshot('loading')); diff --git a/apps/electron-backend/src/app/services/embedded-mpv-native.service.ts b/apps/electron-backend/src/app/services/embedded-mpv-native.service.ts index a4d915eca..7b9e9ca08 100644 --- a/apps/electron-backend/src/app/services/embedded-mpv-native.service.ts +++ b/apps/electron-backend/src/app/services/embedded-mpv-native.service.ts @@ -93,6 +93,7 @@ export class EmbeddedMpvNativeService { private addonLoadError: Error | null = null; private readonly sessions = new Map(); private pollingTimer: NodeJS.Timeout | null = null; + private readonly pollFailuresLogged = new Set(); private powerBlockerId: number | null = null; private readonly loadAddonModule = createRequire(__filename); @@ -465,6 +466,7 @@ export class EmbeddedMpvNativeService { addon.disposeSession(sessionId); } finally { this.sessions.delete(sessionId); + this.pollFailuresLogged.delete(sessionId); const payload: EmbeddedMpvSession = { id: session.id, title: session.title, @@ -507,7 +509,24 @@ export class EmbeddedMpvNativeService { this.pollingTimer = setInterval(() => { [...this.sessions.keys()].forEach((sessionId) => { - this.refreshSession(sessionId); + // An addon-side throw must not escape the interval callback: + // it would surface as an uncaughtException in the main + // process every 500 ms while the timer keeps running. Log + // each session's first failure only — tracked per session so + // a healthy session in the same tick cannot reset the + // suppression for a failing one. + try { + this.refreshSession(sessionId); + this.pollFailuresLogged.delete(sessionId); + } catch (error) { + if (!this.pollFailuresLogged.has(sessionId)) { + this.pollFailuresLogged.add(sessionId); + console.error( + `[Embedded MPV] Polling session "${sessionId}" failed:`, + error + ); + } + } }); }, 500); }