Skip to content

fix(stats): stop reporting oversized session files to Sentry (MAESTRO-M9)#1114

Open
pedramamini wants to merge 1 commit into
rcfrom
fix/sentry-rc-global-stats-oversized-session
Open

fix(stats): stop reporting oversized session files to Sentry (MAESTRO-M9)#1114
pedramamini wants to merge 1 commit into
rcfrom
fix/sentry-rc-global-stats-oversized-session

Conversation

@pedramamini

@pedramamini pedramamini commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

Resolves Sentry issue MAESTRO-M9 - RangeError: Invalid string length in the agentSessions global-stats handler (events on both channel:rc and channel:stable).

The two incremental parse loops in getGlobalStats (agentSessions.ts) called captureException(error) on every parse failure. That includes the expected RangeError: Invalid string length, which V8 throws when fs.readFile(file, 'utf-8') hits a session file too large to read into a single string (> ~512 MB). The result was Sentry noise for a condition we already recover from gracefully: the oversized file is skipped and global stats continue for every other session.

Fix

Treat RangeError as the expected "file too large to parse" condition - log a warning and skip - while still reporting genuinely unexpected errors to Sentry. This mirrors the exact pattern already used in the per-agent storage layer:

  • src/main/storage/claude-session-storage.ts (parseSessionFile)
  • src/main/storage/codex-session-storage.ts (readSessionFile)

