mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-04 22:32:12 +03:00
fix(api): serve /v1/models stale-first and sanitize its error bodies (#8703)
* fix(api): serve the model catalog stale-first and sanitize its error body A client with a short discovery timeout (Claude Code allows 3s) hit a full catalog rebuild — 290 providers plus SQLite reads — every time the memoized entry expired, and got an empty model picker with no error. Serve an expired entry immediately and revalidate in the background, bounded by a staleness window so a permanently failing refresh cannot pin an old catalog forever. Only a cached 200 is eligible; a state change still drops the cache outright. The builder's catch block also returned the raw error message in the response body. Route it through the shared sanitizer (hard rule #12). * fix(api): reject a failed catalog refresh instead of resolving it stale catalogInFlight is shared with the cold path, so resolving the background refresh with the stale entry handed it to callers that had already aged past CATALOG_STALE_WHILE_REVALIDATE_MS — a stale 200 they were no longer entitled to, with a build failure disguised as success. The refresh now rejects; the stale path never awaits it (the rejection is pre-handled, so it can never surface as an unhandledRejection) and a cold-path caller that joins it gets the sanitized 500. A failed refresh still leaves the cached entry untouched. Also sanitize the core builder's own catch — that is the realistically reachable 500 for this endpoint, and it still returned the raw error message (hard rule #12); keep the cache-key format private to the module by having the two test hooks take the Request and derive the key themselves. * refactor(api): extract the model-catalog response cache into its own module The stale-while-revalidate work pushed catalog.ts from 1615 to 1745 lines, past its frozen size cap. Raising the cap on a file already flagged as too large is the wrong answer: the caching layer is a self-contained concern (coalescing, TTL memoization, staleness window, background refresh) that only needs a builder callback from the catalog module. catalogCache.ts now owns the maps, the cache key, the state-change invalidation, the header merge, the background refresh and the test hooks; catalog.ts keeps auth, the builder, and the error shape, and re-exports the hooks so the existing tests keep importing them from where they always did. Net effect: catalog.ts 1745 -> 1513, i.e. 102 lines below the cap it was frozen at, and the test-only surface no longer sits in the production catalog module. No behavior change — all 281 tests across every suite importing catalog.ts pass, including the #6408 one-builder-run guard.
This commit is contained in:
Diego Rodrigues de Sa e Souza
committed by
GitHub
GitHub
parent
6389c5b12f
commit
6706d5ff7d
@@ -95,6 +95,7 @@ import {
|
||||
import { getModelCatalogAuthRejection, isCodexModelCatalogClient } from "./catalogRequest";
|
||||
import { isFreeModel, providerHasFreeModels } from "@/shared/utils/freeModels";
|
||||
import { isCodexDiscoveryModelExcluded } from "@/shared/services/codexDiscoveryPolicy";
|
||||
import { buildErrorBody } from "@omniroute/open-sse/utils/error";
|
||||
|
||||
// Public API of this module is preserved after the catalog helper extraction:
|
||||
// `isVisionModelId` (vision-detection-consistency.test.ts) and
|
||||
@@ -103,87 +104,22 @@ import { isCodexDiscoveryModelExcluded } from "@/shared/services/codexDiscoveryP
|
||||
export { isVisionModelId } from "@/shared/constants/visionModels";
|
||||
export { getCustomVisionCapabilityFields };
|
||||
|
||||
// #6408 — Concurrent GET /v1/models requests serialized (~1.2s each × N). The
|
||||
// per-request builder walks 8 registries + hits SQLite for connections, combos,
|
||||
// custom models, and aliases; under Next.js single-threaded App Router request
|
||||
// handling, N concurrent calls execute back-to-back and the Nth completes
|
||||
// N × single-request latency (linear staircase reproduced in the issue).
|
||||
//
|
||||
// Fix: coalesce identical concurrent requests onto a single in-flight promise,
|
||||
// then memoize the serialized body for a short window so a burst (SDK startup,
|
||||
// multi-tab dashboard poll) returns from cache. Auth-rejection paths are NOT
|
||||
// cached (they depend on live session state — dashboard cookies, API key).
|
||||
type CachedCatalog = {
|
||||
body: string;
|
||||
headers: Record<string, string>;
|
||||
status: number;
|
||||
expiresAt: number;
|
||||
};
|
||||
const CATALOG_CACHE_TTL_MS_DEFAULT = 1500; // fallback; overridden by settings
|
||||
const catalogCache = new Map<string, CachedCatalog>();
|
||||
const catalogInFlight = new Map<string, Promise<CachedCatalog>>();
|
||||
// The response cache (coalescing, short-TTL memoization and stale-while-revalidate)
|
||||
// lives in ./catalogCache. Re-exported here because the existing tests import the
|
||||
// hooks from this module, and CATALOG_STALE_WHILE_REVALIDATE_MS is part of the
|
||||
// documented behavior of this endpoint.
|
||||
import { CATALOG_CACHE_TTL_MS_DEFAULT, resolveCachedCatalogResponse } from "./catalogCache";
|
||||
|
||||
// Test hook — increments each time the full catalog builder runs. Used by
|
||||
// tests/unit/v1-models-concurrent-6408.test.ts to prove concurrent requests
|
||||
// share one execution. Not part of the public API; do not read from app code.
|
||||
let _catalogBuilderRuns = 0;
|
||||
export function __resetCatalogBuilderRunsForTest(): void {
|
||||
_catalogBuilderRuns = 0;
|
||||
catalogCache.clear();
|
||||
catalogInFlight.clear();
|
||||
lastSeenCatalogCacheVersion = getModelCatalogCacheVersion();
|
||||
}
|
||||
export function __getCatalogBuilderRunsForTest(): number {
|
||||
return _catalogBuilderRuns;
|
||||
}
|
||||
|
||||
function buildCatalogCacheKey(request: Request): string {
|
||||
const url = new URL(request.url);
|
||||
const prefix = url.searchParams.get("prefix") || "";
|
||||
const apiKey = extractApiKey(request) || "";
|
||||
const isCodex = isCodexModelCatalogClient(request) ? "1" : "0";
|
||||
const configuredOnly = url.searchParams.get("configuredOnly") === "true" ? "1" : "0";
|
||||
return `${prefix}|${isCodex}|${apiKey}|${configuredOnly}`;
|
||||
}
|
||||
|
||||
// Tracks the model-catalog cache version (src/lib/db/readCache.ts) as of the last
|
||||
// cache access. invalidateDbCache() bumps that version on every settings/connections/
|
||||
// combos/pricing write; when it moves on, every memoized entry here was built from
|
||||
// state that no longer holds, so drop them all rather than keying by version (which
|
||||
// would leak one Map entry per version forever instead of ever pruning old ones).
|
||||
let lastSeenCatalogCacheVersion = getModelCatalogCacheVersion();
|
||||
function dropCatalogCacheIfStateChanged(): void {
|
||||
const currentVersion = getModelCatalogCacheVersion();
|
||||
if (currentVersion === lastSeenCatalogCacheVersion) return;
|
||||
lastSeenCatalogCacheVersion = currentVersion;
|
||||
catalogCache.clear();
|
||||
// Deliberately NOT clearing catalogInFlight: an in-flight build already reads live
|
||||
// DB/settings state as of when it started, so letting it finish and populate the
|
||||
// (now-current) cache entry is correct — clearing it would just force a redundant
|
||||
// second builder run for requests that arrive mid-flight.
|
||||
}
|
||||
|
||||
// Header sources here mix Title-Case keys (diagnosticHeaders, corsHeaders — plain
|
||||
// objects built by app code) with lower-case keys (payload/cached.headers — captured
|
||||
// via the Fetch `Headers` iterator, which always yields lower-cased names). Merging
|
||||
// those with a plain object spread leaves both casings present as distinct object
|
||||
// keys; the `Response` constructor then treats them as the same case-insensitive
|
||||
// header and *appends* rather than overwrites, producing a comma-joined duplicate
|
||||
// (e.g. request-id echoing "foo, foo"). Merge through a real `Headers` instance
|
||||
// instead so `.set()` overwrites case-insensitively. Sources listed earlier are the
|
||||
// base (cached/freshly-built payload headers); `diagnosticHeaders` is applied last so
|
||||
// per-request fields (e.g. X-Request-Id) always reflect the *current* request rather
|
||||
// than whichever request happened to populate the cache entry.
|
||||
function mergeCatalogHeaders(...sources: Array<Record<string, string> | undefined>): Headers {
|
||||
const merged = new Headers();
|
||||
for (const source of sources) {
|
||||
if (!source) continue;
|
||||
for (const [key, value] of Object.entries(source)) {
|
||||
merged.set(key, value);
|
||||
}
|
||||
}
|
||||
return merged;
|
||||
}
|
||||
export {
|
||||
CATALOG_STALE_WHILE_REVALIDATE_MS,
|
||||
__resetCatalogBuilderRunsForTest,
|
||||
__getCatalogBuilderRunsForTest,
|
||||
__expireCatalogCacheForTest,
|
||||
__setCatalogCacheEntryForTest,
|
||||
__flushCatalogBackgroundRefreshForTest,
|
||||
__forceCatalogInFlightRejectionForTest,
|
||||
} from "./catalogCache";
|
||||
export type { CachedCatalog } from "./catalogCache";
|
||||
|
||||
/**
|
||||
* Build unified OpenAI-compatible model catalog response.
|
||||
@@ -212,47 +148,22 @@ export async function getUnifiedModelsResponse(
|
||||
// Fall through to full builder on auth-check failure; core handles errors.
|
||||
}
|
||||
|
||||
dropCatalogCacheIfStateChanged();
|
||||
const cacheKey = buildCatalogCacheKey(request);
|
||||
const cached = catalogCache.get(cacheKey);
|
||||
if (cached && cached.expiresAt > Date.now()) {
|
||||
return new Response(cached.body, {
|
||||
status: cached.status,
|
||||
headers: mergeCatalogHeaders(corsHeaders, cached.headers, diagnosticHeaders),
|
||||
});
|
||||
}
|
||||
let inflight = catalogInFlight.get(cacheKey);
|
||||
if (!inflight) {
|
||||
inflight = buildCatalogPayload(request).then((payload) => {
|
||||
catalogCache.set(cacheKey, {
|
||||
body: payload.body,
|
||||
headers: payload.headers,
|
||||
status: payload.status,
|
||||
expiresAt: Date.now() + payload.cacheTTL,
|
||||
});
|
||||
return payload;
|
||||
});
|
||||
catalogInFlight.set(cacheKey, inflight);
|
||||
inflight.finally(() => {
|
||||
if (catalogInFlight.get(cacheKey) === inflight) catalogInFlight.delete(cacheKey);
|
||||
});
|
||||
}
|
||||
|
||||
try {
|
||||
const payload = await inflight;
|
||||
return new Response(payload.body, {
|
||||
status: payload.status,
|
||||
headers: mergeCatalogHeaders(corsHeaders, payload.headers, diagnosticHeaders),
|
||||
});
|
||||
return await resolveCachedCatalogResponse(
|
||||
request,
|
||||
{ corsHeaders, diagnosticHeaders },
|
||||
buildCatalogPayload
|
||||
);
|
||||
} catch (err) {
|
||||
// Hard rule #12: never put a raw err.message/err.stack in a response body.
|
||||
// Route it through the shared sanitizer instead — same status/type/code as
|
||||
// before, minus the stack-trace/path leak.
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
return Response.json(
|
||||
{
|
||||
error: {
|
||||
message: err instanceof Error ? err.message : String(err),
|
||||
type: "server_error",
|
||||
code: INTERNAL_PROXY_ERROR,
|
||||
},
|
||||
},
|
||||
buildErrorBody(500, message, undefined, {
|
||||
type: "server_error",
|
||||
code: INTERNAL_PROXY_ERROR,
|
||||
}),
|
||||
{ status: 500, headers: { ...corsHeaders, ...diagnosticHeaders } }
|
||||
);
|
||||
}
|
||||
@@ -261,7 +172,6 @@ export async function getUnifiedModelsResponse(
|
||||
async function buildCatalogPayload(
|
||||
request: Request
|
||||
): Promise<{ body: string; headers: Record<string, string>; status: number; cacheTTL: number }> {
|
||||
_catalogBuilderRuns++;
|
||||
const built = await buildUnifiedModelsResponseCore(request);
|
||||
const body = await built.text();
|
||||
const headers: Record<string, string> = {};
|
||||
@@ -1583,14 +1493,14 @@ async function buildUnifiedModelsResponseCore(
|
||||
});
|
||||
} catch (error) {
|
||||
console.log("Error fetching models:", error);
|
||||
// Hard rule #12 — this is the realistically reachable 500 for the endpoint
|
||||
// (the wrapper's catch only fires on an in-flight rejection), so it must go
|
||||
// through the shared sanitizer too. Same status/type/code as before.
|
||||
return Response.json(
|
||||
{
|
||||
error: {
|
||||
message: error instanceof Error ? error.message : String(error),
|
||||
type: "server_error",
|
||||
code: INTERNAL_PROXY_ERROR,
|
||||
},
|
||||
},
|
||||
buildErrorBody(500, error instanceof Error ? error.message : String(error), undefined, {
|
||||
type: "server_error",
|
||||
code: INTERNAL_PROXY_ERROR,
|
||||
}),
|
||||
{
|
||||
status: 500,
|
||||
headers: {
|
||||
|
||||
284
src/app/api/v1/models/catalogCache.ts
Normal file
284
src/app/api/v1/models/catalogCache.ts
Normal file
@@ -0,0 +1,284 @@
|
||||
/**
|
||||
* Response cache for `GET /v1/models`, extracted from catalog.ts.
|
||||
*
|
||||
* #6408 — concurrent catalog requests used to serialize (~1.2 s each × N). The
|
||||
* builder walks 8 registries and hits SQLite for connections, combos, custom
|
||||
* models and aliases; under Next.js's single-threaded App Router request
|
||||
* handling, N concurrent calls execute back-to-back and the Nth completes at
|
||||
* N × single-request latency. So identical concurrent requests are coalesced
|
||||
* onto one in-flight promise and the serialized body is memoized for a short
|
||||
* window.
|
||||
*
|
||||
* Auth rejection is NOT handled here and must stay in the caller: it depends on
|
||||
* live per-request state (dashboard cookie, API key) and must never be cached.
|
||||
*/
|
||||
import { getModelCatalogCacheVersion } from "@/lib/db/readCache";
|
||||
import { extractApiKey } from "@/sse/services/auth";
|
||||
|
||||
import { isCodexModelCatalogClient } from "./catalogRequest";
|
||||
|
||||
export type CachedCatalog = {
|
||||
body: string;
|
||||
headers: Record<string, string>;
|
||||
status: number;
|
||||
expiresAt: number;
|
||||
};
|
||||
|
||||
/** Payload shape returned by the builder the caller injects. */
|
||||
export type CatalogPayload = {
|
||||
body: string;
|
||||
headers: Record<string, string>;
|
||||
status: number;
|
||||
cacheTTL: number;
|
||||
};
|
||||
|
||||
/**
|
||||
* A client with a short discovery timeout (Claude Code allows 3 s) must never
|
||||
* wait on a full rebuild. Once a cached 200 expires it is still served
|
||||
* immediately for up to this long while a background refresh repopulates it.
|
||||
* Bounded so a refresh that keeps failing cannot pin an old catalog forever —
|
||||
* past this window callers fall back to waiting, same as a cold cache.
|
||||
*/
|
||||
export const CATALOG_STALE_WHILE_REVALIDATE_MS = 30_000;
|
||||
|
||||
/** Fallback memoization window; overridden by `settings.cache.modelCatalogCacheTtlMs`. */
|
||||
export const CATALOG_CACHE_TTL_MS_DEFAULT = 1500;
|
||||
|
||||
const catalogCache = new Map<string, CachedCatalog>();
|
||||
const catalogInFlight = new Map<string, Promise<CachedCatalog>>();
|
||||
|
||||
let _catalogBuilderRuns = 0;
|
||||
|
||||
function buildCatalogCacheKey(request: Request): string {
|
||||
const url = new URL(request.url);
|
||||
const prefix = url.searchParams.get("prefix") || "";
|
||||
const apiKey = extractApiKey(request) || "";
|
||||
const isCodex = isCodexModelCatalogClient(request) ? "1" : "0";
|
||||
const configuredOnly = url.searchParams.get("configuredOnly") === "true" ? "1" : "0";
|
||||
return `${prefix}|${isCodex}|${apiKey}|${configuredOnly}`;
|
||||
}
|
||||
|
||||
// Tracks the model-catalog cache version (src/lib/db/readCache.ts) as of the last
|
||||
// cache access. invalidateDbCache() bumps that version on every settings/connections/
|
||||
// combos/pricing write; when it moves on, every memoized entry here was built from
|
||||
// state that no longer holds, so drop them all rather than keying by version (which
|
||||
// would leak one Map entry per version forever instead of ever pruning old ones).
|
||||
let lastSeenCatalogCacheVersion = getModelCatalogCacheVersion();
|
||||
function dropCatalogCacheIfStateChanged(): void {
|
||||
const currentVersion = getModelCatalogCacheVersion();
|
||||
if (currentVersion === lastSeenCatalogCacheVersion) return;
|
||||
lastSeenCatalogCacheVersion = currentVersion;
|
||||
catalogCache.clear();
|
||||
// Deliberately NOT clearing catalogInFlight: an in-flight build already reads live
|
||||
// DB/settings state as of when it started, so letting it finish and populate the
|
||||
// (now-current) cache entry is correct — clearing it would just force a redundant
|
||||
// second builder run for requests that arrive mid-flight.
|
||||
}
|
||||
|
||||
// Header sources mix Title-Case keys (diagnostic/cors headers built by app code) with
|
||||
// lower-case ones (payload headers captured via the Fetch `Headers` iterator). A plain
|
||||
// object spread keeps both casings as distinct keys, and the `Response` constructor
|
||||
// then *appends* rather than overwrites them, producing comma-joined duplicates (e.g.
|
||||
// request-id echoing "foo, foo"). Merge through a real `Headers` so `.set()` overwrites
|
||||
// case-insensitively. Earlier sources are the base; the caller passes diagnostics last
|
||||
// so per-request fields reflect the current request, not whichever one filled the cache.
|
||||
export function mergeCatalogHeaders(
|
||||
...sources: Array<Record<string, string> | undefined>
|
||||
): Headers {
|
||||
const merged = new Headers();
|
||||
for (const source of sources) {
|
||||
if (!source) continue;
|
||||
for (const [key, value] of Object.entries(source)) {
|
||||
merged.set(key, value);
|
||||
}
|
||||
}
|
||||
return merged;
|
||||
}
|
||||
|
||||
function storePayload(cacheKey: string, payload: CatalogPayload): CachedCatalog {
|
||||
const entry: CachedCatalog = {
|
||||
body: payload.body,
|
||||
headers: payload.headers,
|
||||
status: payload.status,
|
||||
expiresAt: Date.now() + payload.cacheTTL,
|
||||
};
|
||||
catalogCache.set(cacheKey, entry);
|
||||
return entry;
|
||||
}
|
||||
|
||||
/**
|
||||
* Kick off a background rebuild so an expired-but-stale-eligible entry can be
|
||||
* refreshed without the current request waiting on it. Reuses catalogInFlight —
|
||||
* no second coalescing mechanism — so a concurrent cold/stale request for the
|
||||
* same key joins this refresh instead of starting another.
|
||||
*
|
||||
* The builder runs one macrotask later so the stale response that triggered this
|
||||
* call is handed back before the builder's synchronous prologue runs; the whole
|
||||
* point of this path is that the caller does not pay for the rebuild.
|
||||
*
|
||||
* The tracked promise **rejects** on failure. catalogInFlight is shared with the
|
||||
* cold path: a caller whose entry aged past the stale window skips the stale
|
||||
* branch and awaits whatever promise it finds here, and resolving with the stale
|
||||
* entry would hand it a body it was no longer entitled to while disguising a
|
||||
* build failure as a 200. The rejection is pre-handled so this path can never
|
||||
* raise an unhandledRejection; a failed refresh simply never overwrites the entry.
|
||||
*/
|
||||
function scheduleBackgroundRefresh(
|
||||
cacheKey: string,
|
||||
request: Request,
|
||||
buildPayload: (request: Request) => Promise<CatalogPayload>
|
||||
): void {
|
||||
if (catalogInFlight.has(cacheKey)) return; // a refresh for this key is already running
|
||||
|
||||
const refreshPromise: Promise<CachedCatalog> = new Promise((resolve, reject) => {
|
||||
setTimeout(() => {
|
||||
runBuilder(buildPayload, request)
|
||||
.then((payload) => resolve(storePayload(cacheKey, payload)))
|
||||
.catch((err) => {
|
||||
console.error(
|
||||
`[catalog] Background stale-while-revalidate refresh failed for key "${cacheKey}":`,
|
||||
err
|
||||
);
|
||||
reject(err);
|
||||
});
|
||||
}, 0);
|
||||
});
|
||||
|
||||
// Nobody on the stale path awaits this, so pre-handle the rejection; a cold-path
|
||||
// caller that joins it via catalogInFlight attaches its own handler and still
|
||||
// observes the failure.
|
||||
refreshPromise.catch(() => {});
|
||||
|
||||
catalogInFlight.set(cacheKey, refreshPromise);
|
||||
refreshPromise
|
||||
.catch(() => {})
|
||||
.finally(() => {
|
||||
if (catalogInFlight.get(cacheKey) === refreshPromise) catalogInFlight.delete(cacheKey);
|
||||
});
|
||||
}
|
||||
|
||||
function runBuilder(
|
||||
buildPayload: (request: Request) => Promise<CatalogPayload>,
|
||||
request: Request
|
||||
): Promise<CatalogPayload> {
|
||||
_catalogBuilderRuns++;
|
||||
return buildPayload(request);
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the cached catalog response for `request`, building it through
|
||||
* `buildPayload` when there is nothing fresh to serve.
|
||||
*
|
||||
* Returns `null` when the caller must build and handle errors itself — i.e. the
|
||||
* in-flight build rejected — so the error-response shape stays in the caller.
|
||||
*/
|
||||
export async function resolveCachedCatalogResponse(
|
||||
request: Request,
|
||||
headerSources: { corsHeaders: Record<string, string>; diagnosticHeaders: Record<string, string> },
|
||||
buildPayload: (request: Request) => Promise<CatalogPayload>
|
||||
): Promise<Response> {
|
||||
const { corsHeaders, diagnosticHeaders } = headerSources;
|
||||
dropCatalogCacheIfStateChanged();
|
||||
|
||||
const cacheKey = buildCatalogCacheKey(request);
|
||||
const now = Date.now();
|
||||
const cached = catalogCache.get(cacheKey);
|
||||
|
||||
if (cached && cached.expiresAt > now) {
|
||||
return new Response(cached.body, {
|
||||
status: cached.status,
|
||||
headers: mergeCatalogHeaders(corsHeaders, cached.headers, diagnosticHeaders),
|
||||
});
|
||||
}
|
||||
|
||||
// Stale-while-revalidate: an expired entry is still served immediately as long as
|
||||
// (a) it was a successful build — a cached error replayed as "stale" would mask an
|
||||
// intermittent failure behind a fake success forever — and (b) it is within the
|
||||
// staleness window, so a refresh that keeps failing eventually falls through to the
|
||||
// cold-path wait instead of pinning ancient data.
|
||||
if (
|
||||
cached &&
|
||||
cached.status === 200 &&
|
||||
now - cached.expiresAt <= CATALOG_STALE_WHILE_REVALIDATE_MS
|
||||
) {
|
||||
scheduleBackgroundRefresh(cacheKey, request, buildPayload);
|
||||
return new Response(cached.body, {
|
||||
status: cached.status,
|
||||
headers: mergeCatalogHeaders(corsHeaders, cached.headers, diagnosticHeaders),
|
||||
});
|
||||
}
|
||||
|
||||
let inflight = catalogInFlight.get(cacheKey);
|
||||
if (!inflight) {
|
||||
inflight = runBuilder(buildPayload, request).then((payload) => storePayload(cacheKey, payload));
|
||||
catalogInFlight.set(cacheKey, inflight);
|
||||
inflight.finally(() => {
|
||||
if (catalogInFlight.get(cacheKey) === inflight) catalogInFlight.delete(cacheKey);
|
||||
});
|
||||
}
|
||||
|
||||
const payload = await inflight;
|
||||
return new Response(payload.body, {
|
||||
status: payload.status,
|
||||
headers: mergeCatalogHeaders(corsHeaders, payload.headers, diagnosticHeaders),
|
||||
});
|
||||
}
|
||||
|
||||
// ── Test hooks ───────────────────────────────────────────────────────────────
|
||||
// Not part of the public API; do not read from app code.
|
||||
|
||||
/** Resets the builder counter and every cached/in-flight entry. */
|
||||
export function __resetCatalogBuilderRunsForTest(): void {
|
||||
_catalogBuilderRuns = 0;
|
||||
catalogCache.clear();
|
||||
catalogInFlight.clear();
|
||||
lastSeenCatalogCacheVersion = getModelCatalogCacheVersion();
|
||||
}
|
||||
|
||||
/** Counts full builder executions — proves concurrent requests share one run (#6408). */
|
||||
export function __getCatalogBuilderRunsForTest(): number {
|
||||
return _catalogBuilderRuns;
|
||||
}
|
||||
|
||||
/**
|
||||
* Marks every cached entry as expired `msAgo` milliseconds ago instead of sleeping
|
||||
* out the real TTL. Pass more than CATALOG_STALE_WHILE_REVALIDATE_MS to simulate an
|
||||
* entry that has aged past the stale-serving window.
|
||||
*/
|
||||
export function __expireCatalogCacheForTest(msAgo = 1): void {
|
||||
const expiresAt = Date.now() - msAgo;
|
||||
for (const [key, entry] of catalogCache.entries()) {
|
||||
catalogCache.set(key, { ...entry, expiresAt });
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Seeds the entry a given request would read, for status/staleness combinations the
|
||||
* intentionally exception-resistant builder cannot be made to produce (e.g. a cached
|
||||
* non-200). Takes the Request so the cache-key format stays private to this module.
|
||||
*/
|
||||
export function __setCatalogCacheEntryForTest(request: Request, entry: CachedCatalog): void {
|
||||
catalogCache.set(buildCatalogCacheKey(request), entry);
|
||||
}
|
||||
|
||||
/** Awaits any background refresh in flight, instead of guessing at a real-time sleep. */
|
||||
export async function __flushCatalogBackgroundRefreshForTest(): Promise<void> {
|
||||
await Promise.all([...catalogInFlight.values()].map((p) => p.catch(() => {})));
|
||||
}
|
||||
|
||||
/**
|
||||
* Injects a synthetic in-flight rejection so the caller's catch branch (sanitized
|
||||
* error body) can be exercised deterministically — the builder core try/catches every
|
||||
* registry and DB read individually, so it is not a practical error-injection point.
|
||||
*
|
||||
* Deliberately does not self-clean the way production entries do: this promise is
|
||||
* already rejected at creation, so a cleanup callback would delete the map entry
|
||||
* within a microtask or two — before the caller's several-await auth check finishes —
|
||||
* silently swapping in a fresh cold build instead of the intended failure. The next
|
||||
* __resetCatalogBuilderRunsForTest() clears it.
|
||||
*/
|
||||
export function __forceCatalogInFlightRejectionForTest(request: Request, error: unknown): void {
|
||||
const rejected: Promise<CachedCatalog> = Promise.reject(error);
|
||||
rejected.catch(() => {}); // mark as handled — avoids an unhandledRejection warning
|
||||
catalogInFlight.set(buildCatalogCacheKey(request), rejected);
|
||||
}
|
||||
206
tests/unit/v1-models-discovery-conformance.test.ts
Normal file
206
tests/unit/v1-models-discovery-conformance.test.ts
Normal file
@@ -0,0 +1,206 @@
|
||||
// Task D1 — GET /v1/models conformance for Claude Code's gateway model discovery.
|
||||
//
|
||||
// With CLAUDE_CODE_ENABLE_GATEWAY_MODEL_DISCOVERY=1, Claude Code issues
|
||||
// `GET /v1/models?limit=1000` with a 3s timeout and `redirect: "fail"`, reading
|
||||
// only `id` + `display_name` from each entry. Three failure modes empty the
|
||||
// picker with no error surfaced to the user: the endpoint taking longer than
|
||||
// 3s, redirecting, or returning an entry without a string `id`. This suite
|
||||
// covers all three, plus the stale-while-revalidate cache behavior that makes
|
||||
// the timeout fix possible and the error-sanitization fix for the builder's
|
||||
// catch block (hard rule #12).
|
||||
//
|
||||
// Harness modeled on tests/unit/v1-models-concurrent-6408.test.ts: temp
|
||||
// DATA_DIR, core.resetDbInstance()/apiKeysDb.resetApiKeyState() cleanup so the
|
||||
// native test runner does not hang on an open SQLite handle.
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-d1-discovery-"));
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "catalog-test-secret";
|
||||
|
||||
const core = await import("../../src/lib/db/core.ts");
|
||||
const apiKeysDb = await import("../../src/lib/db/apiKeys.ts");
|
||||
const readCache = await import("../../src/lib/db/readCache.ts");
|
||||
const v1ModelsCatalog = await import("../../src/app/api/v1/models/catalog.ts");
|
||||
|
||||
async function resetStorage() {
|
||||
core.resetDbInstance();
|
||||
apiKeysDb.resetApiKeyState();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
|
||||
v1ModelsCatalog.__resetCatalogBuilderRunsForTest();
|
||||
}
|
||||
|
||||
test.beforeEach(async () => {
|
||||
await resetStorage();
|
||||
});
|
||||
|
||||
test.after(async () => {
|
||||
core.resetDbInstance();
|
||||
apiKeysDb.resetApiKeyState();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("1. GET /v1/models never returns a 3xx redirect status (regression guard)", async () => {
|
||||
const res = await v1ModelsCatalog.getUnifiedModelsResponse(
|
||||
new Request("http://localhost/v1/models")
|
||||
);
|
||||
assert.ok(
|
||||
res.status < 300 || res.status >= 400,
|
||||
`expected a non-redirect status, got ${res.status}`
|
||||
);
|
||||
});
|
||||
|
||||
test("2. every catalog entry has a non-empty string id, and display_name (if present) is a string", async () => {
|
||||
const res = await v1ModelsCatalog.getUnifiedModelsResponse(
|
||||
new Request("http://localhost/v1/models")
|
||||
);
|
||||
assert.equal(res.status, 200);
|
||||
const body = await res.json();
|
||||
assert.ok(Array.isArray(body.data), "response must carry a data array");
|
||||
assert.ok(body.data.length > 0, "catalog must not be empty");
|
||||
for (const entry of body.data) {
|
||||
assert.equal(typeof entry.id, "string", `entry.id must be a string, got ${typeof entry.id}`);
|
||||
assert.ok(entry.id.length > 0, "entry.id must not be an empty string");
|
||||
if (Object.prototype.hasOwnProperty.call(entry, "display_name")) {
|
||||
assert.equal(
|
||||
typeof entry.display_name,
|
||||
"string",
|
||||
"display_name, when present, must be a string"
|
||||
);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test("3. stale-first: an expired 200 entry within the staleness window is served immediately, then refreshed in the background", async () => {
|
||||
const makeRequest = () => new Request("http://localhost/v1/models");
|
||||
|
||||
const res1 = await v1ModelsCatalog.getUnifiedModelsResponse(makeRequest());
|
||||
assert.equal(res1.status, 200);
|
||||
const body1 = await res1.text();
|
||||
const runsAfterFirst = v1ModelsCatalog.__getCatalogBuilderRunsForTest();
|
||||
assert.equal(runsAfterFirst, 1, "the first request must have run the builder exactly once");
|
||||
|
||||
// Deterministically expire the entry (just-expired — well inside the stale window)
|
||||
// instead of sleeping out the real TTL.
|
||||
v1ModelsCatalog.__expireCatalogCacheForTest();
|
||||
|
||||
const res2 = await v1ModelsCatalog.getUnifiedModelsResponse(makeRequest());
|
||||
assert.equal(res2.status, 200);
|
||||
const body2 = await res2.text();
|
||||
assert.equal(body2, body1, "the stale response must be the cached body, served unchanged");
|
||||
assert.equal(
|
||||
v1ModelsCatalog.__getCatalogBuilderRunsForTest(),
|
||||
runsAfterFirst,
|
||||
"the builder must NOT have run yet at response time — the stale response must not wait for a rebuild"
|
||||
);
|
||||
|
||||
await v1ModelsCatalog.__flushCatalogBackgroundRefreshForTest();
|
||||
assert.equal(
|
||||
v1ModelsCatalog.__getCatalogBuilderRunsForTest(),
|
||||
runsAfterFirst + 1,
|
||||
"the background refresh must have run once flushed"
|
||||
);
|
||||
});
|
||||
|
||||
test("4. beyond the staleness window, the response waits for a fresh build again", async () => {
|
||||
const makeRequest = () => new Request("http://localhost/v1/models");
|
||||
|
||||
const res1 = await v1ModelsCatalog.getUnifiedModelsResponse(makeRequest());
|
||||
assert.equal(res1.status, 200);
|
||||
const runsAfterFirst = v1ModelsCatalog.__getCatalogBuilderRunsForTest();
|
||||
assert.equal(runsAfterFirst, 1);
|
||||
|
||||
// Push the entry's age past CATALOG_STALE_WHILE_REVALIDATE_MS.
|
||||
v1ModelsCatalog.__expireCatalogCacheForTest(
|
||||
v1ModelsCatalog.CATALOG_STALE_WHILE_REVALIDATE_MS + 5_000
|
||||
);
|
||||
|
||||
const res2 = await v1ModelsCatalog.getUnifiedModelsResponse(makeRequest());
|
||||
assert.equal(res2.status, 200);
|
||||
assert.equal(
|
||||
v1ModelsCatalog.__getCatalogBuilderRunsForTest(),
|
||||
runsAfterFirst + 1,
|
||||
"past the staleness window, the builder must run again BEFORE the response is returned " +
|
||||
"(a refresh that keeps failing must not pin a stale catalog forever)"
|
||||
);
|
||||
});
|
||||
|
||||
test("5. a cached non-200 entry is never served as stale", async () => {
|
||||
const request = new Request("http://localhost/v1/models");
|
||||
v1ModelsCatalog.__setCatalogCacheEntryForTest(request, {
|
||||
body: JSON.stringify({ error: { message: "boom", type: "server_error", code: "X" } }),
|
||||
headers: {},
|
||||
status: 500,
|
||||
// "Just expired" — this age would be well within the stale window if the
|
||||
// cached status were 200.
|
||||
expiresAt: Date.now() - 1,
|
||||
});
|
||||
const runsBefore = v1ModelsCatalog.__getCatalogBuilderRunsForTest();
|
||||
assert.equal(runsBefore, 0);
|
||||
|
||||
const res = await v1ModelsCatalog.getUnifiedModelsResponse(
|
||||
new Request("http://localhost/v1/models")
|
||||
);
|
||||
|
||||
assert.equal(
|
||||
v1ModelsCatalog.__getCatalogBuilderRunsForTest(),
|
||||
runsBefore + 1,
|
||||
"a cached error entry must never be served as stale — the builder must run instead"
|
||||
);
|
||||
assert.equal(res.status, 200, "the fresh rebuild replaces the cached error response");
|
||||
});
|
||||
|
||||
test("6. a DB-state change drops the cache outright — the next response is a fresh build, never the stale pre-change body", async () => {
|
||||
const makeRequest = () => new Request("http://localhost/v1/models");
|
||||
|
||||
const res1 = await v1ModelsCatalog.getUnifiedModelsResponse(makeRequest());
|
||||
assert.equal(res1.status, 200);
|
||||
const runsAfterFirst = v1ModelsCatalog.__getCatalogBuilderRunsForTest();
|
||||
assert.equal(runsAfterFirst, 1);
|
||||
|
||||
// Any settings/connections/combos/pricing write bumps this version; catalog.ts
|
||||
// drops its entire cache map the next time it is read, independent of TTL/staleness.
|
||||
readCache.invalidateDbCache();
|
||||
|
||||
const res2 = await v1ModelsCatalog.getUnifiedModelsResponse(makeRequest());
|
||||
assert.equal(res2.status, 200);
|
||||
assert.equal(
|
||||
v1ModelsCatalog.__getCatalogBuilderRunsForTest(),
|
||||
runsAfterFirst + 1,
|
||||
"a state change must force a fresh build — the (now-superseded) cached entry must never be " +
|
||||
"served, stale or otherwise"
|
||||
);
|
||||
});
|
||||
|
||||
test("7. the 500 error path is sanitized — no stack trace or absolute source path leaks into the body", async () => {
|
||||
const request = new Request("http://localhost/v1/models?prefix=alias&__d1_err_test=1");
|
||||
const rawMessage =
|
||||
"Query failed at /home/diegosouzapw/dev/proxys/OmniRoute-Enterprise/secret/catalog.ts:42:1";
|
||||
const err = new Error(rawMessage);
|
||||
v1ModelsCatalog.__forceCatalogInFlightRejectionForTest(request, err);
|
||||
|
||||
const res = await v1ModelsCatalog.getUnifiedModelsResponse(request);
|
||||
assert.equal(res.status, 500);
|
||||
const body = await res.json();
|
||||
|
||||
assert.ok(
|
||||
!body.error.message.includes("at /"),
|
||||
"error body must not leak a stack-trace-like path"
|
||||
);
|
||||
assert.ok(
|
||||
!body.error.message.includes("catalog.ts"),
|
||||
"error body must not leak the source file name"
|
||||
);
|
||||
assert.ok(
|
||||
!body.error.message.includes("diegosouzapw"),
|
||||
"error body must not leak the local username/path"
|
||||
);
|
||||
assert.equal(body.error.type, "server_error");
|
||||
assert.ok(typeof body.error.code === "string" && body.error.code.length > 0);
|
||||
});
|
||||
Reference in New Issue
Block a user