Repository navigation
fix: three bug fixes — sourcemap JSON.parse crash, log per_page cap, dashboard pagination overshoot - #1118
fix: three bug fixes — sourcemap JSON.parse crash, log per_page cap, dashboard pagination overshoot#1118cursor[bot] wants to merge 3 commits into
Conversation
…map files The JSON.parse call at line 155 of debug-id.ts was unguarded — if a .map file contained malformed JSON (truncated, corrupt, or not a sourcemap), the entire sourcemap inject command crashed with an opaque SyntaxError. Every other JSON.parse in the codebase is wrapped in try/catch. This adds the same pattern, throwing a descriptive ValidationError that includes the file path so users know which .map file is problematic. Co-authored-by: Miguel Betegón <[email protected]>
Both listLogs() and listTraceLogs() passed the user's --limit value directly as per_page to the Sentry API. When limit > 100, the API silently caps at 100 items, but the caller's hasMore check (logs.length >= flags.limit) evaluates to false, hiding remaining data. This violates the project convention documented in AGENTS.md: 'Never pass a per_page value larger than API_MAX_PER_PAGE to the API.' Cap both calls with Math.min(limit, API_MAX_PER_PAGE) to match the pattern used in autoPaginate() and other list commands. Co-authored-by: Miguel Betegón <[email protected]>
…list When multi-page fetching accumulates more results than flags.limit, the excess items are trimmed with .slice(0, flags.limit) but the API cursor from the last fetched page is still stored. On '-c next', those trimmed items are skipped entirely. This matches the autoPaginate() pattern in infrastructure.ts (lines 419-420) which explicitly drops nextCursor when results overshoot to prevent skipping items. Co-authored-by: Miguel Betegón <[email protected]>
|
Codecov Results 📊❌ Patch coverage is 71.43%. Project has 5049 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 81.24% 81.24% —%
==========================================
Files 388 388 —
Lines 26910 26914 +4
Branches 17481 17483 +2
==========================================
+ Hits 21861 21865 +4
- Misses 5049 5049 —
- Partials 1822 1822 —Generated by Codecov Action |
| const overshot = results.length > flags.limit; | ||
| const hasMore = overshot || !!nextCursor; | ||
| // When multi-page fetch overshoots the limit, the API cursor points past | ||
| // trimmed items — storing it would skip them on '-c next'. Drop it to | ||
| // match autoPaginate() behavior (see infrastructure.ts lines 419-420). | ||
| const cursorToStore = !overshot && hasMore ? nextCursor : undefined; | ||
|
|
There was a problem hiding this comment.
hasMore = true when overshot but no cursor stored causes -c next to throw a ValidationError
When the multi-page fetch overshoots flags.limit, hasMore is set to true and the paginationHint prints a -c next hint, but cursorToStore is undefined, so advancePaginationState stores no next entry in the stack. Running --cursor next then hits state.index + 1 >= state.stack.length in resolveCursor and throws "No next page saved for this query" — the opposite of what hasMore: true implies.
Evidence
overshot = true→cursorToStore = undefined(line 208);hasMore = true(line 203).advancePaginationState(..., direction='first', undefined)storesstack = ['']with no next entry (pagination.ts:265:nextCursor ? ['', nextCursor] : ['']).outputData.hasMore = truecausespaginationHintto emit the-c nexthint (revisions.ts line 229).resolveCursor(..., 'next')inpagination.ts:198checksstate.index + 1 >= state.stack.length→1 >= 1→ throwsValidationError('No next page saved for this query.').autoPaginate()(infrastructure.ts line 419) never exposeshasMoreto callers, so the cited comparison does not apply here wherehasMoreis surfaced directly in CLI output.
Identified by Warden find-bugs · LHQ-E68
|
Closing as part of archiving getsentry/cli; the CLI now lives in getsentry/toolkit. The dashboard revisions cursor fix is ported in getsentry/toolkit#1608 (remaining-budget pages, cursor dropped on overshoot), and the sourcemap JSON guard in getsentry/toolkit#1609. The log |
`sentry dashboard revisions --limit 150` fetched two full pages of 100, trimmed the last 50, and still stored the server cursor. `-c next` then silently skipped those 50 revisions. Later pages now request only the remaining budget, the same way the shared `paginate()` helper does since getsentry/cli#1615. If an endpoint still returns more than requested, the cursor is dropped, matching `autoPaginate`. Ported from the dashboard part of getsentry/cli#1118, which is being closed with the repo archive. <!-- junior-request-attribution:start --> via **BYK**. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:local%3Aweb%3Ad9c5b0de1ecf3c7114be7025 --> -- [View Junior Session in Sentry](https://sentry.sentry.io/explore/conversations/local%3Aweb%3Ad9c5b0de1ecf3c7114be7025/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: BYK <[email protected]>
- `sentry sourcemap inject` crashed with a bare `SyntaxError` when a companion `.map` file wasn't valid JSON. It now throws a `ValidationError` that names the file, before touching the JS bundle. The dry-run path already handled this case. - `formatRelativeTime` printed the literal text "Invalid Date" for a timestamp it couldn't parse. It now shows the same muted dash as a missing timestamp. Ported from getsentry/cli#1118 and getsentry/cli#1200, which are being closed with the repo archive. The other fixes in those PRs were already on main or not worth porting (details in the close comments). <!-- junior-request-attribution:start --> via **BYK**. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:local%3Aweb%3Ad9c5b0de1ecf3c7114be7025 --> -- [View Junior Session in Sentry](https://sentry.sentry.io/explore/conversations/local%3Aweb%3Ad9c5b0de1ecf3c7114be7025/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: BYK <[email protected]>
Summary
Three independent bug fixes discovered via code analysis of the codebase. Each fix is a separate commit.
1.
fix(sourcemap): Guard JSON.parse in injectDebugId against malformed .map filesRoot cause:
JSON.parse(mapContent)insrc/lib/sourcemap/debug-id.ts:155was the only unguardedJSON.parsein the codebase. Malformed.mapfiles crash thesourcemap injectcommand with an opaqueSyntaxError.Reproduction: Run
sentry sourcemap injecton a directory containing a.jsfile whose companion.maphas invalid JSON.Fix: Wrapped in try/catch that throws a
ValidationErrorwith the file path, matching the pattern used ininline-sourcemap.tsand throughout the rest of the codebase.2.
fix(logs): Cap per_page at API_MAX_PER_PAGE in log list APIsRoot cause: Both
listLogs()andlistTraceLogs()insrc/lib/api/logs.tspassed the user's--limitdirectly asper_page. Whenlimit > 100, the API silently caps at 100, but the caller'shasMorecheck (logs.length >= flags.limit) evaluates tofalse, hiding remaining data.Reproduction: Run
sentry log list --limit 200on a project with >100 logs. Only ~100 are returned and the hint says there are no more.Fix: Cap both calls with
Math.min(limit, API_MAX_PER_PAGE), matching the project convention documented in AGENTS.md: "Never pass a per_page value larger than API_MAX_PER_PAGE to the API."3.
fix(dashboard): Drop nextCursor on pagination overshoot in revisions listRoot cause: In
src/commands/dashboard/revisions.ts, when multi-page fetching accumulates more results thanflags.limit, the excess is trimmed but the API cursor from the last fetched page is still stored. On-c next, those trimmed items are silently skipped.Reproduction: Run
sentry dashboard revisions <id> --limit Nwhere N causes multi-page overshoot (e.g.,--limit 150with 100 items per page, where 2 pages return 160 items total).Fix: Drop
nextCursorwhen results overshoot the limit, matching theautoPaginate()pattern ininfrastructure.ts(lines 419-420).