KeeperHub / KeeperHub/keeperhub

A non-numeric page on /api/analytics/runs yields NaN, returning zero runs beside a non-zero total

Open Beginner friendly
#2,512 0 comments 0 reactions 0 assignees View on GitHub
accepted bug confirmed
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.