diff --git a/README.md b/README.md index f972b37..8abfdbd 100644 --- a/README.md +++ b/README.md @@ -98,20 +98,24 @@ every push and gates the Docker image publish on it passing — see Dockerfile - this dev environment has no Docker - so treat the first real build as the thing to double-check, even though it mirrors exactly the `npm run build` commands already confirmed working locally for both -projects. Same caveat, doubled, for the self-hosted ffmpeg burn-in -pipeline (see docs/architecture.md): no Emby server *or* ffmpeg binary -is reachable here either, so while its pure logic (stream-index -conversion, session lifecycle) is unit-tested with `child_process.spawn` -mocked, the actual command against a real PGS file - and the exact -shape of Emby's direct-stream URL it depends on - has never run for -real. Treat the first real attempt at a Blu-ray-style subtitle as the -thing to double-check here too. Same again for the Emby-playstate sync -(the exact `PlaystateService` endpoint/body shape) and the Playlists -endpoint - three Emby-API assumptions total now waiting on a real -server to actually confirm, all otherwise built and unit-tested the -same way. The one-shared-stream-per-party work is a different kind of -unverified: its own Emby API calls were already live, confirmed code, -just newly shared instead of per-viewer - what's untested is behavior +projects. Same caveat for the self-hosted ffmpeg burn-in pipeline (see +docs/architecture.md): no ffmpeg binary is reachable here, so while its +pure logic (stream-index conversion, session lifecycle) is unit-tested +with `child_process.spawn` mocked, the actual command against a real +PGS file has never run in this environment. Treat the first real +attempt at a Blu-ray-style subtitle as the thing to double-check here. +**Resolved**: three Emby-API assumptions that used to be listed here as +unconfirmed (the ffmpeg direct-stream URL, the Playlists endpoint, the +Emby-playstate sync endpoint) were all checked directly against Emby's +own real OpenAPI spec. Two matched exactly. The third - playstate sync +- didn't: `PositionTicks`/`IsPaused` were being sent as a JSON body, +but Emby expects them as query parameters, alongside a required +`MediaSourceId` that wasn't being sent at all - meaning every progress +report was almost certainly failing outright. Fixed - see +docs/architecture.md's "Sync watch-party progress back to Emby" +section for the full story. The one-shared-stream-per-party work is a +different kind of unverified: its own Emby API calls were already live, +confirmed code, just newly shared instead of per-viewer - what's untested is behavior under real multi-viewer load (whether a dead Emby session actually surfaces as an HTTP failure the way the cache-invalidation logic assumes, how a real transcode's seek handling feels once several diff --git a/docs/architecture.md b/docs/architecture.md index 5cc3f0e..36ac952 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -1970,21 +1970,59 @@ thrown - this runs on a plain `setInterval` (~20s, alongside with no request to fail and no business ever disrupting real playback over a background sync hiccup. -**TODO(verify)**: the exact endpoint/body shape is unconfirmed against -a real Emby server - there isn't one reachable from this dev -environment, the same standing caveat as this session's other two -Emby-API assumptions (the ffmpeg direct-stream URL, the Playlists -endpoint), both of which turned out correct once actually tried. +**TODO(verify) resolved - and the original guess was wrong.** Checked +directly against Emby's own real OpenAPI spec (swagger.emby.media, +pointed at a real server) after a "watch status" bug report suggested +this wasn't actually working: `POST /Users/{UserId}/PlayingItems/{Id} +/Progress` does exist at that exact path, but the spec declares +`PositionTicks`/`IsPaused` as *query string* parameters, not body +fields - and a third parameter, `MediaSourceId`, is *required* and +wasn't being sent at all. The request body schema this endpoint does +list (`Api.OnPlaybackProgress`) covers a handful of unrelated fields +(`PlaylistIndex`/`Shuffle`/`SleepTimerMode`/...), none of them these - +so the original guess was sending `PositionTicks`/`IsPaused` somewhere +Emby never reads them, while omitting a parameter Emby requires just +to accept the call at all. In practice this almost certainly meant +every progress report was failing outright, silently (best-effort by +design - see above - so nothing user-visible ever broke loudly; only +`[watchparty:emby]`'s own status+body capture would have shown it, +and no one had gone looking until this bug report). + +Fixed: `reportProgress` now sends `MediaSourceId`/`PositionTicks`/ +`IsPaused` as query parameters, no body at all. `MediaSourceId` comes +from `PartyStreamService`'s own cache - `CatalogService +.negotiateStreamTarget` already resolves a real one for every party's +shared stream negotiation (see "One shared Emby stream per party" +below), so `PartyStreamService` now caches it alongside the target +path and exposes it read-only via a new `getCachedMediaSourceId +(partyId)`. Deliberately passive - it never triggers a negotiation of +its own - so a party where no one has started playback yet this +session simply has nothing to report; `index.ts`'s sweep skips it, the +same way it already skips a party with nothing playing at all, rather +than paying for an extra PlaybackInfo round trip on every single tick +just to get a value the party's already-negotiated stream produces for +free. + +While checking this, the app's other two Emby-API assumptions (the +ffmpeg direct-stream URL in `TranscodeService.buildSourceUrl`, and the +`/Playlists/{Id}/Items` endpoint in `CatalogService`) were re-checked +against the same real spec - both match with no missing required +parameter, consistent with them already having been tried for real +successfully. Verified: full typecheck/test/build on both projects. Unit tests cover `getCurrentPlaybackState` (video reads the host, music reads the shared queue, both branches, plus the empty-queue null case), -`setMemberEmbyToken`, `PlaystateService.reportProgress`'s exact -request shape and that a thrown failure never propagates, and -`EmbyClient.fetch`'s new body-encoding path (plus a check that a plain -GET, still every other call site, never grows a stray `Content-Type`). -No client changes at all - this is a pure server-side background -process with nothing for the browser to render or react to. +`setMemberEmbyToken`, `PlaystateService.reportProgress`'s corrected +query-string request shape (and that a thrown failure never +propagates), and `PartyStreamService.getCachedMediaSourceId`'s +cache-hit/cache-miss/dropped-on-failure behavior. `EmbyClient.fetch`'s +optional body-encoding capability stays generic plumbing (its own test +no longer claims PlaystateService as the reason it exists) - no +current call site needs it any more, but it costs nothing to keep for +whatever future caller does. No client changes at all - this is a pure +server-side background process with nothing for the browser to render +or react to. ## One shared Emby stream per party (not one per viewer) diff --git a/server/src/index.ts b/server/src/index.ts index e2f4955..595fb16 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -509,13 +509,21 @@ setInterval(() => PartyStreamService.reapIdleSessions(30 * 60 * 1000), 5 * 60 * // (periodic Progress reports only, no explicit start/stop). Best-effort: // PlaystateService.reportProgress swallows its own failures, so one bad // token or a transient Emby hiccup never stops the rest of this sweep. +// MediaSourceId comes from PartyStreamService's own cache, read-only +// (getCachedMediaSourceId never negotiates) - it's a required param on +// the real Progress endpoint (confirmed against Emby's OpenAPI spec, +// see PlaystateService.ts), and skipping a party entirely when nothing's +// cached yet (no one has hit play this session) is simpler and cheaper +// than this sweep ever negotiating a stream of its own just to get one. setInterval(() => { for (const party of PartyService.listActive()) { const state = PartyService.getCurrentPlaybackState(party); if (!state) continue; // nothing currently playing (e.g. an empty music queue) + const mediaSourceId = PartyStreamService.getCachedMediaSourceId(party.id); + if (!mediaSourceId) continue; // nothing negotiated yet this session - Progress requires it for (const member of party.members.values()) { if (!member.connection || !member.embyToken) continue; - PlaystateService.reportProgress(member.embyToken, member.userId, state.itemId, state.positionTicks, state.isPaused); + PlaystateService.reportProgress(member.embyToken, member.userId, state.itemId, mediaSourceId, state.positionTicks, state.isPaused); } } }, 20 * 1000); diff --git a/server/src/services/CatalogService.ts b/server/src/services/CatalogService.ts index a8c8eae..c4ebe18 100644 --- a/server/src/services/CatalogService.ts +++ b/server/src/services/CatalogService.ts @@ -13,6 +13,12 @@ import type { EmbyItem, EmbySession, ItemDto, LibraryDto, MediaType, PlaybackOpt // actually stream. "Voir sur Emby" always works though. const WATCH_PARTY_TYPES = new Set(['Movie', 'Episode', 'Video', 'MusicVideo', 'Audio']); +/** negotiateStreamTarget's return shape - see its own comment. */ +export interface NegotiatedStreamTarget { + targetPath: string; + mediaSourceId: string; +} + // Emby's /Views can return all sorts of things beyond actual media // libraries - boxset collections, live TV, photos, books, a mixed-content // folder with no CollectionType at all - none of which this app has any @@ -302,8 +308,16 @@ class CatalogServiceClass { * subtitles client-side, from real subtitle data instead of an * encoding option this app has no visibility into, sidesteps the * entire class of failure. + * + * Returns `mediaSourceId` alongside the target path, not just the + * path - PartyStreamService caches both, and PlaystateService's own + * Progress report needs a real MediaSourceId too (a required query + * parameter on that endpoint, confirmed against Emby's real OpenAPI + * spec - see PlaystateService.ts). Resolving it here, once per + * negotiation, means the playstate sweep never has to make its own + * extra PlaybackInfo round trip just to get it. */ - async negotiateStreamTarget(session: EmbySession, itemId: string, isAudio: boolean, audioStreamIndex: string, maxStreamingBitrate: string): Promise { + async negotiateStreamTarget(session: EmbySession, itemId: string, isAudio: boolean, audioStreamIndex: string, maxStreamingBitrate: string): Promise { // Applied to the negotiation itself, not just the final URL - a pick // could otherwise silently do nothing if Emby's negotiation didn't // apply the query-string override the same way its own web client's @@ -332,7 +346,7 @@ class CatalogServiceClass { const params = new URLSearchParams(urlQuery || ''); this.applyPlaybackOverrides(params, audioStreamIndex, maxStreamingBitrate); this.logStreamRequest(itemId, 'negotiated', params); - return `${urlPath}?${params}`; + return { targetPath: `${urlPath}?${params}`, mediaSourceId: source.Id || '' }; } const params = new URLSearchParams({ @@ -344,7 +358,7 @@ class CatalogServiceClass { }); this.applyPlaybackOverrides(params, audioStreamIndex, maxStreamingBitrate); this.logStreamRequest(itemId, 'fallback', params); - return `/${isAudio ? 'Audio' : 'Videos'}/${itemId}/master.m3u8?${params}`; + return { targetPath: `/${isAudio ? 'Audio' : 'Videos'}/${itemId}/master.m3u8?${params}`, mediaSourceId: source.Id || '' }; } /** diff --git a/server/src/services/PartyStreamService.ts b/server/src/services/PartyStreamService.ts index 60a989f..d132d86 100644 --- a/server/src/services/PartyStreamService.ts +++ b/server/src/services/PartyStreamService.ts @@ -11,6 +11,13 @@ interface StreamSession { audioStreamIndex: string; maxStreamingBitrate: string; targetPath: string; + /** Alongside targetPath, from the same negotiation - see + * CatalogService.negotiateStreamTarget's own comment. Surfaced + * read-only via getCachedMediaSourceId() below for PlaystateService's + * own Progress report, which needs it too (a required query param on + * that endpoint) but must never trigger a negotiation of its own just + * to get it - see index.ts's periodic sweep. */ + mediaSourceId: string; lastAccessUtc: number; } @@ -58,11 +65,25 @@ class PartyStreamServiceClass { return existing.targetPath; } - const targetPath = await CatalogService.negotiateStreamTarget(session, itemId, isAudio, audioStreamIndex, maxStreamingBitrate); - this.sessions.set(party.id, { itemId, audioStreamIndex, maxStreamingBitrate, targetPath, lastAccessUtc: Date.now() }); + const { targetPath, mediaSourceId } = await CatalogService.negotiateStreamTarget(session, itemId, isAudio, audioStreamIndex, maxStreamingBitrate); + this.sessions.set(party.id, { itemId, audioStreamIndex, maxStreamingBitrate, targetPath, mediaSourceId, lastAccessUtc: Date.now() }); return targetPath; } + /** + * Read-only: the current MediaSourceId behind this party's cached + * stream, or null if nothing has been negotiated yet this session + * (e.g. the party just started and no one has hit play). Deliberately + * does NOT call getOrNegotiate/touch lastAccessUtc - this is a passive + * lookup for index.ts's periodic playstate sweep, not real streaming + * traffic, and must never itself keep an otherwise-idle session alive + * (see reapIdleSessions) or trigger a fresh Emby negotiation on its + * own timer tick. + */ + getCachedMediaSourceId(partyId: string): string | null { + return this.sessions.get(partyId)?.mediaSourceId ?? null; + } + /** GET /api/catalog/party-stream/:partyId/master.m3u8 - the caller * (index.ts) has already checked the requester is a member of `party`. */ async serveMasterPlaylist(req: Request, res: Response, party: Party, session: EmbySession): Promise { diff --git a/server/src/services/PlaystateService.ts b/server/src/services/PlaystateService.ts index ce6f2f9..5cd540b 100644 --- a/server/src/services/PlaystateService.ts +++ b/server/src/services/PlaystateService.ts @@ -15,11 +15,17 @@ import { Logger } from '../Logger'; * * Emby's own "PlaystateService" (the controller name in both Emby's and * Jellyfin's server source) is what exposes this route. - * TODO(verify): unconfirmed against a real Emby server - there isn't - * one reachable from this dev environment, same standing caveat as this - * app's other two recent Emby-API assumptions (the ffmpeg direct-stream - * URL, the Playlists endpoint) - both turned out fine once actually - * tried. + * + * Endpoint/param shape CONFIRMED against Emby's own real OpenAPI spec + * (swagger.emby.media, pointed at a real server) - the original guess + * here was wrong in a way that plausibly broke this entire feature: + * PositionTicks/IsPaused were sent as a JSON body, but Emby declares + * them (and a REQUIRED MediaSourceId this app wasn't sending at all) as + * *query string* parameters instead - the request body schema + * (Api.OnPlaybackProgress) covers a handful of unrelated fields + * (PlaylistIndex/Shuffle/SleepTimerMode/...), none of them these. A + * missing required query parameter almost certainly meant every one of + * these calls was failing outright. */ class PlaystateServiceClass { /** @@ -27,14 +33,20 @@ class PlaystateServiceClass { * a 4xx/5xx) is caught and logged here, never thrown - this runs on a * timer with no request to fail, and one member's bad token or one * transient Emby hiccup must never stop the rest of the sweep, let - * alone touch real playback. + * alone touch real playback. `mediaSourceId` comes from + * PartyStreamService's own cache (see index.ts's sweep) - resolving a + * fresh one here would mean an extra PlaybackInfo round trip on every + * tick, for a value the party's already-negotiated stream already + * produced for free. */ - async reportProgress(embyToken: string, userId: string, itemId: string, positionTicks: number, isPaused: boolean): Promise { + async reportProgress(embyToken: string, userId: string, itemId: string, mediaSourceId: string, positionTicks: number, isPaused: boolean): Promise { try { - await EmbyClient.fetch(`/Users/${userId}/PlayingItems/${itemId}/Progress`, embyToken, { - method: 'POST', - body: { PositionTicks: Math.round(positionTicks), IsPaused: isPaused } + const params = new URLSearchParams({ + MediaSourceId: mediaSourceId, + PositionTicks: String(Math.round(positionTicks)), + IsPaused: String(isPaused) }); + await EmbyClient.fetch(`/Users/${userId}/PlayingItems/${itemId}/Progress?${params}`, embyToken, { method: 'POST' }); } catch (err) { Logger.playstate(`progress report failed userId=${userId} itemId=${itemId}: ${(err as Error).message}`); } diff --git a/server/test/catalogService.test.ts b/server/test/catalogService.test.ts index a3e19dc..0b607d2 100644 --- a/server/test/catalogService.test.ts +++ b/server/test/catalogService.test.ts @@ -47,36 +47,37 @@ describe('CatalogService', () => { PlaySessionId: 'sess1' })); - const target = await CatalogService.negotiateStreamTarget(session, 'item1', false, '2', ''); + const { targetPath, mediaSourceId } = await CatalogService.negotiateStreamTarget(session, 'item1', false, '2', ''); - assert.ok(target.includes('AudioStreamIndex=2')); - assert.ok(!target.includes('Subtitle'), 'this method never touches subtitle params at all'); + assert.ok(targetPath.includes('AudioStreamIndex=2')); + assert.ok(!targetPath.includes('Subtitle'), 'this method never touches subtitle params at all'); + assert.equal(mediaSourceId, 'src1', 'PlaystateService\'s own Progress report needs this too - see PartyStreamService'); }); it('negotiateStreamTarget\'s hand-built fallback picks /Audio/ (and drops VideoCodec) when isAudio is true', async (t) => { t.mock.method(EmbyClient, 'fetch', async () => jsonResponse({ MediaSources: [{ Id: 'src2' }], PlaySessionId: 'sess2' })); - const target = await CatalogService.negotiateStreamTarget(session, 'track1', true, '', ''); + const { targetPath } = await CatalogService.negotiateStreamTarget(session, 'track1', true, '', ''); - assert.ok(target.startsWith('/Audio/track1/master.m3u8')); - assert.ok(!target.includes('VideoCodec')); + assert.ok(targetPath.startsWith('/Audio/track1/master.m3u8')); + assert.ok(!targetPath.includes('VideoCodec')); }); it('negotiateStreamTarget\'s hand-built fallback picks /Videos/ (with VideoCodec) when isAudio is false', async (t) => { t.mock.method(EmbyClient, 'fetch', async () => jsonResponse({ MediaSources: [{ Id: 'src3' }], PlaySessionId: 'sess3' })); - const target = await CatalogService.negotiateStreamTarget(session, 'movie1', false, '', ''); + const { targetPath } = await CatalogService.negotiateStreamTarget(session, 'movie1', false, '', ''); - assert.ok(target.startsWith('/Videos/movie1/master.m3u8')); - assert.ok(target.includes('VideoCodec=h264')); + assert.ok(targetPath.startsWith('/Videos/movie1/master.m3u8')); + assert.ok(targetPath.includes('VideoCodec=h264')); }); it('negotiateStreamTarget applies a quality (MaxStreamingBitrate) pick the same way as an audio pick', async (t) => { t.mock.method(EmbyClient, 'fetch', async () => jsonResponse({ MediaSources: [{ Id: 'src4' }], PlaySessionId: 'sess4' })); - const target = await CatalogService.negotiateStreamTarget(session, 'movie2', false, '', '4000000'); + const { targetPath } = await CatalogService.negotiateStreamTarget(session, 'movie2', false, '', '4000000'); - assert.ok(target.includes('MaxStreamingBitrate=4000000')); + assert.ok(targetPath.includes('MaxStreamingBitrate=4000000')); }); it('negotiateStreamTarget fails cleanly (not a crash) when Emby has no media source for this item', async (t) => { diff --git a/server/test/embyClient.test.ts b/server/test/embyClient.test.ts index 37c1427..3c7eb85 100644 --- a/server/test/embyClient.test.ts +++ b/server/test/embyClient.test.ts @@ -50,21 +50,27 @@ describe('EmbyClient.fetch', () => { assert.equal(logged.filter((l) => l.includes('[watchparty:emby]')).length, 0); }); - it('JSON-encodes a body and sets method/Content-Type - PlaystateService\'s own POST calls need this', async (t) => { + it('JSON-encodes a body and sets method/Content-Type when a caller passes one - generic plumbing', async (t) => { + // Not PlaystateService's own call any more - its real Progress + // endpoint takes query params, not a body (confirmed against Emby's + // real OpenAPI spec, see PlaystateService.ts) - but `fetch` still + // supports an optional body generically, for whatever future caller + // does need one, so this exercises that capability on its own terms + // with an arbitrary example shape. let capturedInit: RequestInit | undefined; t.mock.method(globalThis, 'fetch', async (_url: string, init: RequestInit) => { capturedInit = init; return new Response('{}', { status: 200 }); }); - await EmbyClient.fetch('/Users/u1/PlayingItems/item1/Progress', 'tok', { + await EmbyClient.fetch('/Some/Endpoint', 'tok', { method: 'POST', - body: { PositionTicks: 123, IsPaused: false } + body: { Foo: 123, Bar: false } }); assert.equal(capturedInit?.method, 'POST'); assert.equal((capturedInit?.headers as Record)['Content-Type'], 'application/json'); - assert.equal(capturedInit?.body, JSON.stringify({ PositionTicks: 123, IsPaused: false })); + assert.equal(capturedInit?.body, JSON.stringify({ Foo: 123, Bar: false })); }); it('a plain GET (no body) never gets a Content-Type header - every existing call site relies on this staying untouched', async (t) => { diff --git a/server/test/partyStreamService.test.ts b/server/test/partyStreamService.test.ts index 017ca29..45562b1 100644 --- a/server/test/partyStreamService.test.ts +++ b/server/test/partyStreamService.test.ts @@ -40,7 +40,7 @@ describe('PartyStreamService.getOrNegotiate', () => { let negotiateCount = 0; t.mock.method(CatalogService, 'negotiateStreamTarget', async () => { negotiateCount += 1; - return '/Videos/movie1/master.m3u8?PlaySessionId=sess1'; + return { targetPath: '/Videos/movie1/master.m3u8?PlaySessionId=sess1', mediaSourceId: 'src1' }; }); const party = PartyService.create('u-alice', 'Alice', 'movie1', 'Un Film', '/img/movie1', null, 'video'); @@ -49,6 +49,7 @@ describe('PartyStreamService.getOrNegotiate', () => { assert.equal(negotiateCount, 1, 'the second call must reuse the cached target, not negotiate again'); assert.equal(first, second); + assert.equal(PartyStreamService.getCachedMediaSourceId(party.id), 'src1', 'cached alongside the target - see PlaystateService'); PartyStreamService.endSession(party.id); }); @@ -57,7 +58,7 @@ describe('PartyStreamService.getOrNegotiate', () => { let negotiateCount = 0; t.mock.method(CatalogService, 'negotiateStreamTarget', async () => { negotiateCount += 1; - return `/Videos/movie2/master.m3u8?call=${negotiateCount}`; + return { targetPath: `/Videos/movie2/master.m3u8?call=${negotiateCount}`, mediaSourceId: `src${negotiateCount}` }; }); const party = PartyService.create('u-alice', 'Alice', 'movie2', 'Un Film 2', '/img/movie2', null, 'video'); @@ -76,7 +77,7 @@ describe('PartyStreamService.getOrNegotiate', () => { let negotiateCount = 0; t.mock.method(CatalogService, 'negotiateStreamTarget', async () => { negotiateCount += 1; - return `/Videos/x/master.m3u8?call=${negotiateCount}`; + return { targetPath: `/Videos/x/master.m3u8?call=${negotiateCount}`, mediaSourceId: `src${negotiateCount}` }; }); const party = PartyService.create('u-alice', 'Alice', 'ep1', 'S01E01', '/img/ep1', [ @@ -95,7 +96,7 @@ describe('PartyStreamService.getOrNegotiate', () => { let capturedArgs: unknown[] = []; t.mock.method(CatalogService, 'negotiateStreamTarget', async (...args: unknown[]) => { capturedArgs = args; - return '/Audio/t1/master.m3u8'; + return { targetPath: '/Audio/t1/master.m3u8', mediaSourceId: 'src-t1' }; }); const party = PartyService.create('u-alice', 'Alice', 't1', 'Track One', '/img/t1', null, 'music'); @@ -122,7 +123,7 @@ describe('PartyStreamService failure recovery', () => { let negotiateCount = 0; t.mock.method(CatalogService, 'negotiateStreamTarget', async () => { negotiateCount += 1; - return `/Videos/movie3/master.m3u8?call=${negotiateCount}`; + return { targetPath: `/Videos/movie3/master.m3u8?call=${negotiateCount}`, mediaSourceId: `src${negotiateCount}` }; }); t.mock.method(EmbyClient, 'proxyResponse', async (_req: Request, res: Response) => { (res as unknown as { statusCode: number }).statusCode = 502; // simulate Emby having ended this session on its own @@ -130,6 +131,7 @@ describe('PartyStreamService failure recovery', () => { const party = PartyService.create('u-alice', 'Alice', 'movie3', 'Un Film 3', '/img/movie3', null, 'video'); await PartyStreamService.serveMasterPlaylist(fakeReq(), fakeRes(), party, FAKE_SESSION); + assert.equal(PartyStreamService.getCachedMediaSourceId(party.id), null, 'a failed response must drop the MediaSourceId too, not just the target path'); await PartyStreamService.serveMasterPlaylist(fakeReq(), fakeRes(), party, FAKE_SESSION); assert.equal(negotiateCount, 2, 'a failed response must not be cached - the next request has to renegotiate'); @@ -141,7 +143,7 @@ describe('PartyStreamService failure recovery', () => { let negotiateCount = 0; t.mock.method(CatalogService, 'negotiateStreamTarget', async () => { negotiateCount += 1; - return '/Videos/movie4/master.m3u8'; + return { targetPath: '/Videos/movie4/master.m3u8', mediaSourceId: 'src-movie4' }; }); t.mock.method(EmbyClient, 'proxyResponse', async () => {}); // res.statusCode stays 200, the fakeRes() default @@ -160,11 +162,15 @@ describe('PartyStreamService lifecycle', () => { assert.doesNotThrow(() => PartyStreamService.endSession('no-such-party')); }); + it('getCachedMediaSourceId is null for a party that never negotiated a stream - never negotiates one itself', () => { + assert.equal(PartyStreamService.getCachedMediaSourceId('no-such-party'), null); + }); + it('reapIdleSessions ends sessions past the idle cutoff, leaves a fresh one alone', async (t) => { let negotiateCount = 0; t.mock.method(CatalogService, 'negotiateStreamTarget', async () => { negotiateCount += 1; - return `/Videos/x/master.m3u8?call=${negotiateCount}`; + return { targetPath: `/Videos/x/master.m3u8?call=${negotiateCount}`, mediaSourceId: `src${negotiateCount}` }; }); const party = PartyService.create('u-eve', 'Eve', 'movie5', 'Un Film 5', '/img/movie5', null, 'video'); diff --git a/server/test/playstateService.test.ts b/server/test/playstateService.test.ts index 48b70c5..6c52b70 100644 --- a/server/test/playstateService.test.ts +++ b/server/test/playstateService.test.ts @@ -8,7 +8,7 @@ import { PlaystateService } from '../src/services/PlaystateService'; import { EmbyClient } from '../src/emby/EmbyClient'; describe('PlaystateService.reportProgress', () => { - it('POSTs to this user\'s own PlayingItems/Progress with their own token', async (t) => { + it('POSTs to this user\'s own PlayingItems/Progress with their own token, params in the query string not a body', async (t) => { let capturedPath = ''; let capturedToken = ''; let capturedOptions: unknown; @@ -19,28 +19,37 @@ describe('PlaystateService.reportProgress', () => { return new Response('{}', { status: 200 }); }); - await PlaystateService.reportProgress('tok-alice', 'u-alice', 'item1', 12_345_678, false); + await PlaystateService.reportProgress('tok-alice', 'u-alice', 'item1', 'src1', 12_345_678, false); - assert.equal(capturedPath, '/Users/u-alice/PlayingItems/item1/Progress'); + // Query params, not a JSON body - confirmed against Emby's real + // OpenAPI spec (see PlaystateService.ts's own comment): the + // original body-based guess was missing a required MediaSourceId + // entirely and would have failed outright. + const [path, query] = capturedPath.split('?'); + assert.equal(path, '/Users/u-alice/PlayingItems/item1/Progress'); + assert.equal(new URLSearchParams(query).get('MediaSourceId'), 'src1'); + assert.equal(new URLSearchParams(query).get('PositionTicks'), '12345678'); + assert.equal(new URLSearchParams(query).get('IsPaused'), 'false'); assert.equal(capturedToken, 'tok-alice'); - assert.deepEqual(capturedOptions, { method: 'POST', body: { PositionTicks: 12345678, IsPaused: false } }); + assert.deepEqual(capturedOptions, { method: 'POST' }, 'no body at all - the real body schema (Api.OnPlaybackProgress) has none of these fields'); }); it('rounds a fractional positionTicks (Emby expects an integer)', async (t) => { - let capturedOptions: { body?: { PositionTicks?: number } } | undefined; - t.mock.method(EmbyClient, 'fetch', async (_path: string, _token: string, options: typeof capturedOptions) => { - capturedOptions = options; + let capturedPath = ''; + t.mock.method(EmbyClient, 'fetch', async (path: string) => { + capturedPath = path; return new Response('{}', { status: 200 }); }); - await PlaystateService.reportProgress('tok', 'u1', 'item1', 100.7, true); + await PlaystateService.reportProgress('tok', 'u1', 'item1', 'src1', 100.7, true); - assert.equal(capturedOptions?.body?.PositionTicks, 101); + const query = capturedPath.split('?')[1]; + assert.equal(new URLSearchParams(query).get('PositionTicks'), '101'); }); it('swallows a failure instead of throwing - this runs on a timer with nothing to catch it', async (t) => { t.mock.method(EmbyClient, 'fetch', async () => { throw new Error('Emby unreachable'); }); - await assert.doesNotReject(() => PlaystateService.reportProgress('tok', 'u1', 'item1', 0, false)); + await assert.doesNotReject(() => PlaystateService.reportProgress('tok', 'u1', 'item1', 'src1', 0, false)); }); });