From c59ab9ea9ee3f5422dfbe981e9010db1c543a9a9 Mon Sep 17 00:00:00 2001 From: rejifald Date: Sun, 16 Aug 2026 15:03:38 +0300 Subject: [PATCH] fix(docs): stop a transient search fault from sticking as a silent blank MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fumadocs search dialog renders nothing at all when a request fails — no error, no "no results" — so every failure below reads to a visitor as "search is broken, it shows nothing". Three of them turned a momentary fault into a lasting one. `getEmbedder()` memoized the `loadEmbedder()` promise, rejection included. That gives back exactly what deferring the `@huggingface/transformers` import bought: the deferral (see embed.ts's module header, and the Jul 31 – Aug 10 2026 outage behind it) exists so a failed load stays scoped to the one call that needed it — but parking the rejected promise in `extractor` re-widens it to every later query on that warm instance, with nothing to dislodge it but a recycle. `loadIndex()` had the same shape. Both now clear the slot on rejection so the next call retries. The route answered `200 []` on any error, which made a broken index indistinguishable from a query with no matches in both directions that matter: uptime checks read the outage as healthy, and fumadocs' fetch client memoizes per-URL for the page's lifetime, so the empty array stuck to that query even after the backend recovered. It now answers 503, which is not cached — the visitor's next keystroke retries. `/api/search-docs` also set no `maxDuration` while `/api/mcp` already takes 60 for the identical model load; it now matches. Headroom rather than a fix for a specific timeout — the two paths should not disagree about how long that load may take. The regression spec needs to import an app-router module, hence the `@/` alias in vitest.config.ts: without it a `vi.mock()` of a relative path and the route's `@/`-prefixed import resolve to two different module ids and the mock silently doesn't apply. Co-Authored-By: Claude Opus 5 --- apps/docs/app/api/search-docs/route.ts | 22 +++- apps/docs/lib/search-index/embed.ts | 13 ++- apps/docs/lib/search-index/search.ts | 10 +- apps/docs/test/search-route-failure.spec.ts | 118 ++++++++++++++++++++ apps/docs/vitest.config.ts | 11 ++ 5 files changed, 169 insertions(+), 5 deletions(-) create mode 100644 apps/docs/test/search-route-failure.spec.ts diff --git a/apps/docs/app/api/search-docs/route.ts b/apps/docs/app/api/search-docs/route.ts index d495c7c8..773d590a 100644 --- a/apps/docs/app/api/search-docs/route.ts +++ b/apps/docs/app/api/search-docs/route.ts @@ -13,6 +13,17 @@ import { toSortedResults } from '@/lib/search-index/sorted-result'; export const runtime = 'nodejs'; export const dynamic = 'force-dynamic'; +// A cold instance pays the embedding model's *load* before it can answer any +// query. The weights ship with the function now, so that load reads from disk +// rather than re-fetching ~87 MB from the HF CDN per cold start, and the +// measured cost is seconds, not the tens the CDN fetch cost. The ceiling is +// headroom either way, not a fix for a specific timeout: `/api/mcp` already +// takes 60 for the identical load, and the two paths should not disagree about +// how long that load is allowed to take. The fumadocs dialog renders nothing at +// all on a failed request, so whatever does exceed the ceiling reads to a +// visitor as "search is broken". +export const maxDuration = 60; + export async function GET(request: Request): Promise { // Truncate before doing any work: the embedder tokenizes the whole raw // string, so an unbounded `query` param is a CPU/memory DoS. searchDocs caps @@ -29,9 +40,14 @@ export async function GET(request: Request): Promise { const hits = await searchDocs(query, { limit: 8 }); return Response.json(toSortedResults(hits)); } catch (error) { - // Degrade to "no results" rather than 500-ing the search box if the - // index or model isn't available (e.g. index not built for this env). + // A missing index or a failed model load is infrastructure being down, + // not a query with no matches — say so with a status. Answering 200 [] + // instead made the two indistinguishable in every direction that + // matters: uptime checks read a broken search as healthy, and fumadocs' + // fetch client memoizes per-URL for the page's lifetime, so the cached + // empty array kept the query blank even after the instance warmed up. + // A non-ok response is not cached, so the user's next keystroke retries. console.error('[search-docs] query failed:', error); - return Response.json([]); + return Response.json({ error: 'search unavailable' }, { status: 503 }); } } diff --git a/apps/docs/lib/search-index/embed.ts b/apps/docs/lib/search-index/embed.ts index 7b83d1eb..55544ac4 100644 --- a/apps/docs/lib/search-index/embed.ts +++ b/apps/docs/lib/search-index/embed.ts @@ -44,7 +44,18 @@ let extractor: Promise | undefined; * too, not just the pipeline construction. */ export function getEmbedder(): Promise { if (!extractor) { - extractor = loadEmbedder(); + extractor = loadEmbedder().catch((error: unknown) => { + // Memoizing the *rejection* would give back exactly what deferring + // the import bought. Per the module header, a failed + // @huggingface/transformers load is scoped to the one call that + // needed it — but parking that rejected promise in `extractor` + // re-widens it to every later query on this warm instance, with + // nothing to dislodge it but a recycle. That is the module-load + // failure mode again by another route. Drop the slot so the next + // call retries. + extractor = undefined; + throw error; + }); } return extractor; } diff --git a/apps/docs/lib/search-index/search.ts b/apps/docs/lib/search-index/search.ts index 6467a474..39034f1a 100644 --- a/apps/docs/lib/search-index/search.ts +++ b/apps/docs/lib/search-index/search.ts @@ -42,7 +42,15 @@ function indexPath(): string { /** Restore (once) the persisted Orama index. Throws if it hasn't been built. */ export function loadIndex(): Promise { if (!cached) { - cached = restore('json', readFileSync(indexPath(), 'utf8')); + cached = restore('json', readFileSync(indexPath(), 'utf8')).catch( + (error: unknown) => { + // Same reason as the embedder's loader: a memoized rejection + // would outlive the fault that caused it and fail every later + // query on this instance. Clear it so a retry can succeed. + cached = undefined; + throw error; + }, + ); } return cached; } diff --git a/apps/docs/test/search-route-failure.spec.ts b/apps/docs/test/search-route-failure.spec.ts new file mode 100644 index 00000000..5f01a9c4 --- /dev/null +++ b/apps/docs/test/search-route-failure.spec.ts @@ -0,0 +1,118 @@ +// Regression coverage for how /api/search-docs reports a *broken* search, and +// for the memoized-rejection trap behind it. +// +// Both matter for the same user-visible symptom: the fumadocs search dialog +// renders nothing at all when a request fails, so any failure here reads as +// "search is broken, it shows nothing" with no error anywhere on the page. +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('@/lib/search-index/search', () => ({ + searchDocs: vi.fn(), +})); + +// transformers.js resolves its Node backend to a native onnxruntime addon at +// import; stub the module so this spec stays in the pure vitest job. +vi.mock('@huggingface/transformers', () => ({ + env: {}, + pipeline: vi.fn(), +})); + +const { searchDocs } = await import('@/lib/search-index/search'); +const { pipeline } = await import('@huggingface/transformers'); +const { getEmbedder } = await import('@/lib/search-index/embed'); +const { GET } = await import('@/app/api/search-docs/route'); + +const mockedSearchDocs = vi.mocked(searchDocs); +const mockedPipeline = vi.mocked(pipeline); + +function request(query: string): Request { + return new Request( + `https://stitchapi.dev/api/search-docs?query=${encodeURIComponent(query)}`, + ); +} + +beforeEach(() => { + vi.clearAllMocks(); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('/api/search-docs failure reporting', () => { + it('answers 503 — not 200 [] — when the engine throws', async () => { + // 200 [] made a broken index indistinguishable from a query with no + // matches: uptime checks read the outage as healthy, and fumadocs' + // fetch client memoizes per-URL for the page's lifetime, so the empty + // array stuck to that query even after the backend recovered. + mockedSearchDocs.mockRejectedValueOnce( + new Error("ENOENT: .search-index/docs-index.json doesn't exist"), + ); + vi.spyOn(console, 'error').mockImplementation(() => {}); + + const res = await GET(request('retry')); + + expect(res.status).toBe(503); + expect(res.ok).toBe(false); + }); + + it('still answers 200 [] for a blank query', async () => { + const res = await GET(request(' ')); + + expect(res.status).toBe(200); + await expect(res.json()).resolves.toEqual([]); + expect(mockedSearchDocs).not.toHaveBeenCalled(); + }); + + it('answers 200 with results on the happy path', async () => { + mockedSearchDocs.mockResolvedValueOnce([ + { + pageUrl: '/docs/guides/resilience/retry', + pageTitle: 'Retry & backoff', + heading: 'Options', + anchor: 'options', + text: 'body', + score: 1, + }, + ]); + + const res = await GET(request('retry')); + + expect(res.status).toBe(200); + await expect(res.json()).resolves.toEqual([ + { + id: 'page:/docs/guides/resilience/retry', + url: '/docs/guides/resilience/retry', + type: 'page', + content: 'Retry & backoff', + }, + { + id: 'hit:0:/docs/guides/resilience/retry#options', + url: '/docs/guides/resilience/retry#options', + type: 'heading', + content: 'Options', + }, + ]); + }); +}); + +describe('getEmbedder', () => { + it('does not memoize a rejected load', async () => { + // The load can still fail with the weights shipped alongside the + // function — a bad @huggingface/transformers import is the case the + // deferred import in embed.ts exists for. Memoizing that rejection + // would leave every later query on the same warm instance awaiting the + // same settled promise — a permanent, silent search outage on that + // instance, recoverable only by a recycle, which is the instance-wide + // failure the deferral was meant to prevent. + mockedPipeline.mockRejectedValueOnce(new Error('model load failed')); + + await expect(getEmbedder()).rejects.toThrow('model load failed'); + + // A second call must retry rather than hand back the settled rejection. + const reloaded = { name: 'reloaded' }; + mockedPipeline.mockResolvedValueOnce(reloaded as never); + await expect(getEmbedder()).resolves.toBe(reloaded); + expect(mockedPipeline).toHaveBeenCalledTimes(2); + }); +}); diff --git a/apps/docs/vitest.config.ts b/apps/docs/vitest.config.ts index f238edad..82400129 100644 --- a/apps/docs/vitest.config.ts +++ b/apps/docs/vitest.config.ts @@ -1,9 +1,20 @@ +import { fileURLToPath } from 'node:url'; import { defineConfig } from 'vitest/config'; // Docs-IA guardrail tests only (test/**/*.spec.ts) — manifest ↔ content/docs // two-way sync and gen:docs idempotency. The Playwright e2e suite is separate // (`test:e2e`), and Next/Fumadocs rendering is covered by the build gate. export default defineConfig({ + // Mirror the `@/*` path alias from tsconfig.json so a spec can import an + // app-router module (e.g. app/api/search-docs/route.ts) the same way the + // module imports its own dependencies — without the alias, vi.mock() of a + // relative path and the route's `@/`-prefixed import resolve to two + // different module ids and the mock silently doesn't apply. + resolve: { + alias: { + '@': fileURLToPath(new URL('.', import.meta.url)), + }, + }, test: { globals: true, environment: 'node',