fix: address CodeRabbit review — stream tee, cache guard, base64, layout fallback

- 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)
This commit is contained in:
Thomas Camlong committed 2026-08-07 22:09:11 +02:00
1 parent 00855d4825
commit 4270f2f2f1
5 files changed
+58 -25

No files matched your search

@@ -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<TParams extends Params> {
const BoardLayoutFallback = ({ headerActions }: { headerActions: JSX.Element }) => (
<ClientShell hasNavigation={false}>
<MainHeader
logo={<BoardLogoWithTitle size="md" hideTitleOnMobile />}
actions={headerActions}
hasNavigation={false}
/>
<MainHeader logo={<HomarrLogoWithTitle size="md" />} actions={headerActions} hasNavigation={false} />
<AppShellMain>
<Skeleton height="100%" radius="sm" />
</AppShellMain>
@@ -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" }));
+4 -2
View File
@@ -56,8 +56,10 @@ export const getCachedIntegrationData = async <T>(
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) => {
+7 -9
View File
@@ -1,21 +1,19 @@
import type { CacheHandler } from "next/dist/server/lib/cache-handlers/types";
let handler: CacheHandler;
const getHandler = async (): Promise<CacheHandler> => {
if (handler) return handler;
let handlerPromise: Promise<CacheHandler> | undefined;
const createHandlerAsync = async (): Promise<CacheHandler> => {
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<CacheHandler> => (handlerPromise ??= createHandlerAsync());
export default {
get: async (...args: Parameters<CacheHandler["get"]>) => (await getHandler()).get(...args),
set: async (...args: Parameters<CacheHandler["set"]>) => (await getHandler()).set(...args),
@@ -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<Uint8Array>) {
}
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<CacheEntry | undefined> {
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<CacheEntry>): Promise<void> {
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);
}
}