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>
226 lines
11 KiB
TypeScript
226 lines
11 KiB
TypeScript
// EmbyClient.fetch itself (not mocked away, unlike catalogService.test.ts,
|
|
// which replaces it wholesale to isolate CatalogService) - specifically
|
|
// the error-logging path added after several rounds of not being able to
|
|
// see what Emby actually said on a failure (see docs/architecture.md's
|
|
// player-track-selection section). Mocks the GLOBAL fetch, since that's
|
|
// 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[] = [];
|
|
t.mock.method(console, 'log', (line: string) => { logged.push(line); });
|
|
t.mock.method(globalThis, 'fetch', async () => new Response('Server.MediaEncoding.Api boom', { status: 500 }));
|
|
|
|
const res = await EmbyClient.fetch('/Videos/91988/hls1/main/0.ts?PlaySessionId=abc', 'tok');
|
|
|
|
const embyLines = logged.filter((l) => l.includes('[watchparty:emby]'));
|
|
assert.equal(embyLines.length, 1);
|
|
assert.ok(embyLines[0]!.includes('500'));
|
|
assert.ok(embyLines[0]!.includes('Server.MediaEncoding.Api boom'), 'the real body text should make it into the log line');
|
|
assert.ok(embyLines[0]!.includes('/Videos/91988/hls1/main/0.ts'));
|
|
|
|
// The caller (proxyResponse/fetchJson) still needs to read this same
|
|
// response's body itself - logErrorResponse must read a *clone*, not
|
|
// the original, or this would come back empty/throw.
|
|
assert.equal(await res.text(), 'Server.MediaEncoding.Api boom');
|
|
});
|
|
|
|
it('redacts api_key from the logged path - Emby\'s own TranscodingUrl can carry one', async (t) => {
|
|
const logged: string[] = [];
|
|
t.mock.method(console, 'log', (line: string) => { logged.push(line); });
|
|
t.mock.method(globalThis, 'fetch', async () => new Response('nope', { status: 502 }));
|
|
|
|
await EmbyClient.fetch('/Videos/1/master.m3u8?api_key=super-secret&MediaSourceId=x', 'tok');
|
|
|
|
const line = logged.find((l) => l.includes('[watchparty:emby]'))!;
|
|
assert.ok(!line.includes('super-secret'), 'the real key must never reach the log');
|
|
assert.ok(line.includes('api_key=REDACTED'));
|
|
});
|
|
|
|
it('logs nothing extra on a successful response - only failures are interesting', async (t) => {
|
|
const logged: string[] = [];
|
|
t.mock.method(console, 'log', (line: string) => { logged.push(line); });
|
|
t.mock.method(globalThis, 'fetch', async () => new Response('{}', { status: 200 }));
|
|
|
|
await EmbyClient.fetch('/Users/u1/Views', 'tok');
|
|
|
|
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
|
|
// 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('/Some/Endpoint', 'tok', {
|
|
method: 'POST',
|
|
body: { Foo: 123, Bar: false }
|
|
});
|
|
|
|
assert.equal(capturedInit?.method, 'POST');
|
|
assert.equal((capturedInit?.headers as Record<string, string>)['Content-Type'], 'application/json');
|
|
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) => {
|
|
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/Views', 'tok');
|
|
|
|
assert.equal(capturedInit?.method, undefined);
|
|
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');
|
|
});
|
|
});
|