fix(error-reporting): silence ValidationError from Sentry#1251
fix(error-reporting): silence ValidationError from Sentry#1251sentry[bot] wants to merge 1 commit into
Conversation
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a8418f9. Configure here.
| "validation_error" | ||
| ); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Broken ValidationError tagging tests
Medium Severity
classifySilenced now returns early for ValidationError, so reportCliError never calls withScope or sets grouping tags. The reportCliError integration tests still use capturedScopeTags with ValidationError and assert cli_error.class / cli_error.kind, which will fail because those tags stay empty.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a8418f9. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| expect(classifySilenced(err)).toBeNull(); | ||
| }); | ||
|
|
||
| test("silences ValidationError (user input mistake)", () => { | ||
| expect(classifySilenced(new ValidationError("bad"))).toBe( | ||
| "validation_error" | ||
| ); |
There was a problem hiding this comment.
Bug: Integration tests for ValidationError will fail because they were not updated to reflect that this error is now silenced in reportCliError, which now returns early before calling Sentry.withScope.
Severity: LOW
Suggested Fix
Update the four integration tests for ValidationError in test/lib/error-reporting.test.ts (starting around line 479) to align with the new behavior. These tests should either be removed or modified to assert that ValidationError is correctly silenced and that Sentry.withScope is not called.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: test/lib/error-reporting.test.ts#L298-L304
Potential issue: The function `reportCliError` was modified to silence `ValidationError`
instances by returning early. However, four integration tests in
`test/lib/error-reporting.test.ts` were not updated to reflect this new behavior. These
tests mock `Sentry.withScope` and expect it to be called to capture error tags. Because
`reportCliError` now returns early for a `ValidationError`, `Sentry.withScope` is never
invoked. Consequently, the expected tags are not set, and assertions within these tests
will fail, breaking the test suite.
Did we get this right? 👍 / 👎 to inform future reviews.
This PR addresses CLI-296 by ensuring
ValidationErrorinstances are silenced and not reported to Sentry.Problem:
ValidationErrors, which indicate invalid user input (e.g., incompatible command-line flags like--followwith an absolute date range), were being reported to Sentry. These are user-facing errors that should be presented to the user as usage messages, not treated as internal CLI bugs.Root Cause:
The
classifySilencedfunction insrc/lib/error-reporting.tslacked a specific case to identify and silenceValidationErrorinstances. As a result, these errors fell through the silencing logic and were captured by Sentry.Solution:
'validation_error'to theSilenceReasonunion type insrc/lib/error-reporting.ts.if (error instanceof ValidationError)check withinclassifySilencedto return'validation_error', thereby silencing these errors.test/lib/error-reporting.test.tsto reflect thatValidationErrors are now correctly silenced.classifySilencedexplaining the rationale for silencingValidationErrors.Fixes CLI-296
Comment
@sentry <feedback>on this PR to have Autofix iterate on the changes.