Fix CodeRabbit review feedback

This commit is contained in:
2026-05-23 22:22:22 -07:00
parent afe1731514
commit dc9d7b77bb
13 changed files with 198 additions and 36 deletions
@@ -82,8 +82,9 @@ test('playback handler drives mpv commands and playback state', async () => {
setLastProgressAtMs: (value) => calls.push(`progress:${value}`),
reportPlaying: (payload) => reportPayloads.push(payload as Record<string, unknown>),
showMpvOsd: (text) => calls.push(`osd:${text}`),
recordJellyfinPlaybackMetadata: (metadata) =>
statsMetadata.push(metadata as Record<string, unknown>),
recordJellyfinPlaybackMetadata: (metadata) => {
statsMetadata.push(metadata as Record<string, unknown>);
},
});
await handler({
@@ -164,7 +165,9 @@ test('playback handler publishes Jellyfin title before loading tokenized stream
setLastProgressAtMs: () => {},
reportPlaying: () => {},
showMpvOsd: () => {},
updateCurrentMediaTitle: (title) => timeline.push(`title:${title}`),
updateCurrentMediaTitle: (title) => {
timeline.push(`title:${title}`);
},
});
await handler({
@@ -353,3 +356,51 @@ test('playback handler does not let media title failures block playback startup'
assert.deepEqual(commands[1], ['loadfile', 'https://stream.example/video.m3u8', 'replace']);
});
test('playback handler handles rejected best-effort hook promises', async () => {
const commands: Array<Array<string | number>> = [];
const handler = createPlayJellyfinItemInMpvHandler({
ensureMpvConnectedForPlayback: async () => true,
getMpvClient: () => ({ connected: true, send: () => {} }),
resolvePlaybackPlan: async () => ({
url: 'https://stream.example/video.m3u8',
mode: 'direct',
title: 'Episode 5',
itemTitle: 'Episode 5',
seriesTitle: null,
seasonNumber: null,
episodeNumber: null,
startTimeTicks: 0,
audioStreamIndex: null,
subtitleStreamIndex: null,
}),
applyJellyfinMpvDefaults: () => {},
showVisibleOverlay: () => {},
sendMpvCommand: (command) => commands.push(command),
armQuitOnDisconnect: () => {},
schedule: () => {},
convertTicksToSeconds: (ticks) => ticks / 10_000_000,
preloadExternalSubtitles: () => {},
setActivePlayback: () => {},
setLastProgressAtMs: () => {},
reportPlaying: () => {},
showMpvOsd: () => {},
updateCurrentMediaTitle: async () => {
throw new Error('title async unavailable');
},
recordJellyfinPlaybackMetadata: async () => {
throw new Error('stats async unavailable');
},
});
await handler({
session: baseSession,
clientInfo: baseClientInfo,
jellyfinConfig: {},
itemId: 'item-5',
});
await Promise.resolve();
await Promise.resolve();
assert.deepEqual(commands[1], ['loadfile', 'https://stream.example/video.m3u8', 'replace']);
});
+16 -8
View File
@@ -28,6 +28,14 @@ export type JellyfinPlaybackStatsMetadata = {
itemId: string;
};
function runBestEffortPlaybackHook(callback: () => void | Promise<void>): void {
try {
void Promise.resolve(callback()).catch(() => {});
} catch {
// Best-effort metadata/title hooks must not block playback startup.
}
}
function applyStartTimeTicksToPlaybackUrl(url: string, startTimeTicksOverride?: number): string {
if (typeof startTimeTicksOverride !== 'number') return url;
try {
@@ -78,8 +86,10 @@ export function createPlayJellyfinItemInMpvHandler(deps: {
eventName: 'start';
}) => void;
showMpvOsd: (text: string) => void;
recordJellyfinPlaybackMetadata?: (metadata: JellyfinPlaybackStatsMetadata) => void;
updateCurrentMediaTitle?: (title: string) => void;
recordJellyfinPlaybackMetadata?: (
metadata: JellyfinPlaybackStatsMetadata,
) => void | Promise<void>;
updateCurrentMediaTitle?: (title: string) => void | Promise<void>;
}) {
return async (params: {
session: JellyfinAuthSession;
@@ -112,8 +122,8 @@ export function createPlayJellyfinItemInMpvHandler(deps: {
deps.sendMpvCommand(['set_property', 'sub-auto', 'no']);
const playbackUrl = applyStartTimeTicksToPlaybackUrl(plan.url, params.startTimeTicksOverride);
const playMethod = plan.mode === 'direct' ? 'DirectPlay' : 'Transcode';
try {
deps.updateCurrentMediaTitle?.(plan.title);
runBestEffortPlaybackHook(() => deps.updateCurrentMediaTitle?.(plan.title));
runBestEffortPlaybackHook(() =>
deps.recordJellyfinPlaybackMetadata?.({
mediaPath: playbackUrl,
displayTitle: plan.title,
@@ -122,10 +132,8 @@ export function createPlayJellyfinItemInMpvHandler(deps: {
seasonNumber: plan.seasonNumber,
episodeNumber: plan.episodeNumber,
itemId: params.itemId,
});
} catch {
// Best-effort metadata/title hooks must not block playback startup.
}
}),
);
deps.setActivePlayback({
itemId: params.itemId,
mediaSourceId: undefined,
@@ -1,6 +1,7 @@
import test from 'node:test';
import assert from 'node:assert/strict';
import {
markJellyfinRemotePlaybackLoaded,
createReportJellyfinRemoteProgressHandler,
createReportJellyfinRemoteStoppedHandler,
secondsToJellyfinTicks,
@@ -335,6 +336,21 @@ test('createReportJellyfinRemoteStoppedHandler ignores unloaded active playback'
assert.equal(cleared, false);
});
test('markJellyfinRemotePlaybackLoaded preserves the loaded marker on unload paths', () => {
const playback = {
itemId: 'item-2',
playMethod: 'Transcode' as const,
loadedMediaPath: 'https://stream.example/video.m3u8',
};
markJellyfinRemotePlaybackLoaded(playback, '');
markJellyfinRemotePlaybackLoaded(playback, ' ');
assert.equal(playback.loadedMediaPath, 'https://stream.example/video.m3u8');
markJellyfinRemotePlaybackLoaded(playback, ' https://stream.example/next.m3u8 ');
assert.equal(playback.loadedMediaPath, 'https://stream.example/next.m3u8');
});
test('createReportJellyfinRemoteStoppedHandler ignores startup stop churn before grace expires', async () => {
let cleared = false;
let stopped = false;
@@ -34,6 +34,16 @@ export function secondsToJellyfinTicks(seconds: number, ticksPerSecond: number):
return Math.max(0, Math.floor(seconds * ticksPerSecond));
}
export function markJellyfinRemotePlaybackLoaded(
playback: ActiveJellyfinRemotePlaybackState | null,
path: string,
): void {
const normalizedPath = path.trim();
if (playback && normalizedPath) {
playback.loadedMediaPath = normalizedPath;
}
}
function isMpvPauseEnabled(value: unknown): boolean {
if (typeof value === 'boolean') return value;
if (typeof value === 'number') return value !== 0;