From 2576bd4d96ccd8dc3f915795161ded2df320a5a1 Mon Sep 17 00:00:00 2001 From: is0692vs Date: Tue, 17 Mar 2026 20:33:37 +0900 Subject: [PATCH 1/5] fix(web): surface paper stats loading errors --- .../app/papers/[id]/paper-detail-client.tsx | 29 ++++++++++++++----- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/apps/web/src/app/papers/[id]/paper-detail-client.tsx b/apps/web/src/app/papers/[id]/paper-detail-client.tsx index d7a64d0..b424a71 100644 --- a/apps/web/src/app/papers/[id]/paper-detail-client.tsx +++ b/apps/web/src/app/papers/[id]/paper-detail-client.tsx @@ -139,14 +139,27 @@ export default function PaperDetailClient({ paperId }: PaperDetailClientProps) { const fetchStats = useCallback(async () => { if (!paperId || !isAuthor) return; + try { const res = await apiFetch(`/api/papers/${safePath(paperId)}/stats`); - if (res.ok) { - const data = (await res.json()) as PaperStats; - setStats(data); + if (!res.ok) { + if (res.status === 401) { + setStatsError("統計情報を取得するにはログインが必要です"); + } else if (res.status === 403) { + setStatsError("統計情報を閲覧する権限がありません"); + } else if (res.status === 404) { + setStatsError("統計情報が見つかりません"); + } else { + setStatsError("統計情報の取得に失敗しました"); + } + return; } + + const data = (await res.json()) as PaperStats; + setStats(data); + setStatsError(""); } catch { - // Ignore + setStatsError("統計情報の取得に失敗しました"); } }, [paperId, isAuthor]); @@ -250,14 +263,16 @@ export default function PaperDetailClient({ paperId }: PaperDetailClientProps) { return; } + let cancelled = false; setStatsLoading(true); setStatsError(""); - fetchStats(); - // Also set loading false in fetchStats normally, but here we use useEffect style - setStatsLoading(false); + fetchStats().finally(() => { + if (!cancelled) setStatsLoading(false); + }); return () => { + cancelled = true; }; }, [paper?.id, isAuthor, fetchStats]); From 5f08e1f3da0964a196346a1f263e8e46e84707c8 Mon Sep 17 00:00:00 2001 From: is0692vs Date: Tue, 17 Mar 2026 20:33:11 +0900 Subject: [PATCH 2/5] fix(api): tighten user search cache behavior --- apps/api/src/routes/__tests__/users.test.ts | 24 ++++++++++++--------- apps/api/src/routes/users.ts | 23 ++++++++++++++------ 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/apps/api/src/routes/__tests__/users.test.ts b/apps/api/src/routes/__tests__/users.test.ts index 7a388bc..85fc39a 100644 --- a/apps/api/src/routes/__tests__/users.test.ts +++ b/apps/api/src/routes/__tests__/users.test.ts @@ -271,23 +271,27 @@ describe("users routes", () => { const token = await createTestJWT({ sub: "user-1", githubId: "123", name: "User1" }); const app = await createTestApp(); const env = createTestEnv(); + const smallEnv = { ...env, MAX_CACHE_SIZE: "3" } as any; mockDb.select = vi.fn(() => makeQuery({ allResult: [{ id: "user-2", name: "Result 1" }] })); - // We send 1002 requests with unique queries to hit the limit - // (Wait, sending 1000 requests might be slow. Is there a better way? Let's just do it in a Promise.all or similar) - // Alternatively, we can test MAX_CACHE_SIZE by exporting it in test. Since we can't easily, we'll just loop. - const reqs = []; - for (let i = 0; i < 1002; i++) { - reqs.push(app.request(`/api/users/search?q=limit${i}`, { headers: { Authorization: `Bearer ${token}` } }, env as any)); + // Use a small cache size to keep the eviction test fast and deterministic. + for (let i = 0; i < 3; i++) { + await app.request(`/api/users/search?q=limit${i}`, { headers: { Authorization: `Bearer ${token}` } }, smallEnv); } - await Promise.all(reqs); - const finalReq = await app.request("/api/users/search?q=limittrigger", { headers: { Authorization: `Bearer ${token}` } }, env as any); - expect(finalReq.status).toBe(200); + const callCountAfterFill = mockDb.select.mock.calls.length; + + // trigger eviction + await app.request("/api/users/search?q=limittrigger", { headers: { Authorization: `Bearer ${token}` } }, smallEnv); + + // request oldest cached query again + await app.request(`/api/users/search?q=limit0`, { headers: { Authorization: `Bearer ${token}` } }, smallEnv); + + expect(mockDb.select.mock.calls.length).toBeGreaterThan(callCountAfterFill + 1); }); - it("GET /api/users/:id returns 404 when the user does not exist (covered missed line 154)", async () => { + it("GET /api/users/:id returns 404 for missing user profile fetch", async () => { mockDb.select = vi.fn(() => makeQuery({ getResult: null })); const app = await createTestApp(); const env = createTestEnv(); diff --git a/apps/api/src/routes/users.ts b/apps/api/src/routes/users.ts index bd8c6ea..de4e794 100644 --- a/apps/api/src/routes/users.ts +++ b/apps/api/src/routes/users.ts @@ -76,17 +76,28 @@ const MAX_CACHE_SIZE = 1000; function getCachedResults(key: string): any[] | null { const cached = searchCache.get(key); - if (cached && Date.now() - cached.timestamp < CACHE_TTL_MS) { - return cached.data; + if (cached) { + if (Date.now() - cached.timestamp < CACHE_TTL_MS) { + // Re-insert to move to the end (MRU) + searchCache.delete(key); + searchCache.set(key, cached); + return cached.data; + } + + searchCache.delete(key); } - searchCache.delete(key); return null; } -function setCachedResults(key: string, data: any[]) { +function setCachedResults(key: string, data: any[], maxSize?: number) { + const effectiveMaxSize = + typeof maxSize === "number" && Number.isFinite(maxSize) && maxSize > 0 + ? maxSize + : MAX_CACHE_SIZE; + if (searchCache.has(key)) { searchCache.delete(key); - } else if (searchCache.size >= MAX_CACHE_SIZE) { + } else if (searchCache.size >= effectiveMaxSize) { const oldestKey = searchCache.keys().next().value; if (oldestKey !== undefined) { searchCache.delete(oldestKey); @@ -131,7 +142,7 @@ usersRoute.get("/search", authMiddleware, async (c) => { .limit(10) .all(); - setCachedResults(cacheKey, results); + setCachedResults(cacheKey, results, Number(c.env.MAX_CACHE_SIZE ?? MAX_CACHE_SIZE)); return c.json({ users: results }); }); From 1163569653ec1bbb0e581c367759b86d8ecbab1d Mon Sep 17 00:00:00 2001 From: is0692vs Date: Tue, 17 Mar 2026 20:56:26 +0900 Subject: [PATCH 3/5] fix(api): address cache review comments --- apps/api/src/routes/__tests__/users.test.ts | 2 +- apps/api/src/routes/users.ts | 2 +- apps/api/src/types.ts | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/api/src/routes/__tests__/users.test.ts b/apps/api/src/routes/__tests__/users.test.ts index 85fc39a..15c5175 100644 --- a/apps/api/src/routes/__tests__/users.test.ts +++ b/apps/api/src/routes/__tests__/users.test.ts @@ -288,7 +288,7 @@ describe("users routes", () => { // request oldest cached query again await app.request(`/api/users/search?q=limit0`, { headers: { Authorization: `Bearer ${token}` } }, smallEnv); - expect(mockDb.select.mock.calls.length).toBeGreaterThan(callCountAfterFill + 1); + expect(mockDb.select.mock.calls.length).toBe(callCountAfterFill + 2); }); it("GET /api/users/:id returns 404 for missing user profile fetch", async () => { diff --git a/apps/api/src/routes/users.ts b/apps/api/src/routes/users.ts index de4e794..817a621 100644 --- a/apps/api/src/routes/users.ts +++ b/apps/api/src/routes/users.ts @@ -142,7 +142,7 @@ usersRoute.get("/search", authMiddleware, async (c) => { .limit(10) .all(); - setCachedResults(cacheKey, results, Number(c.env.MAX_CACHE_SIZE ?? MAX_CACHE_SIZE)); + setCachedResults(cacheKey, results, c.env.MAX_CACHE_SIZE ? Number(c.env.MAX_CACHE_SIZE) : undefined); return c.json({ users: results }); }); diff --git a/apps/api/src/types.ts b/apps/api/src/types.ts index cb90cd3..d2da5dd 100644 --- a/apps/api/src/types.ts +++ b/apps/api/src/types.ts @@ -10,6 +10,7 @@ export type Env = { ALLOWED_ORIGINS?: string; ENABLE_TEST_AUTH?: string; TEST_AUTH_SECRET?: string; + MAX_CACHE_SIZE?: string; }; export type JwtPayload = { From 928046953703927d8e3949a1cca6a3d2cb586eed Mon Sep 17 00:00:00 2001 From: is0692vs Date: Tue, 17 Mar 2026 21:03:15 +0900 Subject: [PATCH 4/5] test(api): verify MRU promotion in cache eviction --- apps/api/src/routes/__tests__/users.test.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/apps/api/src/routes/__tests__/users.test.ts b/apps/api/src/routes/__tests__/users.test.ts index 15c5175..d2ced06 100644 --- a/apps/api/src/routes/__tests__/users.test.ts +++ b/apps/api/src/routes/__tests__/users.test.ts @@ -282,13 +282,17 @@ describe("users routes", () => { const callCountAfterFill = mockDb.select.mock.calls.length; - // trigger eviction - await app.request("/api/users/search?q=limittrigger", { headers: { Authorization: `Bearer ${token}` } }, smallEnv); + // cache hit should promote limit0 to MRU without new DB query + await app.request(`/api/users/search?q=limit0`, { headers: { Authorization: `Bearer ${token}` } }, smallEnv); + const callCountAfterPromotion = mockDb.select.mock.calls.length; + expect(callCountAfterPromotion).toBe(callCountAfterFill); - // request oldest cached query again + // trigger eviction, then verify limit1 (now oldest) is evicted while limit0 stays cached + await app.request("/api/users/search?q=limittrigger", { headers: { Authorization: `Bearer ${token}` } }, smallEnv); + await app.request(`/api/users/search?q=limit1`, { headers: { Authorization: `Bearer ${token}` } }, smallEnv); await app.request(`/api/users/search?q=limit0`, { headers: { Authorization: `Bearer ${token}` } }, smallEnv); - expect(mockDb.select.mock.calls.length).toBe(callCountAfterFill + 2); + expect(mockDb.select.mock.calls.length).toBe(callCountAfterPromotion + 2); }); it("GET /api/users/:id returns 404 for missing user profile fetch", async () => { From a89a8744b7f1f6dacf4b5edc7123f418aaef94fa Mon Sep 17 00:00:00 2001 From: is0692vs Date: Tue, 17 Mar 2026 21:10:45 +0900 Subject: [PATCH 5/5] Fix stats refetch error handling and cancellation guards --- .../app/papers/[id]/paper-detail-client.tsx | 84 ++++++++++++------- 1 file changed, 55 insertions(+), 29 deletions(-) diff --git a/apps/web/src/app/papers/[id]/paper-detail-client.tsx b/apps/web/src/app/papers/[id]/paper-detail-client.tsx index b424a71..bf3246c 100644 --- a/apps/web/src/app/papers/[id]/paper-detail-client.tsx +++ b/apps/web/src/app/papers/[id]/paper-detail-client.tsx @@ -86,6 +86,8 @@ type PaperStats = { const isAbsoluteUrl = (url: string) => /^https?:\/\//i.test(url); +const STATS_FETCH_ERROR_MESSAGE = "統計情報の取得に失敗しました"; + const isValidExternalUrl = (urlStr: string) => { try { const url = new URL(urlStr); @@ -107,6 +109,7 @@ function formatStatsDateLabel(date: string) { export default function PaperDetailClient({ paperId }: PaperDetailClientProps) { const { user } = useAuth(); const trackedViewPaperIdRef = useRef(null); + const mountedRef = useRef(true); const [paper, setPaper] = useState(null); const [files, setFiles] = useState([]); @@ -137,31 +140,53 @@ export default function PaperDetailClient({ paperId }: PaperDetailClientProps) { const isAuthor = authors.some((a) => a.userId === user?.id); - const fetchStats = useCallback(async () => { - if (!paperId || !isAuthor) return; + const fetchStats = useCallback( + async (options?: { withLoading?: boolean; isCancelled?: () => boolean }) => { + if (!paperId || !isAuthor) return; - try { - const res = await apiFetch(`/api/papers/${safePath(paperId)}/stats`); - if (!res.ok) { - if (res.status === 401) { - setStatsError("統計情報を取得するにはログインが必要です"); - } else if (res.status === 403) { - setStatsError("統計情報を閲覧する権限がありません"); - } else if (res.status === 404) { - setStatsError("統計情報が見つかりません"); - } else { - setStatsError("統計情報の取得に失敗しました"); - } - return; + const canUpdateState = + typeof options?.isCancelled === "function" + ? () => mountedRef.current && !options.isCancelled?.() + : () => mountedRef.current; + + if (options?.withLoading ?? true) { + if (canUpdateState()) setStatsLoading(true); } + if (canUpdateState()) setStatsError(""); - const data = (await res.json()) as PaperStats; - setStats(data); - setStatsError(""); - } catch { - setStatsError("統計情報の取得に失敗しました"); - } - }, [paperId, isAuthor]); + try { + const res = await apiFetch(`/api/papers/${safePath(paperId)}/stats`); + + if (!res.ok) { + if (!canUpdateState()) return; + setStats(null); + if (res.status === 401) { + setStatsError("統計情報を取得するにはログインが必要です"); + } else if (res.status === 403) { + setStatsError("統計情報を閲覧する権限がありません"); + } else if (res.status === 404) { + setStatsError("統計情報が見つかりません"); + } else { + setStatsError(STATS_FETCH_ERROR_MESSAGE); + } + return; + } + + const data = (await res.json()) as PaperStats; + if (!canUpdateState()) return; + + setStats(data); + setStatsError(""); + } catch { + if (!canUpdateState()) return; + setStats(null); + setStatsError(STATS_FETCH_ERROR_MESSAGE); + } finally { + if (canUpdateState()) setStatsLoading(false); + } + }, + [paperId, isAuthor], + ); const applyCountedView = useCallback(() => { setPaper((current) => { @@ -173,7 +198,7 @@ export default function PaperDetailClient({ paperId }: PaperDetailClientProps) { }); // Refresh full stats after a recorded view to ensure consistency - void fetchStats(); + void fetchStats({ withLoading: false }); }, [fetchStats]); const fetchPaper = useCallback(async () => { @@ -225,6 +250,12 @@ export default function PaperDetailClient({ paperId }: PaperDetailClientProps) { if (isUploader) fetchInvites(); }, [isUploader, fetchInvites]); + useEffect(() => { + return () => { + mountedRef.current = false; + }; + }, []); + useEffect(() => { if (!paper?.id || trackedViewPaperIdRef.current === paper.id) return; @@ -264,12 +295,7 @@ export default function PaperDetailClient({ paperId }: PaperDetailClientProps) { } let cancelled = false; - setStatsLoading(true); - setStatsError(""); - - fetchStats().finally(() => { - if (!cancelled) setStatsLoading(false); - }); + void fetchStats({ isCancelled: () => cancelled }); return () => { cancelled = true;