From 4270f2f2f18c238887af9ee625291d3d80156340 Mon Sep 17 00:00:00 2001 From: Thomas Camlong Date: Fri, 7 Aug 2026 22:04:18 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20address=20CodeRabbit=20review=20?= =?UTF-8?q?=E2=80=94=20stream=20tee,=20cache=20guard,=20base64,=20layout?= =?UTF-8?q?=20fallback?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - BoardLayoutFallback: use HomarrLogoWithTitle (no board context required) - integration-data-cache: guard cache.set with placeholder identity to prevent stale write-back after invalidation + test - next-cache-handler: memoize the promise (not resolved handler) to prevent concurrent Redis connections - redis-handler: tee ReadableStream before consuming in set(), hand fresh branch to each get() caller - redis-handler: base64-encode body instead of number[] (3-4x smaller) --- .../app/[locale]/boards/_layout-creator.tsx | 7 ++-- .../api/src/integration-data-cache.spec.ts | 21 ++++++++++++ packages/api/src/integration-data-cache.ts | 6 ++-- packages/next-cache-handler/src/index.ts | 16 ++++----- .../next-cache-handler/src/redis-handler.ts | 33 ++++++++++++++----- 5 files changed, 58 insertions(+), 25 deletions(-) diff --git a/apps/nextjs/src/app/[locale]/boards/_layout-creator.tsx b/apps/nextjs/src/app/[locale]/boards/_layout-creator.tsx index 47b99fe06..f3cf7855c 100644 --- a/apps/nextjs/src/app/[locale]/boards/_layout-creator.tsx +++ b/apps/nextjs/src/app/[locale]/boards/_layout-creator.tsx @@ -12,6 +12,7 @@ import { createLogger } from "@homarr/core/infrastructure/logs"; import { MainHeader } from "~/components/layout/header"; import { BoardLogoWithTitle } from "~/components/layout/logo/board-logo"; +import { HomarrLogoWithTitle } from "~/components/layout/logo/homarr-logo"; import { ClientShell } from "~/components/layout/shell"; import { BoardTourGate } from "~/components/onboarding/board-tour-gate"; import { env } from "~/env"; @@ -31,11 +32,7 @@ interface CreateBoardLayoutProps { const BoardLayoutFallback = ({ headerActions }: { headerActions: JSX.Element }) => ( - } - actions={headerActions} - hasNavigation={false} - /> + } actions={headerActions} hasNavigation={false} /> diff --git a/packages/api/src/integration-data-cache.spec.ts b/packages/api/src/integration-data-cache.spec.ts index 6e463bd19..aa54bb544 100644 --- a/packages/api/src/integration-data-cache.spec.ts +++ b/packages/api/src/integration-data-cache.spec.ts @@ -83,6 +83,27 @@ describe("getCachedIntegrationData", () => { expect(fetcher).toHaveBeenCalledTimes(1); }); + test("invalidation during pending fetch prevents stale write-back", async () => { + let resolveOuter!: (v: { value: string }) => void; + const fetcher = vi.fn( + () => + new Promise<{ value: string }>((resolve) => { + resolveOuter = resolve; + }), + ); + + const p1 = getCachedIntegrationData("integration-a", "widget:race", fetcher); + + invalidateIntegrationDataCache("integration-a"); + resolveOuter({ value: "stale" }); + await p1; + + const freshFetcher = vi.fn(async () => ({ value: "fresh" })); + const result = await getCachedIntegrationData("integration-a", "widget:race", freshFetcher); + expect(result).toEqual({ value: "fresh" }); + expect(freshFetcher).toHaveBeenCalledTimes(1); + }); + test("invalidates all cache entries for an integration", async () => { const fetcherOne = vi.fn(async () => ({ value: "one" })); const fetcherTwo = vi.fn(async () => ({ value: "two" })); diff --git a/packages/api/src/integration-data-cache.ts b/packages/api/src/integration-data-cache.ts index 7af08dbdd..1ade65399 100644 --- a/packages/api/src/integration-data-cache.ts +++ b/packages/api/src/integration-data-cache.ts @@ -56,8 +56,10 @@ export const getCachedIntegrationData = async ( const pending = fetcher() .then((data) => { - cache.set(key, { data, timestamp: Date.now() }); - evictOldestEntry(); + if (cache.get(key) === placeholder) { + cache.set(key, { data, timestamp: Date.now() }); + evictOldestEntry(); + } return data; }) .catch((error) => { diff --git a/packages/next-cache-handler/src/index.ts b/packages/next-cache-handler/src/index.ts index f8052c855..d5dae9b2a 100644 --- a/packages/next-cache-handler/src/index.ts +++ b/packages/next-cache-handler/src/index.ts @@ -1,21 +1,19 @@ import type { CacheHandler } from "next/dist/server/lib/cache-handlers/types"; -let handler: CacheHandler; - -const getHandler = async (): Promise => { - if (handler) return handler; +let handlerPromise: Promise | undefined; +const createHandlerAsync = async (): Promise => { const useRedis = process.env.REDIS_HOST || process.env.REDIS_IS_EXTERNAL === "true"; if (useRedis) { const { RedisCacheHandler } = await import("./redis-handler"); - handler = new RedisCacheHandler(); - } else { - const { createDefaultCacheHandler } = await import("next/dist/server/lib/cache-handlers/default"); - handler = createDefaultCacheHandler(32 * 1024 * 1024); + return new RedisCacheHandler(); } - return handler; + const { createDefaultCacheHandler } = await import("next/dist/server/lib/cache-handlers/default"); + return createDefaultCacheHandler(32 * 1024 * 1024); }; +const getHandler = (): Promise => (handlerPromise ??= createHandlerAsync()); + export default { get: async (...args: Parameters) => (await getHandler()).get(...args), set: async (...args: Parameters) => (await getHandler()).set(...args), diff --git a/packages/next-cache-handler/src/redis-handler.ts b/packages/next-cache-handler/src/redis-handler.ts index 62b23b235..5ece07735 100644 --- a/packages/next-cache-handler/src/redis-handler.ts +++ b/packages/next-cache-handler/src/redis-handler.ts @@ -10,7 +10,7 @@ type StoredEntry = { timestamp: number; expire: number; revalidate: number; - body: number[]; + body: string; }; // eslint-disable-next-line no-underscore-dangle @@ -38,7 +38,7 @@ async function readStreamBody(stream: ReadableStream) { } function entryFromStored(parsed: StoredEntry): CacheEntry { - const body = new Uint8Array(parsed.body); + const body = new Uint8Array(Buffer.from(parsed.body, "base64")); return { value: new ReadableStream({ start(controller) { @@ -91,7 +91,16 @@ export class RedisCacheHandler implements CacheHandler { async get(cacheKey: string, softTags: string[]): Promise { const pending = this.pendingSets.get(cacheKey); - if (pending) return pending; + if (pending) { + try { + const entry = await pending; + const [forCaller, forRest] = entry.value.tee(); + entry.value = forRest; + return { ...entry, value: forCaller }; + } catch { + return undefined; + } + } try { await this.ensureConnected(); @@ -117,14 +126,20 @@ export class RedisCacheHandler implements CacheHandler { } async set(cacheKey: string, pendingEntry: Promise): Promise { - this.pendingSets.set(cacheKey, pendingEntry); + const safeEntry = pendingEntry.then((entry) => { + const [forCallers, forPersist] = entry.value.tee(); + entry.value = forCallers; + return { entry, forPersist }; + }); + const callerPromise = safeEntry.then(({ entry }) => entry); + this.pendingSets.set(cacheKey, callerPromise); try { - const entry = await pendingEntry; - const body = await readStreamBody(entry.value); + const { entry, forPersist } = await safeEntry; + const body = await readStreamBody(forPersist); const ttlMs = entry.expire * 1000; - if (ttlMs <= 0) return; // Dynamic entries (expire: 0) should not be persisted + if (ttlMs <= 0) return; const serialized = JSON.stringify({ tags: entry.tags, @@ -132,7 +147,7 @@ export class RedisCacheHandler implements CacheHandler { timestamp: entry.timestamp, expire: entry.expire, revalidate: entry.revalidate, - body: Array.from(body), + body: Buffer.from(body).toString("base64"), }); await this.ensureConnected(); @@ -140,7 +155,7 @@ export class RedisCacheHandler implements CacheHandler { } catch { // Fail open } finally { - if (this.pendingSets.get(cacheKey) === pendingEntry) { + if (this.pendingSets.get(cacheKey) === callerPromise) { this.pendingSets.delete(cacheKey); } }