KeeperHub / KeeperHub/keeperhub
A non-numeric page on /api/analytics/runs yields NaN, returning zero runs beside a non-zero total
- Dominant language
- TypeScript
- Stars
- 24
- Forks
- 93
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 266
Description
Found while reviewing #2505, which is what makes `page` a documented public parameter.
**Reason.** `app/api/analytics/runs/route.ts:34` is `const page = pageParam ? Math.max(1, Number(pageParam)) : undefined;`. `Math.max(1, NaN)` is `NaN`, so any non-numeric `page` passes straight through instead of being clamped or rejected.
Downstream, `lib/analytics/queries.ts:1451` computes `offset = (NaN - 1) * pageLimit`, `:1500` calls `slice(NaN, NaN)` which returns an empty array, and `:1509` echoes `page` back, which `JSON.stringify` renders as `null`. So `GET /api/analytics/runs?page=abc` returns zero runs alongside a non-zero `total` and `page: null` - which reads as data loss rather than as a rejected parameter. An empty `?page=` from a templated client hits the same path.
This is reachable today, but #2505 is what advertises `page` in the public docs, so it moves from a parameter nobody knew about to one integrators will send.
**Scope.** `app/api/analytics/runs/route.ts`, the `page` parse. The guard already exists one file over: `lib/analytics/parse-run-filters.ts:27-32` `parseNonNegativeInt` does `Number.isFinite(value) && value >= 0 ? Math.floor(value) : undefined`, which is exactly the shape needed.
Does not touch the query, the response shape, or any other parameter. Note `limit` on the line below has the same unguarded `Number()` and is worth checking in the same pass, though it behaves differently because `queries.ts:1449` caps it.
Out of scope: whether a bad parameter should be a 400 rather than a silent fallback to the default. That is a wider API-convention question across the analytics routes, not something to settle here.
**Plan.** Reuse the `parseNonNegativeInt` shape - parse, test `Number.isFinite`, floor, and fall back to `undefined` when it fails, so a malformed `page` behaves exactly as an absent one. A test asserting `?page=abc` returns the first page rather than an empty one would pin it.
Contributor guide
Research direction
Start at app/api/analytics/runs/route.ts:34 and compare its page parsing with lib/analytics/parse-run-filters.ts:27-32. Add or update coverage for GET /api/analytics/runs?page=abc, confirming it returns the first page like an absent parameter; check the adjacent limit parsing without expanding the stated scope.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100