[DEV-113] Make errors in the devintern CLI and desktop app visible in Sentry so failures can be found and fixed quickly - #125
Merged
Merged
Conversation
…p app visible in Sentry so failures can be found and fixed quickly
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task: DEV-113
Summary: Make errors in the devintern CLI and desktop app visible in Sentry so failures can be found and fixed quickly
Implementation Details
Now let me launch parallel explorations of the codebase to audit catch sites while I read key files.
Thorough audit complete. Now let me read the key files I'll modify.
Let me read the remaining catch sites I plan to modify.
Now let me check docs structure, pm-desktop package.json, and how existing tests mock electron.
I now have the full picture. Let me check a few last details (renderer ui components, how
captureErroris imported in ipc.ts context, tests runner config for code package).All research done. Starting implementation.
Now wire redaction into
sentry.ts:Now add redaction tests to the utils sentry test file:
The mock intercepts
init, sobeforeSendnever runs. Let me test thebeforeSendoption directly instead:Utils done. Now the code package — create
lib/sentry-init.ts:Now update
index.ts— replace the localinitSentryOncewith the shared one:That wrapper is convoluted — let me do it cleanly instead:
Now update
worker.ts:Now the webhook-server standalone entry:
Static imports are cleaner for bundling — let me check webhook-server's imports and switch:
Now add
captureErrorat the pipeline catch sites in index.ts. First,processSingleTask:Now the estimation catch and its exit path:
Now the batch loop exit and
main()catch:Now the resolve-conflicts subcommand, PR creation failure, and commit failure:
Now
gracefulShutdownflush:Now the webhook-server review-processing failure. Let me check what's in scope at that catch:
Now pm-desktop. First the shared contract additions:
Now extend
main/error-tracking.tswith context support and dedup:Now update
ipc.ts— thehandle()wrapper and the new renderer-error handler:Now add the
reportRendererErrorhandler inregisterIpcHandlers:Now the preload API addition:
Now
window.ts— renderer crash and preload-error reporting:Now the renderer error-reporting lib:
Now the React error boundary component. Let me check an existing component for style conventions first:
Now wire it all into
main.tsx:Now the tests. First, extend
error-tracking.test.tswith dedup + forwarded-error tests:Now the contract parser test and the renderer error-reporting test:
The
installedguard is process-wide (by design). Let me restructure the tests to use a single install flow:Now the code package
sentry-inittest:All code tests pass. Now typecheck all touched packages:
Two type fixes. First
sentry.ts— simplify the event typing:Now the ErrorBoundary
overridemodifiers:All typechecks pass. Now the pm-desktop full test suite:
Now update the docs. First
docs/code/configuration.md— expand the Error Reporting section:Now run the full verification: pm-desktop tests, code tests, lint, and format:
The "1 error" is pre-existing (baseline shows the same react-dom
windowflake), and failures went from 1 → 0. Now lint and format:Lint clean (0 errors). Now format:
Now run the full monorepo test suite to verify everything together:
Build passes. Let me do a final review of the complete diff:
Code package diff looks right. Now the pm-desktop diff:
Let me check the analytics settings UI copy — it should accurately say the toggle also gates error reporting:
Let me verify the pm-desktop electron-vite build compiles all three bundles:
Build passes. Final verification round after all edits:
All green. Completing the final todo:
DEV-113 is implemented and verified. Summary of what changed and why:
Shared (
@devintern/utils)src/redact.ts(new): defense-in-depth secret scrubbing — token shapes (ghp_*,github_pat_*,sk-ant-*, Bearer headers, URL credentials, credential query params) and secret-shaped context keys.src/sentry.ts:beforeSendnow redactsextra, messages, and exception values before events leave the machine; opt-out behavior unchanged.packages/codelib/sentry-init.ts(initSentryOnce, once-guarded per process) used by the CLI (index.ts),worker.ts(startWorker), and the standalonewebhook-server.tsentry (which also installs fatal handlers + signal flush). All failure exit paths (processSingleTask, batch, estimation,maincatch, SIGINT/SIGTERM, worker shutdown) now flush beforeprocess.exit.captureErrorwithtaskKey/tracker/stage/commandcontext: failed task run (process-task), failed estimation, PR creation failure (both the thrown and silentsuccess:falsepaths), commit failure,resolve-conflictsthrown errors, webhook review swallow + PR-comment queue failures. Intentionally silent (left alone): cleanup (sandboxCleanup), best-effort tracker comments/status transitions, run-recorder, analytics, reactions, gitignore — degraded-but-continuing paths that would be noise. Already reported: process-level fatal handlers,address-review, acquirer spawn/poll/dispatch. No site is double-reported (batch rethrows are captured only inprocessSingleTask; usage-limit deferrals are excluded).pm-desktoplib/error-reporting.tsinstallswindowerror/unhandledrejectionhandlers; newErrorBoundaryreports React crashes (with component stack) and shows a reload fallback. Both forward via a new typed IPC channelpm:report-renderer-error(RendererErrorReportin the contract, validated/bounded byparseRendererErrorReport).handle()wrapper now reports handler failures (deduped viacaptureErrorOnce, 60s window;invalid_input/auth_required/in_progressuser mistakes excluded);render-process-gone(excluding clean-exit) andpreload-errorare reported inwindow.ts. Forwarded errors respect the live telemetry toggle andSENTRY_DISABLED=1because capture stays in main's shared tracking state.Tests & docs
utils/sentry.test.ts(redaction viabeforeSend),utils/redactunit tests,code/tests/sentry-init.test.ts(init-once), pm-desktoperror-tracking.test.ts(context passthrough, dedup, opt-out suppression incl. forwarded errors),shared/renderer-error-report.test.ts(validation/bounding),renderer/src/lib/error-reporting.test.ts(global handlers, drop noise, swallow bridge failures).docs/code/configuration.mdanddocs/pm/configuration.mdError Reporting sections now describe what is reported, what is never reported, and both opt-outs; Settings toggle copy updated to mention error reports.Verification:
bun run lint(0 errors),bun run format,bun run typecheck(all packages),bun run buildfor bothcodeandpm-desktop, and the full monorepo test suite (11/11 tasks, 0 failures — the one pm-desktop "unhandled error" between tests is pre-existing onmain).This PR was automatically created by @devintern/code