Fix slow catalog browsing: images were never cacheable

Reported directly ("l'api du contenu des catalogues est hyper lente").
Timed CatalogService's own listing calls directly against the real
server - a 300-item library listing came back in well under half a
second, so that wasn't it. Confirmed the real cost instead:
EmbyClient.proxyResponse (what every thumbnail goes through, via
CatalogService.proxyImage) was silently dropping every caching signal
Emby sent. Confirmed directly against the real server that an image
response already carries Cache-Control: public and a real ETag, but
this method only ever forwarded content-type/content-range/
accept-ranges. With no validator and no cache policy ever reaching the
browser, every one of potentially 300+ tiles in a library grid had to
be re-downloaded in full, from scratch, on every single visit.

Fixed both directions:
- EmbyClient.fetch now also accepts ifNoneMatch/ifModifiedSince, sent
  as If-None-Match/If-Modified-Since - and a 304 (the whole point of
  sending those) is explicitly excluded from the failure-logging path,
  since !res.ok is true for it but it's Emby correctly saying
  "unchanged", not an error.
- proxyResponse reads the browser's own If-None-Match/If-Modified-Since
  request headers and forwards them to Emby, and forwards
  cache-control/etag/last-modified back from Emby's response,
  alongside the existing content-range/accept-ranges pair.

Confirmed end to end against the real server, not just reasoned about:
a plain image GET comes back 200 with Cache-Control: public + a real
ETag; the same request repeated with that ETag as If-None-Match comes
back a real 304 with a zero-byte body. Harmless for everything else
this method proxies (video/audio segments, playlists, subtitles) - it
only forwards whatever Emby itself decided, never invents a policy.

New EmbyClient.proxyResponse tests (never exercised directly before -
every existing test that touches it mocks it away entirely) cover
forwarding the cache headers on a real bodied response, forwarding the
browser's conditional headers to Emby and relaying a real 304 with no
body, and that a response with neither header doesn't choke or invent
one. New EmbyClient.fetch tests cover a 304 not hitting the failure
log and the two new headers being sent only when given. Full
typecheck/test/build green (101/101 tests). docs/architecture.md and
README.md updated.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

README.md

