From d0709a684cbef991fddd6d87e3aa21ce159306ac Mon Sep 17 00:00:00 2001 From: Thomas Camlong Date: Sat, 4 Jul 2026 10:41:36 +0200 Subject: [PATCH] fix: namespace request-handler cache per handler and stop persist retries in demo mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The in-memory request handler cache introduced in #6075 keyed entries by JSON.stringify(input) in a Map shared across all handlers. Any two handlers querying the same integration with equal options (e.g. {}) collided and read each other's data — with the mock integration serving every widget kind, all demo widgets received whichever handler's data was cached first (beszel getting health-monitoring payloads, etc.). Cache keys are now namespaced per handler definition. Also disable the query-cache persister after the first FORBIDDEN response so demo mode doesn't retry a mutation that is always rejected. --- .../query-cache-persister.ts | 17 ++++ .../src/lib/integration-request-handler.ts | 27 ++++-- .../src/lib/request-handler.ts | 84 +++++++++++-------- 3 files changed, 86 insertions(+), 42 deletions(-) diff --git a/apps/nextjs/src/app/[locale]/_client-providers/query-cache-persister.ts b/apps/nextjs/src/app/[locale]/_client-providers/query-cache-persister.ts index 3af24c508..d2143de6b 100644 --- a/apps/nextjs/src/app/[locale]/_client-providers/query-cache-persister.ts +++ b/apps/nextjs/src/app/[locale]/_client-providers/query-cache-persister.ts @@ -1,22 +1,39 @@ import { createAsyncStoragePersister } from "@tanstack/query-async-storage-persister"; +import { TRPCClientError } from "@trpc/client"; import superjson from "superjson"; import { fetchApi } from "@homarr/api/client"; import { getActiveQueryCacheBoardId, queryCacheStoragePrefix } from "@homarr/api/query-cache"; +// Set once the server rejects a persist mutation as read-only (demo mode), +// so we stop retrying a mutation that can never succeed. +let persistDisabled = false; + +const isReadOnlyError = (error: unknown) => error instanceof TRPCClientError && error.data?.code === "FORBIDDEN"; + const queryCacheStorage = { getItem: (_key: string) => null as string | null, async setItem(_key: string, value: string) { + if (persistDisabled) return; const boardId = getActiveQueryCacheBoardId(); if (!boardId) return; await fetchApi.queryCache.setItem.mutate({ boardId, value }).catch((error) => { + if (isReadOnlyError(error)) { + persistDisabled = true; + return; + } console.warn("[query-cache] persist failed", error); }); }, async removeItem(_key: string) { + if (persistDisabled) return; const boardId = getActiveQueryCacheBoardId(); if (!boardId) return; await fetchApi.queryCache.removeItem.mutate({ boardId }).catch((error) => { + if (isReadOnlyError(error)) { + persistDisabled = true; + return; + } console.warn("[query-cache] remove failed", error); }); }, diff --git a/packages/request-handler/src/lib/integration-request-handler.ts b/packages/request-handler/src/lib/integration-request-handler.ts index d2fa0f1a4..c21f8cacf 100644 --- a/packages/request-handler/src/lib/integration-request-handler.ts +++ b/packages/request-handler/src/lib/integration-request-handler.ts @@ -14,17 +14,28 @@ interface Options, input: TInput) => Promise; } +let integrationHandlerCounter = 0; + export const createIntegrationRequestHandler = < TData, TKind extends IntegrationKind, TInput extends Record, >( options: Options, -) => ({ - handler: (integration: IntegrationOfKind, itemOptions: TInput) => { - const inner = createRequestHandler({ - requestAsync: async (input) => options.requestAsync(integration, input.options), - }); - return inner.handler({ integrationId: integration.id, options: itemOptions }); - }, -}); +) => { + // Stable per handler definition: the inner request handler is re-created on every + // call, so without an explicit cacheKey each call would get a fresh namespace + // (no cache hits) — and without any namespace different handlers hitting the same + // integration with equal options would share cache entries and cross their data. + const cacheKey = `integration-handler-${integrationHandlerCounter++}`; + + return { + handler: (integration: IntegrationOfKind, itemOptions: TInput) => { + const inner = createRequestHandler({ + cacheKey, + requestAsync: async (input) => options.requestAsync(integration, input.options), + }); + return inner.handler({ integrationId: integration.id, options: itemOptions }); + }, + }; +}; diff --git a/packages/request-handler/src/lib/request-handler.ts b/packages/request-handler/src/lib/request-handler.ts index 7018dd4ee..f769762d4 100644 --- a/packages/request-handler/src/lib/request-handler.ts +++ b/packages/request-handler/src/lib/request-handler.ts @@ -1,6 +1,13 @@ interface Options> { requestAsync: (input: TInput) => Promise; cacheTtlMs?: number; + /** + * Namespace for cache entries of this handler. Defaults to a unique id per + * createRequestHandler call. Callers that re-create handlers per request + * (e.g. createIntegrationRequestHandler) must pass a stable key themselves, + * otherwise the cache never hits. + */ + cacheKey?: string; } type CacheEntry = { data: TData; timestamp: Date; expiresAt: number }; @@ -18,43 +25,52 @@ const evictExpired = () => { } }; +let autoCacheKeyCounter = 0; + export const createRequestHandler = >( options: Options, -) => ({ - handler: (input: TInput) => ({ - async getDataAsync(): Promise<{ data: TData; timestamp: Date }> { - const ttl = options.cacheTtlMs ?? DEFAULT_TTL_MS; - const key = JSON.stringify(input); +) => { + // The cache map is shared between all handlers, so entries have to be namespaced + // per handler — otherwise handlers receiving the same input (e.g. the same + // integrationId with empty options) would read each other's data. + const cacheKey = options.cacheKey ?? `handler-${autoCacheKeyCounter++}`; - const cached = cache.get(key) as CacheEntry | undefined; - if (cached && Date.now() < cached.expiresAt) { - return { data: cached.data, timestamp: cached.timestamp }; - } - if (cached) cache.delete(key); + return { + handler: (input: TInput) => ({ + async getDataAsync(): Promise<{ data: TData; timestamp: Date }> { + const ttl = options.cacheTtlMs ?? DEFAULT_TTL_MS; + const key = `${cacheKey}:${JSON.stringify(input)}`; - const existing = inflight.get(key); - if (existing) return existing as Promise>; + const cached = cache.get(key) as CacheEntry | undefined; + if (cached && Date.now() < cached.expiresAt) { + return { data: cached.data, timestamp: cached.timestamp }; + } + if (cached) cache.delete(key); - const promise = options - .requestAsync(input) - .then((data) => { - if (cache.size >= MAX_CACHE_SIZE) evictExpired(); - if (cache.size >= MAX_CACHE_SIZE) { - const oldest = cache.keys().next().value; - if (oldest) cache.delete(oldest); - } - const entry: CacheEntry = { data, timestamp: new Date(), expiresAt: Date.now() + ttl }; - cache.set(key, entry as CacheEntry); - inflight.delete(key); - return entry; - }) - .catch((err) => { - inflight.delete(key); - throw err; - }); + const existing = inflight.get(key); + if (existing) return existing as Promise>; - inflight.set(key, promise as Promise>); - return promise; - }, - }), -}); + const promise = options + .requestAsync(input) + .then((data) => { + if (cache.size >= MAX_CACHE_SIZE) evictExpired(); + if (cache.size >= MAX_CACHE_SIZE) { + const oldest = cache.keys().next().value; + if (oldest) cache.delete(oldest); + } + const entry: CacheEntry = { data, timestamp: new Date(), expiresAt: Date.now() + ttl }; + cache.set(key, entry as CacheEntry); + inflight.delete(key); + return entry; + }) + .catch((err) => { + inflight.delete(key); + throw err; + }); + + inflight.set(key, promise as Promise>); + return promise; + }, + }), + }; +};