mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-15 06:52:04 +00:00
fix(insights,cache): surface revalidation to clients and evict on interactive writes
Stale-served insights responses now carry a revalidating flag; the advisor and comprehensive queries poll on a bounded 25 s cadence until the background warm lands, so an open session converges instead of pinning yesterday's briefing for an hour. Interactive measurement writes (single create, edit, delete, bulk-delete, restore) hard-evict the analytics cache so a manually entered value is reflected on the next read; background batch sync keeps the mark-stale path. The visit-triggered force warm collapses to one job per user instead of one per locale, and the umami script proxy failure now annotates a named action with the failure reason instead of a bare flag.
This commit is contained in:
1 parent
78fe82e348
commit
03453aaed2
23 files changed
+461
-55
No files matched your search
@@ -51,7 +51,7 @@ export const GET = apiHandler(async () => {
|
||||
// the former rate-limit bucket was an unconditional Postgres UPSERT
|
||||
// per request, i.e. the limiter WAS the per-request DB load it claimed
|
||||
// to prevent. The route stays authenticated; the only uncached work is
|
||||
// one `appSettings` read per 60 s process-wide.
|
||||
// one `appSettings` read per 10 min (the bucket's TTL) process-wide.
|
||||
|
||||
// Cache the global app-settings shape on a singleton key (per blueprint §3).
|
||||
// `isAdmin` lives outside the cache because it varies per request. The
|
||||
|
||||
@@ -91,7 +91,7 @@ import { getSession } from "@/lib/auth/session";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { buildComprehensiveAggregate } from "@/lib/insights/comprehensive-aggregator";
|
||||
import { checkAnalyticsReadRateLimit } from "@/lib/rate-limit";
|
||||
import { __resetAllCachesForTests } from "@/lib/cache/server-cache";
|
||||
import { __resetAllCachesForTests, caches } from "@/lib/cache/server-cache";
|
||||
|
||||
const SESSION_OK = {
|
||||
session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) },
|
||||
@@ -438,3 +438,64 @@ describe("GET /api/insights/comprehensive — envelope shape", () => {
|
||||
expect(body.data.weightBpCorrelation?.n).toBe(5);
|
||||
});
|
||||
});
|
||||
|
||||
// v1.16.7 — the route serves a marked-stale cache entry immediately
|
||||
// (stale-while-revalidate) and must mark that body `revalidating: true`
|
||||
// so the client can poll until the background rebuild lands. A fresh
|
||||
// (cold or warm) read carries `revalidating: false`.
|
||||
describe("GET /api/insights/comprehensive — stale-while-revalidate marker", () => {
|
||||
const EMPTY_AGGREGATE = {
|
||||
summaries: {},
|
||||
bpRawRows: { sys: [], dia: [] },
|
||||
dailyByType: {},
|
||||
firstMeasurementAt: null,
|
||||
totalMeasurements: 0,
|
||||
};
|
||||
|
||||
it("marks a cold (inline-computed) read revalidating: false", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
(
|
||||
buildComprehensiveAggregate as ReturnType<typeof vi.fn>
|
||||
).mockResolvedValue(EMPTY_AGGREGATE);
|
||||
|
||||
const res = await callGet(makeReq());
|
||||
expect(res.status).toBe(200);
|
||||
const body = (await res.json()) as { data: { revalidating: boolean } };
|
||||
expect(body.data.revalidating).toBe(false);
|
||||
});
|
||||
|
||||
it("marks a stale-served read revalidating: true and a converged read false again", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
(
|
||||
buildComprehensiveAggregate as ReturnType<typeof vi.fn>
|
||||
).mockResolvedValue(EMPTY_AGGREGATE);
|
||||
|
||||
// Prime the cache entry (cold compute).
|
||||
await callGet(makeReq());
|
||||
|
||||
// A measurement sync marks the bucket stale (the invalidate helper's
|
||||
// default posture) — the next read serves the prior body + the flag.
|
||||
caches.analytics.markStaleByPrefix(`${SESSION_OK.user.id}|`);
|
||||
const staleRes = await callGet(makeReq());
|
||||
expect(staleRes.status).toBe(200);
|
||||
const staleBody = (await staleRes.json()) as {
|
||||
data: { revalidating: boolean };
|
||||
};
|
||||
expect(staleBody.data.revalidating).toBe(true);
|
||||
|
||||
// The stale read kicked off a background rebuild; once it settles
|
||||
// (the cache entry is fresh again) the next read is a plain hit and
|
||||
// the flag drops so the poll stops.
|
||||
await vi.waitFor(() => {
|
||||
const entry = caches.analytics.getAllowStale(
|
||||
`${SESSION_OK.user.id}|comprehensive`,
|
||||
);
|
||||
expect(entry?.stale).toBe(false);
|
||||
});
|
||||
const freshRes = await callGet(makeReq());
|
||||
const freshBody = (await freshRes.json()) as {
|
||||
data: { revalidating: boolean };
|
||||
};
|
||||
expect(freshBody.data.revalidating).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -25,7 +25,11 @@ import { apiHandler, requireAuth, type AuthContext } from "@/lib/api-handler";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { requireAssistantSurface } from "@/lib/feature-flags";
|
||||
import { checkAnalyticsReadRateLimit } from "@/lib/rate-limit";
|
||||
import { cachedSwr, caches, type ServerCache } from "@/lib/cache/server-cache";
|
||||
import {
|
||||
cachedSwrWithMeta,
|
||||
caches,
|
||||
type ServerCache,
|
||||
} from "@/lib/cache/server-cache";
|
||||
import { buildComprehensiveAggregate } from "@/lib/insights/comprehensive-aggregator";
|
||||
import {
|
||||
ensureUserMoodRollupsFresh,
|
||||
@@ -63,7 +67,13 @@ export const GET = apiHandler(async () => {
|
||||
// rebuild on the very next mount, all day). The truly-cold first
|
||||
// read of the day still computes inline — there is no prior body to
|
||||
// serve — but it no longer recurs per visit.
|
||||
const body = await cachedSwr(
|
||||
//
|
||||
// The `revalidating` flag marks a stale-served body honestly so the
|
||||
// client can poll (bounded) until the background rebuild lands —
|
||||
// mirroring the `/api/insights/generate` GET contract. It rides on
|
||||
// the response, never inside the cached body, so a warmed entry
|
||||
// doesn't freeze the flag.
|
||||
const { value: body, outcome } = await cachedSwrWithMeta(
|
||||
caches.analytics as ServerCache<Awaited<
|
||||
ReturnType<typeof buildComprehensiveResponse>
|
||||
>>,
|
||||
@@ -72,7 +82,7 @@ export const GET = apiHandler(async () => {
|
||||
annotate,
|
||||
);
|
||||
|
||||
return apiSuccess(body);
|
||||
return apiSuccess({ ...body, revalidating: outcome === "stale" });
|
||||
});
|
||||
|
||||
type AuthedUser = AuthContext["user"];
|
||||
|
||||
@@ -448,14 +448,16 @@ describe("GET /api/insights/generate — read-only advisor read", () => {
|
||||
);
|
||||
expect(res.status).toBe(200);
|
||||
const body = (await res.json()) as {
|
||||
data: { insights: unknown; cached: boolean };
|
||||
data: { insights: unknown; cached: boolean; revalidating: boolean };
|
||||
};
|
||||
expect(body.data.cached).toBe(true);
|
||||
expect(body.data.insights).toEqual(cached);
|
||||
// No completion is ever run on the read path.
|
||||
expect(resolveProvider).not.toHaveBeenCalled();
|
||||
// A fresh cache (just now) does not trigger a warm.
|
||||
// A fresh cache (just now) does not trigger a warm — and the payload
|
||||
// says so, so the client never starts a convergence poll.
|
||||
expect(enqueueForceWarm).not.toHaveBeenCalled();
|
||||
expect(body.data.revalidating).toBe(false);
|
||||
});
|
||||
|
||||
it("enqueues an out-of-band warm when the cache is stale and a provider exists", async () => {
|
||||
@@ -475,6 +477,10 @@ describe("GET /api/insights/generate — read-only advisor read", () => {
|
||||
locale: "en",
|
||||
});
|
||||
expect(resolveProvider).not.toHaveBeenCalled();
|
||||
// The stale serve is honest: `revalidating: true` rides on the
|
||||
// payload so the client polls (bounded) until the warm lands.
|
||||
const body = (await res.json()) as { data: { revalidating: boolean } };
|
||||
expect(body.data.revalidating).toBe(true);
|
||||
});
|
||||
|
||||
it("returns an empty payload (no warm) on a cold cache without a provider", async () => {
|
||||
@@ -490,10 +496,12 @@ describe("GET /api/insights/generate — read-only advisor read", () => {
|
||||
);
|
||||
expect(res.status).toBe(200);
|
||||
const body = (await res.json()) as {
|
||||
data: { insights: unknown; cached: boolean };
|
||||
data: { insights: unknown; cached: boolean; revalidating: boolean };
|
||||
};
|
||||
expect(body.data.cached).toBe(false);
|
||||
expect(body.data.insights).toBeNull();
|
||||
expect(enqueueForceWarm).not.toHaveBeenCalled();
|
||||
// No warm enqueued → no convergence poll for the client to run.
|
||||
expect(body.data.revalidating).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -313,6 +313,11 @@ export const GET = apiHandler(async (request: NextRequest) => {
|
||||
cached: true,
|
||||
cachedAt,
|
||||
legacyPayload,
|
||||
// Honest stale-serve marker: true while the out-of-band warm is
|
||||
// in flight, so the client can poll (bounded) until the fresh
|
||||
// briefing lands instead of sitting on the stale payload for the
|
||||
// rest of the session.
|
||||
revalidating,
|
||||
});
|
||||
} catch {
|
||||
// Invalid cache row — fall through to the empty payload below. The
|
||||
@@ -325,7 +330,12 @@ export const GET = apiHandler(async (request: NextRequest) => {
|
||||
action: { name: "insights.generate.read" },
|
||||
meta: { cached: false, revalidating },
|
||||
});
|
||||
return apiSuccess({ insights: null, cached: false, legacyPayload: false });
|
||||
return apiSuccess({
|
||||
insights: null,
|
||||
cached: false,
|
||||
legacyPayload: false,
|
||||
revalidating,
|
||||
});
|
||||
});
|
||||
|
||||
export const POST = apiHandler(async (request: NextRequest) => {
|
||||
|
||||
@@ -148,8 +148,9 @@ export const PUT = apiHandler(
|
||||
});
|
||||
|
||||
// v1.4.34 IW-G — bust per-user analytics + achievements + workouts
|
||||
// caches so subsequent reads reflect the edited row.
|
||||
invalidateUserMeasurements(user.id);
|
||||
// caches so subsequent reads reflect the edited row. Interactive
|
||||
// edit — hard-evict so the SWR readers don't serve the pre-edit body.
|
||||
invalidateUserMeasurements(user.id, { evict: true });
|
||||
|
||||
// v1.5.0 — refresh the rollup row for the affected day. When the
|
||||
// measuredAt moved across day boundaries (or the row was re-typed)
|
||||
@@ -236,8 +237,10 @@ export const DELETE = apiHandler(
|
||||
});
|
||||
|
||||
// v1.4.34 IW-G — bust per-user analytics + achievements + workouts
|
||||
// caches so subsequent reads reflect the deletion.
|
||||
invalidateUserMeasurements(user.id);
|
||||
// caches so subsequent reads reflect the deletion. Interactive
|
||||
// delete — hard-evict so the SWR readers don't serve the pre-delete
|
||||
// body.
|
||||
invalidateUserMeasurements(user.id, { evict: true });
|
||||
|
||||
// v1.5.0 — refresh the rollup row for the affected day (the
|
||||
// recompute drops the row when the day's measurement count goes
|
||||
|
||||
@@ -125,7 +125,11 @@ describe("POST /api/measurements/bulk-delete", () => {
|
||||
});
|
||||
expect(call.data.deletedAt).toBeInstanceOf(Date);
|
||||
|
||||
expect(invalidateUserMeasurements).toHaveBeenCalledWith("user-1");
|
||||
// v1.16.7 — interactive write: hard-evict so the SWR readers
|
||||
// never serve the pre-write analytics body back to the user.
|
||||
expect(invalidateUserMeasurements).toHaveBeenCalledWith("user-1", {
|
||||
evict: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("collapses the rollup recompute to the unique (type, day) set, not per-row", async () => {
|
||||
|
||||
@@ -105,8 +105,10 @@ async function postBulkDelete(request: NextRequest): Promise<Response> {
|
||||
|
||||
if (count > 0) {
|
||||
// v1.4.34 IW-G — bust per-user analytics + achievements + workouts
|
||||
// caches so subsequent reads reflect the deletions.
|
||||
invalidateUserMeasurements(user.id);
|
||||
// caches so subsequent reads reflect the deletions. Interactive
|
||||
// multi-select delete from the management list — hard-evict so the
|
||||
// SWR readers don't serve the pre-delete body.
|
||||
invalidateUserMeasurements(user.id, { evict: true });
|
||||
|
||||
// Collapse the deleted rows to the unique `(type, day)` set BEFORE
|
||||
// recomputing so a 200-row delete spanning one day fires ~1 recompute
|
||||
|
||||
@@ -122,7 +122,11 @@ describe("POST /api/measurements/restore", () => {
|
||||
syncVersion: { increment: 1 },
|
||||
});
|
||||
|
||||
expect(invalidateUserMeasurements).toHaveBeenCalledWith("user-1");
|
||||
// v1.16.7 — interactive write: hard-evict so the SWR readers
|
||||
// never serve the pre-write analytics body back to the user.
|
||||
expect(invalidateUserMeasurements).toHaveBeenCalledWith("user-1", {
|
||||
evict: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("collapses the rollup recompute to the unique (type, day) set, not per-row", async () => {
|
||||
|
||||
@@ -106,8 +106,9 @@ async function postRestore(request: NextRequest): Promise<Response> {
|
||||
|
||||
if (count > 0) {
|
||||
// Bust per-user analytics + achievements + workouts caches so
|
||||
// subsequent reads reflect the restored rows.
|
||||
invalidateUserMeasurements(user.id);
|
||||
// subsequent reads reflect the restored rows. Interactive undo —
|
||||
// hard-evict so the SWR readers don't serve the pre-restore body.
|
||||
invalidateUserMeasurements(user.id, { evict: true });
|
||||
|
||||
// Collapse the restored rows to the unique `(type, day)` set BEFORE
|
||||
// recomputing — mirrors the bulk-delete path. Best-effort: a
|
||||
|
||||
@@ -916,8 +916,10 @@ async function postMeasurement(request: NextRequest) {
|
||||
});
|
||||
|
||||
// v1.4.34 IW-G — flush every cache that reflects this user's
|
||||
// measurement set so the next read paints the new rows.
|
||||
invalidateUserMeasurements(user.id);
|
||||
// measurement set so the next read paints the new rows. Interactive
|
||||
// multi-entry create (combined BP + pulse form) — hard-evict so the
|
||||
// SWR readers don't serve the pre-write body back to the user.
|
||||
invalidateUserMeasurements(user.id, { evict: true });
|
||||
|
||||
// v1.5.0 — refresh the persistent rollup table for every distinct
|
||||
// (type, day) the batch touched so the next analytics / coach read
|
||||
@@ -1027,8 +1029,10 @@ async function postMeasurement(request: NextRequest) {
|
||||
});
|
||||
|
||||
// v1.4.34 IW-G — flush every cache that reflects this user's
|
||||
// measurement set so the next read paints the new row.
|
||||
invalidateUserMeasurements(user.id);
|
||||
// measurement set so the next read paints the new row. Interactive
|
||||
// single-entry create — hard-evict so the SWR readers don't serve the
|
||||
// pre-write body back to the user.
|
||||
invalidateUserMeasurements(user.id, { evict: true });
|
||||
|
||||
// v1.5.0 — refresh the persistent rollup row for the affected
|
||||
// (type, day) tuple. Runs inline so the next read of the
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
/**
|
||||
* Fail-soft contract for the Umami proxy route: an unreachable / refused
|
||||
* upstream degrades to the noop script (200, never a 500) AND stays
|
||||
* observable — the catch annotates a proper wide-event action
|
||||
* (`monitoring.umami_script.fetch_failed`) with a truncated reason so
|
||||
* dashboards can see the degraded state.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
vi.mock("@/lib/api-handler", () => ({
|
||||
apiHandler: <T extends (...args: unknown[]) => unknown>(fn: T) => fn,
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/logging/context", () => ({
|
||||
annotate: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/monitoring-settings", () => ({
|
||||
getPublicMonitoringSettings: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/safe-fetch", () => ({
|
||||
safeFetch: vi.fn(),
|
||||
}));
|
||||
|
||||
import { GET } from "../route";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { getPublicMonitoringSettings } from "@/lib/monitoring-settings";
|
||||
import { safeFetch } from "@/lib/safe-fetch";
|
||||
|
||||
const callGet = GET as unknown as () => Promise<Response>;
|
||||
|
||||
const ENABLED_SETTINGS = {
|
||||
umamiEnabled: true,
|
||||
umamiScriptUrl: "https://stats.example.com/script.js",
|
||||
umamiWebsiteId: "site-1",
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(getPublicMonitoringSettings).mockResolvedValue(
|
||||
ENABLED_SETTINGS as never,
|
||||
);
|
||||
});
|
||||
|
||||
describe("GET /api/monitoring/umami-script — fail-soft observability", () => {
|
||||
it("serves the noop script (200) and annotates the failure action when the upstream fetch throws", async () => {
|
||||
vi.mocked(safeFetch).mockRejectedValue(
|
||||
new Error("connect ETIMEDOUT 192.0.2.10:443"),
|
||||
);
|
||||
|
||||
const res = await callGet();
|
||||
expect(res.status).toBe(200);
|
||||
expect(await res.text()).toContain("umami disabled");
|
||||
|
||||
// The fail-soft must stay observable: a proper
|
||||
// `<surface>.<noun>.<verb>` action + a truncated reason, not a bare
|
||||
// meta flag.
|
||||
expect(annotate).toHaveBeenCalledWith({
|
||||
action: { name: "monitoring.umami_script.fetch_failed" },
|
||||
meta: { reason: "connect ETIMEDOUT 192.0.2.10:443" },
|
||||
});
|
||||
});
|
||||
|
||||
it("truncates an oversized error message in the reason meta", async () => {
|
||||
vi.mocked(safeFetch).mockRejectedValue(new Error("x".repeat(500)));
|
||||
|
||||
const res = await callGet();
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
const failureCall = vi
|
||||
.mocked(annotate)
|
||||
.mock.calls.find(
|
||||
(call) =>
|
||||
call[0]?.action?.name === "monitoring.umami_script.fetch_failed",
|
||||
);
|
||||
expect(failureCall).toBeTruthy();
|
||||
expect(
|
||||
(failureCall?.[0]?.meta as { reason: string }).reason,
|
||||
).toHaveLength(200);
|
||||
});
|
||||
|
||||
it("serves the upstream script untouched on a healthy fetch (no failure action)", async () => {
|
||||
vi.mocked(safeFetch).mockResolvedValue(
|
||||
new Response("/* real script */", { status: 200 }) as never,
|
||||
);
|
||||
|
||||
const res = await callGet();
|
||||
expect(res.status).toBe(200);
|
||||
expect(await res.text()).toBe("/* real script */");
|
||||
|
||||
const failureCall = vi
|
||||
.mocked(annotate)
|
||||
.mock.calls.find(
|
||||
(call) =>
|
||||
call[0]?.action?.name === "monitoring.umami_script.fetch_failed",
|
||||
);
|
||||
expect(failureCall).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -43,8 +43,19 @@ export const GET = apiHandler(async () => {
|
||||
// the input-time `isPublicUrl` accept and the socket connect.
|
||||
{ requirePublicHost: true },
|
||||
);
|
||||
} catch {
|
||||
annotate({ meta: { umami_script_fetch_failed: true } });
|
||||
} catch (err) {
|
||||
// Fail-soft stays a 200, but the failure must stay observable:
|
||||
// a proper action name keeps the degraded state visible on the
|
||||
// wide-event dashboards instead of hiding behind a bare meta flag.
|
||||
annotate({
|
||||
action: { name: "monitoring.umami_script.fetch_failed" },
|
||||
meta: {
|
||||
reason:
|
||||
err instanceof Error
|
||||
? err.message.slice(0, 200)
|
||||
: String(err).slice(0, 200),
|
||||
},
|
||||
});
|
||||
return new NextResponse(NOOP_SCRIPT, {
|
||||
status: 200,
|
||||
headers: {
|
||||
|
||||
Reference in new issue
Block a user