@@ -279,6 +279,9 @@ Then put it behind your existing reverse proxy like everything else
       (was still each viewer's own local choice until now); fixed the
       audio picker briefly showing an empty option on first load — see
       docs/architecture.md
+- [x] Fixed catalog browsing feeling slow: image thumbnails were never
+      cacheable (Emby's own Cache-Control/ETag were silently dropped) —
+      see docs/architecture.md
 - [ ] Try a full party end-to-end with two real accounts (still the main gap)
 - [ ] Tune transcoding target / support direct play when possible
 

docs/architecture.md

@@ -2286,6 +2286,56 @@ select restores the party's current text or burn-in pick correctly on
 load; and the audio select now lands on a real first option instead of
 an empty one when `streamAudioIndex` is `null`.
 
+## Catalog browsing was slow: images were never cacheable
+
+Reported directly ("l'api du contenu des catalogues est hyper lente").
+`CatalogService`'s own listing methods (`getLibraryItems`, `getArtists`,
+etc.) were already fine - each is exactly one Emby call, no per-item
+follow-up requests, and timed directly against the real server a
+300-item library listing (`getLibraryItems`'s own `Limit: 300`) came
+back in well under half a second. The real cost was downstream of that:
+every one of those up to 300 tiles needs its own thumbnail, each
+proxied through `EmbyClient.proxyImage` → `proxyResponse`, and
+`proxyResponse` was silently dropping every caching signal Emby sent -
+confirmed directly against the real server that an image response
+already carries `Cache-Control: public` and a real `ETag`, but this
+method only ever forwarded `content-type`/`content-range`/
+`accept-ranges`. With no validator and no cache policy ever reaching
+the browser, it had no choice but to re-download every single thumbnail
+in full, from scratch, on every visit to every library, home-page
+carousel, series/season/album page - the same images, over and over,
+compounding across however many tiles a page shows.
+
+**Fixed both directions of the same problem.** `EmbyClient.fetch`
+already accepted `range` to forward; it now also accepts
+`ifNoneMatch`/`ifModifiedSince`, sent as `If-None-Match`/
+`If-Modified-Since` - and a `304` (the entire point of sending those)
+is explicitly excluded from the failure-logging path (`!res.ok` is true
+for a 304, but it's Emby correctly saying "unchanged", not an error
+worth `[watchparty:emby]`). `proxyResponse` now reads the browser's own
+`If-None-Match`/`If-Modified-Since` request headers and forwards them
+to Emby, and forwards `cache-control`/`etag`/`last-modified` back from
+Emby's response, alongside the existing `content-range`/`accept-ranges`
+pair. Confirmed end to end against the real server, not just reasoned
+about: a plain image GET comes back `200` with `Cache-Control: public`
++ a real `ETag`; the *same* request repeated with that `ETag` as
+`If-None-Match` comes back a real `304` with a zero-byte body. Harmless
+for everything else this method proxies (video/audio segments, HLS
+playlists, subtitles) - it only ever forwards whatever Emby itself
+decided, never invents a policy, so something Emby doesn't mark
+cacheable (a per-session transcode) simply never gets these headers set
+at all, same as before.
+
+Verified: full typecheck/test/build on both projects. New
+`EmbyClient.proxyResponse` tests (this method had never been exercised
+directly before - every existing test that touches it mocks it away
+entirely to isolate whatever calls it) cover forwarding the cache
+headers on a real bodied response, forwarding the browser's conditional
+headers to Emby and relaying a real 304 with no body, and that a
+response with neither header (most proxied paths) doesn't choke or
+invent one; `EmbyClient.fetch` tests cover a 304 not hitting the
+failure log and the two new headers being sent only when given.
+
 ## Known limitations
 
 - Playback always goes through `hls.js` against whatever Emby's

server/src/emby/EmbyClient.ts

@@ -14,25 +14,34 @@ import { Logger } from '../Logger';
  */
 export class EmbyClient {
   /** Fetches a path from Emby with the given user's token, forwarding
-   *  Range (needed for direct-play video seeking) when the browser sent one.
+   *  Range (needed for direct-play video seeking) when the browser sent
-   *  `method`/`body` are for PlaystateService's own POST calls (see
+   *  one, and If-None-Match/If-Modified-Since (needed for images to
-   *  PlaystateService.ts) - every other existing call site only ever
+   *  revalidate as a cheap 304 instead of re-downloading the full body
-   *  passed `range`, so this stays a plain GET for them, unaffected.
+   *  every time - see proxyResponse's own comment on caching) when the
-   *  `body`, when given, is JSON-encoded here so every caller passes a
+   *  browser sent those. `method`/`body` are for PlaystateService's own
-   *  plain object rather than remembering to stringify it themselves.
+   *  POST calls (see PlaystateService.ts) - every other existing call
-   *  Every Emby call in this app goes through here - see logErrorResponse
+   *  site only ever passed `range`, so this stays a plain GET for them,
-   *  below for what happens when Emby answers with anything but success. */
+   *  unaffected. `body`, when given, is JSON-encoded here so every
-  static async fetch(pathAndQuery: string, token: string, options: { range?: string; method?: string; body?: unknown } = {}): Promise<globalThis.Response> {
+   *  caller passes a plain object rather than remembering to stringify
+   *  it themselves. Every Emby call in this app goes through here - see
+   *  logErrorResponse below for what happens when Emby answers with
+   *  anything but success. */
+  static async fetch(pathAndQuery: string, token: string, options: { range?: string; ifNoneMatch?: string; ifModifiedSince?: string; method?: string; body?: unknown } = {}): Promise<globalThis.Response> {
     const url = pathAndQuery.startsWith('http') ? pathAndQuery : `${EMBY_SERVER_URL}${pathAndQuery}`;
     const headers: Record<string, string> = { 'X-Emby-Token': token };
     if (options.range) headers.Range = options.range;
+    if (options.ifNoneMatch) headers['If-None-Match'] = options.ifNoneMatch;
+    if (options.ifModifiedSince) headers['If-Modified-Since'] = options.ifModifiedSince;
     let body: string | undefined;
     if (options.body !== undefined) {
       headers['Content-Type'] = 'application/json';
       body = JSON.stringify(options.body);
     }
     const res = await fetch(url, { headers, method: options.method, body });
-    if (!res.ok) await EmbyClient.logErrorResponse(res, pathAndQuery);
+    // A 304 (the whole point of the conditional headers above) is
+    // Emby correctly saying "unchanged" - not a failure worth this
+    // app's own error log, even though !res.ok is true for it.
+    if (!res.ok && res.status !== 304) await EmbyClient.logErrorResponse(res, pathAndQuery);
     return res;
   }
 
@@ -92,21 +101,44 @@ export class EmbyClient {
 
   /**
    * Proxies an Emby response to an Express response: forwards status,
-   * content-type, and the headers that make byte-range seeking work
+   * content-type, the headers that make byte-range seeking work
-   * (content-range/accept-ranges), rewriting HLS playlists in transit and
+   * (content-range/accept-ranges), and the ones that make caching work
-   * streaming everything else (images, video/audio segments) through
+   * (cache-control/etag/last-modified - see below), rewriting HLS
-   * untouched - no buffering, so this doesn't hold a whole segment in
+   * playlists in transit and streaming everything else (images, video/
-   * memory. `routeKey` is only ever read back out by `rewritePlaylist`
+   * audio segments) through untouched - no buffering, so this doesn't
-   * above - a real itemId for `proxyImage`/`proxySubtitle` (which never
+   * hold a whole segment in memory. `routeKey` is only ever read back
-   * actually reach that branch - wrong content-type/extension - so it's
+   * out by `rewritePlaylist` above - a real itemId for `proxyImage`/
-   * inert for them either way) and `party.id` for `PartyStreamService`,
+   * `proxySubtitle` (which never actually reach that branch - wrong
-   * whose whole point is routing every member back to the *same* shared
+   * content-type/extension - so it's inert for them either way) and
-   * stream regardless of which item they think they're watching.
+   * `party.id` for `PartyStreamService`, whose whole point is routing
+   * every member back to the *same* shared stream regardless of which
+   * item they think they're watching.
+   *
+   * Caching: added after a "the catalog is really slow" report traced
+   * to this - confirmed directly against the real server that Emby
+   * already answers an image request with `Cache-Control: public` and a
+   * real `ETag`, but neither ever reached the browser: this method used
+   * to forward only content-type/content-range/accept-ranges, silently
+   * dropping both, so every tile in every library grid re-downloaded
+   * its full image on every single visit with no way to even ask Emby
+   * "has this changed?". Now forwards cache-control/etag/last-modified
+   * too, and forwards the browser's own conditional headers (If-None-
+   * Match/If-Modified-Since - see EmbyClient.fetch above) through to
+   * Emby on the way there, so a cache hit round-trips as a real,
+   * bodyless 304 instead of the full image every time. Harmless for the
+   * other things this method proxies (playlists/segments/subtitles) -
+   * this only ever forwards whatever Emby itself already decided, never
+   * invents a policy, so a transcoded segment Emby marks non-cacheable
+   * stays exactly that.
    */
   static async proxyResponse(req: Request, res: Response, pathAndQuery: string, token: string, routeKey: string): Promise<void> {
     let embyRes: globalThis.Response;
     try {
-      embyRes = await EmbyClient.fetch(pathAndQuery, token, { range: req.headers.range });
+      embyRes = await EmbyClient.fetch(pathAndQuery, token, {
+        range: req.headers.range,
+        ifNoneMatch: req.headers['if-none-match'],
+        ifModifiedSince: req.headers['if-modified-since']
+      });
     } catch {
       res.status(502).json({ error: 'Serveur Emby injoignable.' });
       return;
@@ -115,7 +147,7 @@ export class EmbyClient {
     res.status(embyRes.status);
     const contentType = embyRes.headers.get('content-type') || '';
     if (contentType) res.setHeader('Content-Type', contentType);
-    for (const h of ['content-range', 'accept-ranges']) {
+    for (const h of ['content-range', 'accept-ranges', 'cache-control', 'etag', 'last-modified']) {
       const v = embyRes.headers.get(h);
       if (v) res.setHeader(h, v);
     }

server/test/embyClient.test.ts

@@ -6,8 +6,55 @@
 // what EmbyClient.fetch itself calls.
 import { describe, it } from 'node:test';
 import assert from 'node:assert/strict';
+import { PassThrough } from 'stream';
+import type { Request, Response } from 'express';
 import { EmbyClient } from '../src/emby/EmbyClient';
 
+/** A real Writable (via PassThrough) so `.pipe(res)` in proxyResponse's
+ *  streaming branch works exactly as it would against a real Express
+ *  response, decorated with the handful of Express Response methods
+ *  proxyResponse actually calls - same "just enough of the real
+ *  interface, cast through `unknown`" approach as partyStreamService
+ *  .test.ts's own fakeRes, but pipe-able too since, unlike that file,
+ *  EmbyClient.proxyResponse itself (not a mock of it) runs for real
+ *  here. Returned already cast to plain `Response` for passing into
+ *  proxyResponse; callers read back `.statusCode`/`.headers`/`.body`
+ *  through the same `unknown` cast the fake was built from. */
+type FakeRes = { statusCode: number; headers: Record<string, string>; body: Promise<string> };
+
+function fakeRes(): Response {
+  const stream = new PassThrough();
+  const chunks: Buffer[] = [];
+  stream.on('data', (c) => chunks.push(c));
+  const body = new Promise<string>((resolve) => stream.on('end', () => resolve(Buffer.concat(chunks).toString())));
+  const headers: Record<string, string> = {};
+  // Untyped (`any`) for the construction phase only - trying to keep this
+  // typed as the full `Response & FakeRes` intersection while assigning
+  // its methods one at a time confuses TS into thinking each assignment
+  // narrows the *whole* intersection, not just the one property. `inspect`
+  // and the `Response` return type below are the only two shapes anything
+  // outside this function ever sees it as.
+  const fake: any = stream;
+  fake.headers = headers;
+  fake.statusCode = 200;
+  fake.body = body;
+  fake.status = (code: number) => { fake.statusCode = code; return fake; };
+  fake.setHeader = (name: string, value: string) => { headers[name.toLowerCase()] = value; return fake; };
+  fake.send = (text: string) => { stream.end(text); return fake; };
+  return fake as Response;
+}
+
+/** Reads back the tracking fields fakeRes() attached, through the same
+ *  `unknown` cast - `res` itself is typed as plain `Response` at every
+ *  call site (matching what proxyResponse's own signature expects). */
+function inspect(res: Response): FakeRes {
+  return res as unknown as FakeRes;
+}
+
+function fakeReq(headers: Record<string, string> = {}): Request {
+  return { headers } as unknown as Request;
+}
+
 describe('EmbyClient.fetch', () => {
   it('logs Emby\'s status and response body on a failure, without consuming the body the real caller still needs', async (t) => {
     const logged: string[] = [];
@@ -50,6 +97,34 @@ describe('EmbyClient.fetch', () => {
     assert.equal(logged.filter((l) => l.includes('[watchparty:emby]')).length, 0);
   });
 
+  it('does not log a 304 as a failure - a cache hit is the whole point of sending If-None-Match, not an error', async (t) => {
+    const logged: string[] = [];
+    t.mock.method(console, 'log', (line: string) => { logged.push(line); });
+    t.mock.method(globalThis, 'fetch', async () => new Response(null, { status: 304 }));
+
+    await EmbyClient.fetch('/Items/1/Images/Primary', 'tok', { ifNoneMatch: '"abc"' });
+
+    assert.equal(logged.filter((l) => l.includes('[watchparty:emby]')).length, 0, 'a 304 is !res.ok but must not hit the failure log');
+  });
+
+  it('sends If-None-Match/If-Modified-Since only when given - a plain GET stays exactly as before', async (t) => {
+    let capturedInit: RequestInit | undefined;
+    t.mock.method(globalThis, 'fetch', async (_url: string, init: RequestInit) => {
+      capturedInit = init;
+      return new Response('{}', { status: 200 });
+    });
+
+    await EmbyClient.fetch('/Items/1/Images/Primary', 'tok', { ifNoneMatch: '"abc"', ifModifiedSince: 'Mon, 01 Jan 2024 00:00:00 GMT' });
+    let headers = capturedInit?.headers as Record<string, string>;
+    assert.equal(headers['If-None-Match'], '"abc"');
+    assert.equal(headers['If-Modified-Since'], 'Mon, 01 Jan 2024 00:00:00 GMT');
+
+    await EmbyClient.fetch('/Users/u1/Views', 'tok');
+    headers = capturedInit?.headers as Record<string, string>;
+    assert.equal(headers['If-None-Match'], undefined);
+    assert.equal(headers['If-Modified-Since'], undefined);
+  });
+
   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
@@ -86,3 +161,66 @@ describe('EmbyClient.fetch', () => {
     assert.equal((capturedInit?.headers as Record<string, string>)['Content-Type'], undefined);
   });
 });
+
+// proxyResponse itself, never exercised for real before (every other test
+// file that touches it mocks it away entirely, to isolate whatever calls
+// it) - specifically the caching path added after a real "the catalog is
+// really slow" report traced to this method silently dropping Emby's own
+// Cache-Control/ETag on every image it proxied. Mocks the GLOBAL fetch,
+// same as EmbyClient.fetch's own tests above - proxyResponse is real code
+// running on top of it, not itself replaced.
+describe('EmbyClient.proxyResponse', () => {
+  it('forwards Cache-Control/ETag/Last-Modified from a real (bodied) response, alongside the existing content-range/accept-ranges', async (t) => {
+    t.mock.method(globalThis, 'fetch', async () => new Response('fake-jpeg-bytes', {
+      status: 200,
+      headers: {
+        'Content-Type': 'image/jpeg',
+        'Cache-Control': 'public',
+        'ETag': '"abc123"',
+        'Accept-Ranges': 'bytes'
+      }
+    }));
+
+    const res = fakeRes();
+    await EmbyClient.proxyResponse(fakeReq(), res, '/Items/1/Images/Primary', 'tok', '1');
+    const got = inspect(res);
+
+    assert.equal(got.statusCode, 200);
+    assert.equal(got.headers['cache-control'], 'public');
+    assert.equal(got.headers['etag'], '"abc123"');
+    assert.equal(got.headers['accept-ranges'], 'bytes');
+    assert.equal(await got.body, 'fake-jpeg-bytes', 'the body itself must still stream through untouched');
+  });
+
+  it('forwards the browser\'s own If-None-Match/If-Modified-Since to Emby, and relays a real 304 with no body', async (t) => {
+    let capturedInit: RequestInit | undefined;
+    t.mock.method(globalThis, 'fetch', async (_url: string, init: RequestInit) => {
+      capturedInit = init;
+      return new Response(null, { status: 304, headers: { ETag: '"abc123"' } });
+    });
+
+    const req = fakeReq({ 'if-none-match': '"abc123"', 'if-modified-since': 'Mon, 01 Jan 2024 00:00:00 GMT' });
+    const res = fakeRes();
+    await EmbyClient.proxyResponse(req, res, '/Items/1/Images/Primary', 'tok', '1');
+    const got = inspect(res);
+
+    const sentHeaders = capturedInit?.headers as Record<string, string>;
+    assert.equal(sentHeaders['If-None-Match'], '"abc123"', 'the browser\'s validator must reach Emby, or Emby can never actually answer 304');
+    assert.equal(sentHeaders['If-Modified-Since'], 'Mon, 01 Jan 2024 00:00:00 GMT');
+    assert.equal(got.statusCode, 304);
+    assert.equal(got.headers['etag'], '"abc123"');
+  });
+
+  it('does not choke on no Cache-Control/ETag at all - most proxied responses (video segments, subtitles) never had one', async (t) => {
+    t.mock.method(globalThis, 'fetch', async () => new Response('vtt content', { status: 200, headers: { 'Content-Type': 'text/vtt' } }));
+
+    const res = fakeRes();
+    await EmbyClient.proxyResponse(fakeReq(), res, '/Videos/1/x/Subtitles/2/Stream.vtt', 'tok', '1');
+    const got = inspect(res);
+
+    assert.equal(got.statusCode, 200);
+    assert.equal(got.headers['cache-control'], undefined);
+    assert.equal(got.headers['etag'], undefined);
+    assert.equal(await got.body, 'vtt content');
+  });
+});