fix(seer): add debug logging to silent catch blocks in seer-trial - #1460
Merged
Conversation
promptAndStartTrial had two catch blocks that silently discarded error objects. The trial availability check (getProductTrials) silently returned false on any error, and the trial start (startProductTrial) showed a user-facing warning but discarded the actual error object. Per AGENTS.md, every catch block must surface the error for diagnostic visibility. Add log.debug() calls so the underlying error (network failure, auth issue, unexpected response) is visible with --verbose. Update the test mock to include the debug method. Co-authored-by: Miguel Betegón <miguelbetegongarcia@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
BYK
approved these changes
Aug 25, 2026
BYK
marked this pull request as ready for review
August 25, 2026 08:49
jared-outpost Bot
added a commit
that referenced
this pull request
Aug 25, 2026
## What Makes error-handling enforcement systemic, per #1470. The repo already had `check:errors` (`packages/cli/script/check-error-patterns.ts`) wired into CI, but its silent-catch scan was **advisory only** — it warned and never failed. That is exactly how drifts like #1459 and #1460 slipped through. ## How Convert the silent-catch scan into an enforcing **ratchet**, following the existing `check-env-coverage.ts` allowlist precedent: - Grandfather the current backlog (169 silent catches across 84 files) into a committed baseline, `script/silent-catch-baseline.json`, keyed by file → count. - A **new** silent catch (a file exceeding its baseline count, or a file not in the baseline at all) now **fails CI**. - Removing silent catches **without** lowering the baseline also fails, so the baseline stays honest and can only ratchet down. - `pnpm run check:errors -- --update` regenerates the baseline after an intentional change. - Refactored the script to export its detectors and baseline logic so they're unit-testable; the standalone CLI behavior is unchanged. - Updated `packages/cli/AGENTS.md` to document the ratchet (replacing the old "advisory / SENTRY_STRICT_SILENT_CATCH" note). ## Tests - New `test/script/check-error-patterns.test.ts`: unit tests for `findSilentCatches`, `findContextErrorNewlines`, `findAdHocTryPatterns`, `countByFile`, and `compareToBaseline` (regression + stale-baseline directions), plus a subprocess test asserting the current tree passes against the committed baseline. - `pnpm run check:errors` passes; `vitest run test/script/check-error-patterns.test.ts` → 17 passing; biome + tsc clean on the changed files. Closes #1470 <!-- Plan: check:errors already existed and ran in CI (.github/workflows/ci.yml) but silent-catch was advisory (169-item backlog), so new drift was invisible to CI. Fix = ratchet baseline (same pattern as check-env-coverage.ts INTERNAL_ENV_VARS allowlist). Refactored check-error-patterns.ts to export pure detectors (findSilentCatches/findContextErrorNewlines/findAdHocTryPatterns) + scanFiles/countByFile/compareToBaseline; main() gated behind direct-invocation check so tests can import. Baseline committed at script/silent-catch-baseline.json (169 catches / 84 files). --update flag regenerates it. New silent catch OR stale (shrunk) baseline both fail CI. CI check count: ci.yml runs check:errors in the same job as check:deps/check:patches/check:stale-refs. --> Co-authored-by: jared-outpost[bot] <jared-outpost[bot]@users.noreply.github.com>
5 tasks
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
2 participants
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.
Root Cause
promptAndStartTrialinsrc/lib/seer-trial.tshad two catch blocks that silently discarded error objects:catch { return false; }— silently swallowed errors fromgetProductTrials()API callcatch { log.warn(...) }— showed user-facing warning but discarded the actual error objectPer AGENTS.md, every catch block must surface the error for diagnostic visibility. When the API call fails due to network issues, auth problems, or unexpected responses, there was no diagnostic output even with
--verbose.Reproduction
sentry issue explainwith a Seer-eligible error and network issues--verbose, no diagnostic information about the failure is loggedFix
log.debug()calls to both catch blocks so the underlying error (network failure, auth issue, unexpected response) is visible with--verbosedebugmethod