From 0579d253c4e9a1273423c1be72cd76a3bef8bf4b Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 26 Jul 2026 19:49:17 +0200 Subject: [PATCH] fix(electron): fail closed on unknown performance completions --- ...reload-performance-correlation.complete.ts | 19 +- ...performance-correlation.completion.spec.ts | 192 ++++++++++++++++++ libs/m3u-state/src/lib/actions.ts | 5 +- ...=> playlist-update.effect-handler.spec.ts} | 2 +- .../src/lib/electron-api.interface.ts | 10 +- 5 files changed, 222 insertions(+), 6 deletions(-) rename libs/m3u-state/src/lib/{playlist-update.effect.spec.ts => playlist-update.effect-handler.spec.ts} (95%) diff --git a/apps/electron-backend/src/app/api/preload-performance-correlation.complete.ts b/apps/electron-backend/src/app/api/preload-performance-correlation.complete.ts index b27e327b3..e8dc4e699 100644 --- a/apps/electron-backend/src/app/api/preload-performance-correlation.complete.ts +++ b/apps/electron-backend/src/app/api/preload-performance-correlation.complete.ts @@ -168,12 +168,27 @@ export function completePreloadPerformanceCall( ): PreloadPerformanceMarkerMetadata { const call = calls.get(event.ipcCallId); if (!call) { + const invalidSequence = + playlistId === null + ? undefined + : invalidatePreloadPerformanceSequence( + calls, + sequences, + playlistId, + PRELOAD_PERFORMANCE_INVALID_REASON.OUT_OF_ORDER + ); + closeInvalidPreloadPerformanceSequenceWhenSettled( + calls, + sequences, + playlistId + ); return createPreloadPerformanceMarkerMetadata( event, playlistId, - operationId, + invalidSequence?.operationId ?? operationId, PRELOAD_PERFORMANCE_CORRELATION_STATE.INVALID, - PRELOAD_PERFORMANCE_INVALID_REASON.OUT_OF_ORDER + invalidSequence?.invalidReason ?? + PRELOAD_PERFORMANCE_INVALID_REASON.OUT_OF_ORDER ); } diff --git a/apps/electron-backend/src/app/api/preload-performance-correlation.completion.spec.ts b/apps/electron-backend/src/app/api/preload-performance-correlation.completion.spec.ts index 6cde5c2cc..e10251ddd 100644 --- a/apps/electron-backend/src/app/api/preload-performance-correlation.completion.spec.ts +++ b/apps/electron-backend/src/app/api/preload-performance-correlation.completion.spec.ts @@ -8,9 +8,48 @@ import { OPERATION_ONE, OPERATION_TWO, PLAYLIST_ONE, + PLAYLIST_TWO, type CorrelationHarness, } from './preload-performance-correlation.test-helpers'; +function completeDatabaseSequence( + advance: CorrelationHarness['advance'], + firstCallId: number, + playlistId: string, + operationId: string +) { + return [ + advance({ + ipcCallId: firstCallId, + method: PRELOAD_PERFORMANCE_METHOD.DB_GET_APP_PLAYLIST, + operationId, + phase: 'start', + playlistId, + }), + advance({ + ipcCallId: firstCallId, + method: PRELOAD_PERFORMANCE_METHOD.DB_GET_APP_PLAYLIST, + operationId, + phase: 'success', + playlistId, + }), + advance({ + ipcCallId: firstCallId + 1, + method: PRELOAD_PERFORMANCE_METHOD.DB_UPSERT_APP_PLAYLIST, + operationId, + phase: 'start', + playlistId, + }), + advance({ + ipcCallId: firstCallId + 1, + method: PRELOAD_PERFORMANCE_METHOD.DB_UPSERT_APP_PLAYLIST, + operationId, + phase: 'success', + playlistId, + }), + ]; +} + describe('preload performance marker correlation completions', () => { let advance: CorrelationHarness['advance']; @@ -18,6 +57,159 @@ describe('preload performance marker correlation completions', () => { ({ advance } = createCorrelationHarness()); }); + it.each(['success', 'error'] as const)( + 'invalidates only the matching playlist after an unknown call ID %s', + (phase) => { + advance({ + ipcCallId: 1, + method: PRELOAD_PERFORMANCE_METHOD.REFRESH_PLAYLIST, + operationId: OPERATION_ONE, + phase: 'start', + playlistId: PLAYLIST_ONE, + }); + advance({ + ipcCallId: 10, + method: PRELOAD_PERFORMANCE_METHOD.REFRESH_PLAYLIST, + operationId: OPERATION_TWO, + phase: 'start', + playlistId: PLAYLIST_TWO, + }); + + const unknownCompletion = advance({ + ipcCallId: 99, + method: PRELOAD_PERFORMANCE_METHOD.REFRESH_PLAYLIST, + operationId: OPERATION_TWO, + phase, + playlistId: PLAYLIST_ONE, + }); + const matchingCompletion = advance({ + ipcCallId: 1, + method: PRELOAD_PERFORMANCE_METHOD.REFRESH_PLAYLIST, + operationId: OPERATION_ONE, + phase: 'success', + playlistId: PLAYLIST_ONE, + }); + const samePlaylistPersistence = completeDatabaseSequence( + advance, + 20, + PLAYLIST_ONE, + OPERATION_ONE + ); + + const unrelatedRefreshCompletion = advance({ + ipcCallId: 10, + method: PRELOAD_PERFORMANCE_METHOD.REFRESH_PLAYLIST, + operationId: OPERATION_TWO, + phase: 'success', + playlistId: PLAYLIST_TWO, + }); + const unrelatedPersistence = completeDatabaseSequence( + advance, + 30, + PLAYLIST_TWO, + OPERATION_TWO + ); + + expect(unknownCompletion).toMatchObject({ + correlationState: PRELOAD_PERFORMANCE_CORRELATION_STATE.INVALID, + invalidReason: PRELOAD_PERFORMANCE_INVALID_REASON.OUT_OF_ORDER, + operationId: OPERATION_ONE, + playlistId: PLAYLIST_ONE, + }); + expect(matchingCompletion).toMatchObject({ + correlationState: PRELOAD_PERFORMANCE_CORRELATION_STATE.INVALID, + invalidReason: PRELOAD_PERFORMANCE_INVALID_REASON.OUT_OF_ORDER, + }); + expect( + [matchingCompletion, ...samePlaylistPersistence].some( + ({ correlationState }) => + correlationState === + PRELOAD_PERFORMANCE_CORRELATION_STATE.COMPLETE + ) + ).toBe(false); + expect(unrelatedRefreshCompletion.correlationState).toBe( + PRELOAD_PERFORMANCE_CORRELATION_STATE.CORRELATED + ); + expect(unrelatedPersistence.at(-1)).toMatchObject({ + correlationState: + PRELOAD_PERFORMANCE_CORRELATION_STATE.COMPLETE, + operationId: OPERATION_TWO, + playlistId: PLAYLIST_TWO, + }); + } + ); + + it('invalidates persistence after a duplicate completion without touching another playlist', () => { + advance({ + ipcCallId: 1, + method: PRELOAD_PERFORMANCE_METHOD.REFRESH_PLAYLIST, + operationId: OPERATION_ONE, + phase: 'start', + playlistId: PLAYLIST_ONE, + }); + advance({ + ipcCallId: 10, + method: PRELOAD_PERFORMANCE_METHOD.REFRESH_PLAYLIST, + operationId: OPERATION_TWO, + phase: 'start', + playlistId: PLAYLIST_TWO, + }); + advance({ + ipcCallId: 1, + method: PRELOAD_PERFORMANCE_METHOD.REFRESH_PLAYLIST, + operationId: OPERATION_ONE, + phase: 'success', + playlistId: PLAYLIST_ONE, + }); + + const duplicateCompletion = advance({ + ipcCallId: 1, + method: PRELOAD_PERFORMANCE_METHOD.REFRESH_PLAYLIST, + operationId: OPERATION_ONE, + phase: 'success', + playlistId: PLAYLIST_ONE, + }); + const samePlaylistPersistence = completeDatabaseSequence( + advance, + 20, + PLAYLIST_ONE, + OPERATION_ONE + ); + + advance({ + ipcCallId: 10, + method: PRELOAD_PERFORMANCE_METHOD.REFRESH_PLAYLIST, + operationId: OPERATION_TWO, + phase: 'success', + playlistId: PLAYLIST_TWO, + }); + const unrelatedPersistence = completeDatabaseSequence( + advance, + 30, + PLAYLIST_TWO, + OPERATION_TWO + ); + + expect(duplicateCompletion).toMatchObject({ + correlationState: PRELOAD_PERFORMANCE_CORRELATION_STATE.INVALID, + invalidReason: PRELOAD_PERFORMANCE_INVALID_REASON.OUT_OF_ORDER, + operationId: OPERATION_ONE, + playlistId: PLAYLIST_ONE, + }); + expect( + samePlaylistPersistence.some( + ({ correlationState }) => + correlationState === + PRELOAD_PERFORMANCE_CORRELATION_STATE.COMPLETE + ) + ).toBe(false); + expect(unrelatedPersistence.at(-1)).toMatchObject({ + correlationState: PRELOAD_PERFORMANCE_CORRELATION_STATE.COMPLETE, + operationId: OPERATION_TWO, + playlistId: PLAYLIST_TWO, + }); + }); + it('invalidates a correlated sequence when a target call errors', () => { advance({ ipcCallId: 1, diff --git a/libs/m3u-state/src/lib/actions.ts b/libs/m3u-state/src/lib/actions.ts index 3ff7df791..3f6008509 100644 --- a/libs/m3u-state/src/lib/actions.ts +++ b/libs/m3u-state/src/lib/actions.ts @@ -16,7 +16,10 @@ export const PlaylistActions = createActionGroup({ 'Remove Playlist': props<{ playlistId: string }>(), 'Update Playlist Meta': props<{ playlist: PlaylistMeta }>(), 'Update Playlist': props<{ - /** Instrumentation-only; never persisted or sent to main/worker IPC. */ + /** + * Instrumentation-only; stripped before the DB invoke. + * Never enters the DB worker payload or persisted playlist data. + */ operationId?: string; playlist: Playlist; playlistId: string; diff --git a/libs/m3u-state/src/lib/playlist-update.effect.spec.ts b/libs/m3u-state/src/lib/playlist-update.effect-handler.spec.ts similarity index 95% rename from libs/m3u-state/src/lib/playlist-update.effect.spec.ts rename to libs/m3u-state/src/lib/playlist-update.effect-handler.spec.ts index 6041442a3..3e80ff0d4 100644 --- a/libs/m3u-state/src/lib/playlist-update.effect.spec.ts +++ b/libs/m3u-state/src/lib/playlist-update.effect-handler.spec.ts @@ -3,7 +3,7 @@ import { of } from 'rxjs'; import { PlaylistActions } from './actions'; import { persistPlaylistUpdate } from './playlist-update.effect-handler'; -describe('PlaylistEffects updatePlaylist', () => { +describe('persistPlaylistUpdate', () => { it('forwards the refresh operation ID to playlist persistence', () => { const playlistsService = { updatePlaylist: jest.fn(() => of(undefined)), diff --git a/libs/shared/interfaces/src/lib/electron-api.interface.ts b/libs/shared/interfaces/src/lib/electron-api.interface.ts index 3fd7dec36..de3695b46 100644 --- a/libs/shared/interfaces/src/lib/electron-api.interface.ts +++ b/libs/shared/interfaces/src/lib/electron-api.interface.ts @@ -686,7 +686,10 @@ export interface ElectronBridgeApi { ) => Promise; dbUpsertAppPlaylist: ( playlist: Playlist, - /** Instrumentation-only; never persisted or sent to main/worker IPC. */ + /** + * Instrumentation-only; stripped before the DB invoke. + * Never enters the DB worker payload or persisted playlist data. + */ operationId?: string ) => Promise; dbUpsertAppPlaylists: ( @@ -696,7 +699,10 @@ export interface ElectronBridgeApi { dbGetAppPlaylistMetas: () => Promise; dbGetAppPlaylist: ( playlistId: string, - /** Instrumentation-only; never persisted or sent to main/worker IPC. */ + /** + * Instrumentation-only; stripped before the DB invoke. + * Never enters the DB worker payload or persisted playlist data. + */ operationId?: string ) => Promise; dbGetAppPlaylistFavoriteChannels: (