Both global-stats loops (Claude + Codex) now follow the same convention. No behavior change for any readable file; this only silences telemetry on an expected, recoverable data condition (consistent with the project's error-handling guidance in CLAUDE.md and the earlier rc telemetry-noise cleanup in #1094).

Validation

  • tsc -p tsconfig.main.json --noEmit - 0 errors
  • eslint + prettier --check on the changed file - clean
  • vitest run src/__tests__/main/ipc/handlers/agentSessions.test.ts - 20/20 pass

Notes

  • Targets rc (the affected code lives on both rc and main; this PR fixes the branch of focus).
  • Other currently-unresolved Sentry issues were triaged and intentionally left out of this PR because they lack an obvious code-level fix: renderer/WebContents crashes (MAESTRO-5A/4Y, real crash symptoms), partition_alloc::OnNoMemoryInternal OOM crashes (MAESTRO-9J/4T/5W), Window unresponsive (MAESTRO-62), the Linux glibc 2.38 packaging error for better-sqlite3 (MAESTRO-RS), a different project's cron (CAM-WORKERS-9), and maestro-p --status (MAESTRO-Q2, previously deliberately deferred).

Summary by CodeRabbit

  • Bug Fixes
    • Improved error handling for oversized session files—large files now log a warning without triggering error reports, preventing unnecessary alerts while maintaining proper logging for other error conditions.

The global-stats incremental parse loops in agentSessions.ts called
captureException() on every parse failure, including the expected
RangeError: Invalid string length thrown when fs.readFile(..., 'utf-8')
hits a session file too large for V8 to read into a single string.

This produced Sentry noise (MAESTRO-M9) for an expected, recoverable
condition: the oversized file is simply skipped and global stats continue
for every other session. Mirror the pattern already used in
claude-session-storage.ts and codex-session-storage.ts - treat RangeError
as 'file too large to parse' (log a warning, skip) and keep reporting
genuinely unexpected errors to Sentry.
@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fb5847b4-8850-49b0-831b-8872274d1154

📥 Commits

Reviewing files that changed from the base of the PR and between 8590bad and d50dc81.

📒 Files selected for processing (1)
  • src/main/ipc/handlers/agentSessions.ts

📝 Walkthrough

Walkthrough

In agentSessions.ts, the error handling inside the agentSessions:getGlobalStats incremental parsing loops for both Claude Code and Codex session files is updated to branch on RangeError. When a RangeError is caught, a "session file too large" warning is logged with filePath and captureException is not called; all other errors continue to invoke captureException and now include error as a structured argument in the warning log.

Changes

RangeError handling for oversized session file parsing

Layer / File(s) Summary
RangeError branching in Claude and Codex parse error handlers
src/main/ipc/handlers/agentSessions.ts
Both the Claude Code and Codex session file catch blocks now check instanceof RangeError to log a size warning with filePath and skip captureException; non-RangeError paths retain captureException and add structured { error } to the warning log.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • RunMaestro/Maestro#585: Adds defensive handling for oversized Claude/Codex session files in the session storage parsers, directly complementing this PR's RangeError catch in the global-stats loop.

Poem

🐇 A file grew so large, it caused quite a fright,
The parser said "RangeError!" and gave up the fight.
But now with a branch and a warning so neat,
Oversized sessions are no longer a defeat.
Sentry stays quiet, the log holds the tale —
No crash, no alert, just a bunny-approved bail! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: stopping oversized session file errors from being reported to Sentry, which directly addresses the fix described in the PR objectives.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sentry-rc-global-stats-oversized-session

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Jun 21, 2026

Copy link
Copy Markdown

Greptile Summary

This PR stops Sentry from receiving RangeError: Invalid string length noise from the getGlobalStats handler in agentSessions.ts. When fs.readFile tries to load a session file that is too large for V8's single-string limit, a RangeError is now caught separately, logged as a warning, and silently skipped rather than forwarded to Sentry.

  • Both the Claude and Codex incremental-parse loops get identical instanceof RangeError guards that mirror the same pattern already established in claude-session-storage.ts and codex-session-storage.ts.
  • All other errors continue to reach captureException, so genuine unexpected failures are still reported.

Confidence Score: 5/5

Safe to merge — the change is a targeted fix duplicated symmetrically for Claude and Codex; all readable files behave identically to before.

The guard is a direct copy of the pattern already validated in both storage modules. Unexpected errors still reach Sentry; only the known, gracefully-handled oversized-file condition is silenced. No logic that affects cache population or streaming updates was touched.

No files require special attention.

Important Files Changed

Filename Overview
src/main/ipc/handlers/agentSessions.ts Adds RangeError guards in both Claude and Codex session parse loops; consistent with existing storage-layer patterns; no behavioral change for readable files.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[fs.readFile session file] -->|success| B[parseClaudeSessionContent / parseCodexSessionContent]
    B -->|success| C[Add session to cache]
    A -->|throws| D{error instanceof RangeError?}
    B -->|throws| D
    D -->|Yes - file too large| E[logger.warn - skip file, no Sentry]
    D -->|No - unexpected error| F[captureException to Sentry]
    F --> G[logger.warn - skip file]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[fs.readFile session file] -->|success| B[parseClaudeSessionContent / parseCodexSessionContent]
    B -->|success| C[Add session to cache]
    A -->|throws| D{error instanceof RangeError?}
    B -->|throws| D
    D -->|Yes - file too large| E[logger.warn - skip file, no Sentry]
    D -->|No - unexpected error| F[captureException to Sentry]
    F --> G[logger.warn - skip file]
Loading

Reviews (1): Last reviewed commit: "fix(stats): don't report oversized sessi..." | Re-trigger Greptile

pedramamini pushed a commit that referenced this pull request Jun 21, 2026
Addresses four stable-channel (main) Sentry issues seen on the current
0.17.x release, all where the bug and path to resolution are clear:

- MAESTRO-RR: the Codex usage sampler reported network "request: fetch
  failed" errors to Sentry (the dominant cause, ~375 events). Offline /
  DNS / TLS / abort-timeout failures are expected for a best-effort
  background sampler, so skip them - the UI still surfaces the error
  state. The existing 401/403 carve-out (un-logged-in CODEX_HOME) stays.

- MAESTRO-M9: the global-stats refresh reported the RangeError ("Invalid
  string length") that V8 throws when a session log is too large to read
  into a single string. Treat it as an expected, recoverable data
  condition (skip the file, log a warning) instead of a Sentry crash -
  mirrors the storage-layer pattern and the rc fix in #1114 so the two
  converge cleanly on the next rc -> main merge.

- MAESTRO-SP: stats:record-shortcut-usage threw "Database not
  initialized" when a shortcut fired before the stats DB finished
  initializing. Skip the fire-and-forget analytics write when the DB
  isn't ready yet (new StatsDB.isReady()).

- MAESTRO-QF: the debounced session-persistence flush reported the main
  process's deliberate "recoverable disk error" (a false return thrown
  only to preserve isPending for retry) to Sentry. Keep that expected
  user-environment condition out of Sentry; genuine flush failures still
  report.

Adds regression tests for RR, SP, and QF.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant