From 00f91c2fc788397474b089a1522e6a2247418bc4 Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Wed, 8 Jul 2026 11:31:08 -0400 Subject: [PATCH] fix(search): bound the Meili settings/arming fetch with a timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit applyMeiliSettings' PATCH ran unbounded. On the cron path it fires inside ensureInit BEFORE the ingest hard-timeout race, so a stalling (not erroring) Meili endpoint would hang the whole tick past the 55s backstop and starve notify/drip; the xrpc handler ran the same unbounded fetch on user requests. Thread an AbortSignal.timeout (SETTINGS_TIMEOUT_MS=8s, overridable for tests) through applyMeiliSettings so a stall degrades to "sink disabled this cycle" (ensureInit's existing try/catch), retried next tick. Upsert/remove stay unbounded — they run only inside contrail.ingest, already under the cron race. --- .../src/lib/search/server/meili-sink.test.ts | 35 +++++++++- apps/web/src/lib/search/server/meili-sink.ts | 66 +++++++++++++------ 2 files changed, 80 insertions(+), 21 deletions(-) diff --git a/apps/web/src/lib/search/server/meili-sink.test.ts b/apps/web/src/lib/search/server/meili-sink.test.ts index 32d1d00..99f1f78 100644 --- a/apps/web/src/lib/search/server/meili-sink.test.ts +++ b/apps/web/src/lib/search/server/meili-sink.test.ts @@ -236,7 +236,11 @@ describe('createMeiliSink onRecords', () => { it('applies index settings once, before the first write (fresh-index safety)', async () => { const { fn, calls } = fakeFetch(); - const sink = createMeiliSink(() => BACKEND, () => null, fn); + const sink = createMeiliSink( + () => BACKEND, + () => null, + fn + ); // Two batches on the same sink: a fresh-rollout `pnpm backfill` must not // let PUT /documents auto-create a bare index whose _geo/startsAt searches @@ -292,6 +296,35 @@ describe('fetch is invoked detached (workerd Illegal invocation guard)', () => { }); }); +describe('applyMeiliSettings bounds the settings fetch', () => { + it('passes an AbortSignal on the settings request', async () => { + let sawSignal: AbortSignal | null | undefined; + const fn = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + sawSignal = init?.signal; + return new Response(null, { status: 202 }); + }) as unknown as typeof fetch; + + await applyMeiliSettings(BACKEND, fn); + expect(sawSignal).toBeInstanceOf(AbortSignal); + }); + + it('rejects when the settings fetch stalls past the timeout', async () => { + // A fetch that never settles on its own — only the AbortSignal can end it. + // A short timeout keeps this fast and deterministic without fake timers. + const stalling = vi.fn( + (_input: RequestInfo | URL, init?: RequestInit) => + new Promise((_resolve, reject) => { + const signal = init?.signal; + if (signal) { + signal.addEventListener('abort', () => reject(signal.reason ?? new Error('aborted'))); + } + }) + ) as unknown as typeof fetch; + + await expect(applyMeiliSettings(BACKEND, stalling, 10)).rejects.toThrow(); + }); +}); + /** A minimal D1 double: prepare().bind().all() returns the seeded resolved * rows whose address_norm is in the bound args. Throw mode exercises the * best-effort swallow. */ diff --git a/apps/web/src/lib/search/server/meili-sink.ts b/apps/web/src/lib/search/server/meili-sink.ts index ca2219b..2e39434 100644 --- a/apps/web/src/lib/search/server/meili-sink.ts +++ b/apps/web/src/lib/search/server/meili-sink.ts @@ -27,6 +27,16 @@ type RecordEvent = Parameters[0][number]; /** The one collection we index for search. */ export const EVENT_COLLECTION = 'community.lexicon.calendar.event'; +/** How long the settings PATCH (which also arms/creates the index) may run + * before we abort it. This request fires on the ensureInit arming path — and + * for the cron handler that runs BEFORE the ingest hard-timeout race, so an + * unbounded fetch to a *stalling* (not erroring) Meili endpoint would hang the + * whole tick past the 55s backstop and starve notify/drip. A short bound + * degrades that to "sink disabled this cycle" (caught by ensureInit's + * try/catch), retried next tick. Upsert/remove stay unbounded here: they only + * run inside contrail.ingest, already under the cron race. */ +const SETTINGS_TIMEOUT_MS = 8_000; + export interface MeiliSinkBackend { url: string; apiKey: string; @@ -82,21 +92,28 @@ export class MeiliEventIndex { /** PATCHing settings auto-creates the index, so this doubles as ensure-index. * _geo in filterable enables _geoRadius/_geoBoundingBox; in sortable, _geoPoint. - * Must mirror the filters ./meili.ts issues (startsAt/endsAt range, _geo). */ - async applySettings(): Promise { - await this.request('PATCH', `/indexes/${this.indexUid}/settings`, { - searchableAttributes: ['name', 'description'], - filterableAttributes: [ - '_geo', - 'startsAt', - 'endsAt', - 'status', - 'mode', - 'did', - 'locationTypes' - ], - sortableAttributes: ['_geo', 'startsAt', 'endsAt'] - }); + * Must mirror the filters ./meili.ts issues (startsAt/endsAt range, _geo). + * Bounded by an AbortSignal so a stalling Meili endpoint can't hang the + * arming path (see SETTINGS_TIMEOUT_MS). */ + async applySettings(timeoutMs: number = SETTINGS_TIMEOUT_MS): Promise { + await this.request( + 'PATCH', + `/indexes/${this.indexUid}/settings`, + { + searchableAttributes: ['name', 'description'], + filterableAttributes: [ + '_geo', + 'startsAt', + 'endsAt', + 'status', + 'mode', + 'did', + 'locationTypes' + ], + sortableAttributes: ['_geo', 'startsAt', 'endsAt'] + }, + AbortSignal.timeout(timeoutMs) + ); } async upsert(docs: SearchDoc[]): Promise { @@ -109,7 +126,12 @@ export class MeiliEventIndex { await this.request('POST', `/indexes/${this.indexUid}/documents/delete-batch`, ids); } - private async request(method: string, path: string, body: unknown): Promise { + private async request( + method: string, + path: string, + body: unknown, + signal?: AbortSignal + ): Promise { // Call fetch detached, not as `this.fetch(...)`: on workerd the global // fetch throws "Illegal invocation" when invoked with `this` bound to a // non-global object (which method-call syntax would do). A bare call @@ -122,7 +144,8 @@ export class MeiliEventIndex { authorization: `Bearer ${this.apiKey}`, 'content-type': 'application/json' }, - body: JSON.stringify(body) + body: JSON.stringify(body), + signal }); if (!res.ok) { // No body/key echoed — it can end up in logs or error pages. @@ -133,12 +156,15 @@ export class MeiliEventIndex { /** PATCH the index settings (and auto-create the index) for a backend. Call * once per worker before the sink starts upserting so the read path's filters - * resolve. */ + * resolve. Bounded by SETTINGS_TIMEOUT_MS (override for tests) so a stalling + * Meili endpoint can't hang the caller — the arming path runs outside the cron + * ingest race. */ export async function applyMeiliSettings( backend: MeiliSinkBackend, - fetchFn?: typeof fetch + fetchFn?: typeof fetch, + timeoutMs: number = SETTINGS_TIMEOUT_MS ): Promise { - await new MeiliEventIndex(backend, fetchFn).applySettings(); + await new MeiliEventIndex(backend, fetchFn).applySettings(timeoutMs); } /** Best-effort fill of doc._geo from the geocode_cache. Read-only; a missing -- 2.51.2