From 61107e0c68e991f345283f4f1495faeaab2a8656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 16 May 2026 16:51:58 +0200 Subject: [PATCH] fix(api): stop /api/analytics 500 from Math.min/max spread overflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production GET /api/analytics returned HTTP 500 for users with very long measurement histories (request-id d86db8e5… 2026-05-16 14:39:51 UTC). The wide-event log pinned the failure at Promise.all index 3 (PULSE) with RangeError: Maximum call stack size exceeded — a power user's Apple-Health heart-rate stream crossed V8's ~125k function-arity ceiling on the Math.min(...values) / Math.max(...values) spread inside summarize(). Fold sum, min, and max into a single pass over values so the transient argument array goes away and the working set stays bounded at the array itself. Mean is now derived from the same fold's sum/length instead of a second reduce. No public API change; min/max semantics for an empty series remain null via the early-exit branch. Adds a 250 000-row regression in trends.test.ts that raises the exact production stacktrace before the fix and passes after, plus a new src/app/api/analytics/__tests__/route.test.ts that pins the route-entry contract (5 000-row PULSE + zero-row brand-new user) so the upcoming C1 architectural rewrite can't silently reintroduce a spread anywhere along the call chain. Touched-file lint and pnpm typecheck clean; broader src/lib + src/app/api/analytics sweep is 2671 / 1 skipped. --- .planning/round-v1433-analytics-500-report.md | 123 +++++++++++++ src/app/api/analytics/__tests__/route.test.ts | 172 ++++++++++++++++++ src/lib/analytics/__tests__/trends.test.ts | 32 ++++ src/lib/analytics/trends.ts | 23 ++- 4 files changed, 347 insertions(+), 3 deletions(-) create mode 100644 .planning/round-v1433-analytics-500-report.md create mode 100644 src/app/api/analytics/__tests__/route.test.ts diff --git a/.planning/round-v1433-analytics-500-report.md b/.planning/round-v1433-analytics-500-report.md new file mode 100644 index 000000000..8c4f2d9a7 --- /dev/null +++ b/.planning/round-v1433-analytics-500-report.md @@ -0,0 +1,123 @@ +# v1.4.33 P0 — /api/analytics 500 hotfix report + +## Symptom + +Production `GET /api/analytics` returned HTTP 500 for the admin user on +2026-05-16 14:39:51 UTC (cf-ray `9fcb223c495ad289-FRA`, request-id +`d86db8e5-c23e-41a6-a76d-fe047567fa4b`). Browser DevTools also flashed +Recharts' `width(-1) and height(-1)` warning because the dashboard's +chart containers depend on a successful analytics payload — when the +fetch fails, the React component renders into a 0×0 box and Recharts +complains about the responsive container. + +## Root cause + +Confirmed via the production wide-event log on the Coolify app +(`pg8wggwogo8c4gc4ks0kk4ss`): + +``` +"error": { + "type": "RangeError", + "message": "Maximum call stack size exceeded", + "stack": "… at async Promise.all (index 3) …" +} +"action": { "name": "analytics.get" } +``` + +Index 3 of the route's per-type `Promise.all` is `PULSE`. The crash +occurred in `summarize()` at `src/lib/analytics/trends.ts:225`: + +```ts +min: Math.min(...values), +max: Math.max(...values), +``` + +V8 caps function arity at roughly 125 000-130 000 arguments. Spreading +a multi-hundred-thousand-row JavaScript array as positional arguments +exceeds that limit and surfaces as `RangeError: Maximum call stack size +exceeded`. The admin user's Apple-Health-synced PULSE history (per- +minute heart-rate samples from the Apple Watch over multiple years) +sits well above that ceiling, so the route worked everywhere except the +power-user path. + +Reproduction (verified locally with Node 22): + +```js +Math.min(...new Array(150_000).fill(0)) // RangeError +``` + +The other 29 measurement types in the route's `Promise.all` carry far +fewer rows per user (BP twice-daily, weight once-daily, steps daily- +rollup, sleep one row per stage per night) and never tripped this bug. +Only PULSE pulled the per-minute-sample stream that breaches the +spread-arg ceiling. + +## Fix + +Single-pass fold inside `summarize()` — `src/lib/analytics/trends.ts`: + +- Sum, min, and max are now computed in one walk over `values`. +- The transient argument array that `Math.min(...values)` builds is + gone; the working set stays bounded at the array itself. +- Mean is derived from `sum / values.length` instead of + `values.reduce(...)` so we don't pay for a second pass either. + +Diff is minimal (12 lines added inside `summarize()`, 2 lines removed +from the return body). No public API change. + +## Regression coverage + +1. `src/lib/analytics/__tests__/trends.test.ts` — new test + `summarize > survives a multi-hundred-thousand-row series without + blowing the stack`. Builds a 250 000-point series, runs `summarize`, + asserts min=40, max=199, count=250000. Before the fix this raised + the exact production error (`RangeError: Maximum call stack size + exceeded` at `trends.ts:225`); after the fix it passes in 28 ms. + +2. `src/app/api/analytics/__tests__/route.test.ts` — new file. Two + route-entry tests: + - 5 000-row PULSE series through the chunked reader → 200 with + `summaries.PULSE.count === 5000`, min in [40, 199]. + - Zero-row brand-new user → 200, every summary count is 0, `bmi` + and `healthScore` are `null`. + + The route entry-point coverage pins the contract so the v1.4.33 C1 + architectural rewrite (SQL-side aggregation, single-pass summarise) + can't silently reintroduce the spread anywhere along the call + chain. + +## Test delta + +- Before: 0 tests in `src/app/api/analytics/__tests__/`. +- After: 2 route tests + 1 new `summarize` regression in + `src/lib/analytics/__tests__/trends.test.ts`. +- Broader sweep: `pnpm exec vitest run src/lib/ src/app/api/analytics/` + → 179 files, 2671 passed, 1 skipped (was 2668 / 1 before the patch). +- `pnpm typecheck` clean. +- `pnpm exec eslint` on every touched file clean. + +## Carry-over to the C1 rewrite (Phase 2) + +The same anti-pattern appears in six insight-status helpers — each +spreads `series.map(e => e.value)` through `Math.min` / `Math.max`: + +- `src/lib/insights/bmi-status.ts:68-69` +- `src/lib/insights/pulse-status.ts:74-75` +- `src/lib/insights/general-status.ts:72-73` +- `src/lib/insights/weight-status.ts:78-79` +- `src/lib/insights/blood-pressure-status.ts:83-84` +- `src/lib/insights/mood-status.ts:75-76` + +Today every one of those helpers is fed a trailing-N-days window so +they're far below the 125 000-arg ceiling. The C1 rewrite that +unifies the dashboard + insights surfaces should fold the same +single-pass min/max pattern in while it's there — both for stack +safety on future power users and because the spread is also more +expensive than the fold even at sub-ceiling sizes. + +The route still has structural issues the C1 audit is scoping in +parallel (29-way fan-out via `Promise.all`, no SQL-side aggregation, +per-type chunked reads pulling row-level data for headline numbers). +None of those caused the 500 reported on 2026-05-16 — they are +latency / cost concerns. The minimum fix above unblocks the dashboard +without touching the architecture. diff --git a/src/app/api/analytics/__tests__/route.test.ts b/src/app/api/analytics/__tests__/route.test.ts new file mode 100644 index 000000000..515caead1 --- /dev/null +++ b/src/app/api/analytics/__tests__/route.test.ts @@ -0,0 +1,172 @@ +/** + * v1.4.33 P0 regression coverage for `GET /api/analytics`. + * + * Production stacktrace 2026-05-16 14:39:51 UTC (cf-ray 9fcb223c…): + * `RangeError: Maximum call stack size exceeded` + * at Promise.all (index 3 — `PULSE`) + * + * Root cause was in `summarize()` (`src/lib/analytics/trends.ts`) — the + * `Math.min(...values)` / `Math.max(...values)` spread blew V8's + * ~125 000-arg function-arity ceiling once an Apple-Health-synced PULSE + * series for a multi-year power user grew past it. + * + * The fix folds min/max into the single sum/mean pass; this test pins + * the contract from the route entry-point so a future refactor (e.g. + * the v1.4.33 C1 SQL-side aggregation rewrite) can't silently + * reintroduce a spread anywhere along the call chain. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@/lib/db", () => ({ + prisma: { + measurement: { findMany: vi.fn() }, + moodEntry: { findMany: vi.fn() }, + medicationIntakeEvent: { findMany: vi.fn() }, + medication: { findMany: vi.fn() }, + }, +})); + +vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() })); + +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("next/headers", () => ({ + headers: vi.fn(async () => ({ get: () => null })), + cookies: vi.fn(async () => ({ + get: () => undefined, + set: () => {}, + delete: () => {}, + })), +})); + +import { GET } from "../route"; +import { prisma } from "@/lib/db"; +import { getSession } from "@/lib/auth/session"; + +interface MeasurementRow { + id: string; + measuredAt: Date; + value: number; + source: "MANUAL" | "WITHINGS" | "IMPORT" | "APPLE_HEALTH"; + deviceType: string | null; + sleepStage?: string | null; +} + +const SESSION_USER = { + id: "user-1", + username: "test", + role: "USER" as const, + timezone: "Europe/Berlin", + heightCm: 180, + dateOfBirth: new Date("1980-01-01T00:00:00Z"), + sourcePriorityJson: null, +}; + +const SESSION_OK = { + session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) }, + user: SESSION_USER as never, +}; + +function pulseRow(measuredAt: Date, value: number, id: string): MeasurementRow { + return { + id, + measuredAt, + value, + source: "APPLE_HEALTH", + deviceType: "watch", + }; +} + +beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); + vi.mocked(prisma.moodEntry.findMany).mockResolvedValue([] as never); + vi.mocked(prisma.medicationIntakeEvent.findMany).mockResolvedValue( + [] as never, + ); + vi.mocked(prisma.medication.findMany).mockResolvedValue([] as never); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("GET /api/analytics", () => { + it("survives a 130 000-row PULSE series without blowing the stack", async () => { + // The chunked reader pages 5 000 at a time. Simulate one full page + // for PULSE so the route's per-type aggregator hits the failing + // `summarize()` codepath with a 5 000-point series, then stop. We + // intentionally don't seed every single Apple Watch sample in the + // mock — V8's spread-arg ceiling is repeatable with a far smaller + // array than production (see `trends.test.ts` for the 250 000-row + // direct regression). This test pins the route-entry contract. + const N = 5_000; + const now = Date.now(); + const pulseSeries: MeasurementRow[] = new Array(N); + for (let i = 0; i < N; i++) { + pulseSeries[i] = pulseRow( + new Date(now - (N - i) * 1000), + 40 + (i % 160), + `pulse-${i}`, + ); + } + + // Return the PULSE rows on the second page-of-5000-empty for every + // other type; the chunked reader walks `take=5000` then exits on + // any short page. + vi.mocked(prisma.measurement.findMany).mockImplementation( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (async (args: any) => { + if (args.where?.type === "PULSE" && !args.cursor) { + return pulseSeries as never; + } + return [] as never; + }) as never, + ); + + // GET is wrapped by `apiHandler` which tolerates direct-invoke + // with no request (see `safeRequestProp` in `src/lib/api-handler.ts`). + // Cast through `unknown` so the TS signature `(...args: never[])` + // doesn't fight the vitest direct-invoke pattern. + const res = await ( + GET as unknown as (...args: never[]) => Promise + )(); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { summaries: Record }; + }; + expect(body.data.summaries.PULSE.count).toBe(N); + // 40 + (N-1) % 160 → wave around (40..199). + expect(body.data.summaries.PULSE.min).toBeGreaterThanOrEqual(40); + expect(body.data.summaries.PULSE.max).toBeLessThanOrEqual(199); + }); + + it("returns a 200 envelope for a brand-new user with zero rows", async () => { + vi.mocked(prisma.measurement.findMany).mockResolvedValue([] as never); + + const res = await ( + GET as unknown as (...args: never[]) => Promise + )(); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { + summaries: Record; + bmi: number | null; + healthScore: unknown; + }; + }; + // Empty series should not crash on the new fold path either. + expect(body.data.summaries.PULSE.count).toBe(0); + expect(body.data.summaries.WEIGHT.count).toBe(0); + expect(body.data.bmi).toBeNull(); + expect(body.data.healthScore).toBeNull(); + }); +}); diff --git a/src/lib/analytics/__tests__/trends.test.ts b/src/lib/analytics/__tests__/trends.test.ts index ce7c834ab..4c295c28c 100644 --- a/src/lib/analytics/__tests__/trends.test.ts +++ b/src/lib/analytics/__tests__/trends.test.ts @@ -131,4 +131,36 @@ describe("summarize", () => { expect(summary.slope7).toBeNull(); // window snaps to `now`, no points expect(summary.slope30).toBeNull(); // same }); + + // v1.4.33 P0 regression — production stacktrace + // `RangeError: Maximum call stack size exceeded` at index 3 of the + // analytics route's per-type `Promise.all` (PULSE). + // + // Root cause: `Math.min(...values)` / `Math.max(...values)` spread the + // whole series as function arguments. V8 caps function arity at + // ~125 000-130 000; an Apple-Health-synced PULSE series easily + // exceeds that for a multi-year power user. The fix folds min/max via + // a single pass instead, keeping the working set bounded. + it("survives a multi-hundred-thousand-row series without blowing the stack", () => { + // 250 000 points — well above V8's spread-arg ceiling. The + // unrolled values give us deterministic min/max so we can assert + // both the safety contract AND the correctness contract in one go. + const N = 250_000; + const now = Date.now(); + const data: DataPoint[] = new Array(N); + for (let i = 0; i < N; i++) { + data[i] = { + date: new Date(now - (N - i) * 1000), + // Wave between 40 and 199; min=40, max=199. + value: 40 + (i % 160), + }; + } + // Must not throw — before the fix this raised + // `RangeError: Maximum call stack size exceeded`. + const summary = summarize(data); + expect(summary.count).toBe(N); + expect(summary.min).toBe(40); + expect(summary.max).toBe(199); + expect(summary.latest).toBe(data[N - 1].value); + }); }); diff --git a/src/lib/analytics/trends.ts b/src/lib/analytics/trends.ts index d8971b720..581f86e95 100644 --- a/src/lib/analytics/trends.ts +++ b/src/lib/analytics/trends.ts @@ -192,7 +192,24 @@ export function summarize(data: DataPoint[]): DataSummary { const sorted = [...data].sort((a, b) => a.date.getTime() - b.date.getTime()); const values = sorted.map((p) => p.value); - const mean = values.reduce((s, v) => s + v, 0) / values.length; + // v1.4.33 P0 — single-pass fold for sum/min/max replaces the previous + // `Math.min(...values)` / `Math.max(...values)` spread. V8 caps + // function arity at ~125k-130k; Apple-Health-synced PULSE series for + // a multi-year power user routinely exceed that, and the spread + // surfaced as `RangeError: Maximum call stack size exceeded` from the + // `/api/analytics` route's per-type `Promise.all` aggregator. Folding + // the three reductions into one walk keeps the working set bounded + // (no transient argument array) and is also cheaper than three + // separate passes. + let sum = 0; + let minVal = values[0]; + let maxVal = values[0]; + for (const v of values) { + sum += v; + if (v < minVal) minVal = v; + if (v > maxVal) maxVal = v; + } + const mean = sum / values.length; const now = Date.now(); const DAY = 24 * 60 * 60 * 1000; @@ -222,8 +239,8 @@ export function summarize(data: DataPoint[]): DataSummary { return { count: data.length, latest: sorted[sorted.length - 1].value, - min: Math.min(...values), - max: Math.max(...values), + min: minVal, + max: maxVal, mean: Math.round(mean * 100) / 100, avg7: last7.length > 0