fix(error-reporting): silence ValidationError from user input#1225
fix(error-reporting): silence ValidationError from user input#1225sentry[bot] wants to merge 1 commit into
Conversation
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| if (error instanceof ValidationError) { | ||
| return "validation_error"; | ||
| } |
There was a problem hiding this comment.
Bug: When a ValidationError is silenced, the recorded metric in recordSilencedError omits the field property, losing context about which validation rule failed.
Severity: LOW
Suggested Fix
In recordSilencedError(), add a condition to include the field attribute in the metric when the error is an instance of ValidationError and the field property is present. This should follow the existing pattern for ContextError.resource. For example: if (error instanceof ValidationError && error.field) { attributes.field = error.field; }.
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: src/lib/error-reporting.ts#L89-L91
Potential issue: The pull request silences `ValidationError` instances, causing them to
be processed by the `recordSilencedError` function. However, this function fails to
include the `field` property from the error object when creating the
`cli.error.silenced` metric. This is inconsistent with how analogous errors like
`ContextError` are handled, where the `resource` property is explicitly preserved to
maintain context in metrics. This change results in a loss of observability, as it
becomes impossible to distinguish which specific validation rule failed based on the
silenced error metrics, whereas previously the `field` was used for grouping in Sentry.
Did we get this right? 👍 / 👎 to inform future reviews.
![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 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f48ea79. Configure here.
| // whole class to avoid noise from AI-agent callers passing bad input. | ||
| if (error instanceof ValidationError) { | ||
| return "validation_error"; | ||
| } |
There was a problem hiding this comment.
Over-broad ValidationError silencing
Medium Severity
The new branch treats every ValidationError as user input, but some throws come from API or internal state rather than caller arguments. buildFormatFromUrl raises ValidationError when a server install URL has an unrecognized response_format, so those product/API mismatches would now be silenced instead of captured.
Reviewed by Cursor Bugbot for commit f48ea79. Configure here.
| // whole class to avoid noise from AI-agent callers passing bad input. | ||
| if (error instanceof ValidationError) { | ||
| return "validation_error"; | ||
| } |
There was a problem hiding this comment.
Outdated silence classification test
Medium Severity
classifySilenced now returns validation_error for ValidationError, but the existing classifySilenced suite still expects ValidationError under "does NOT silence". That assertion will fail, and there is no positive coverage for the new silence reason.
Reviewed by Cursor Bugbot for commit f48ea79. Configure here.
| // whole class to avoid noise from AI-agent callers passing bad input. | ||
| if (error instanceof ValidationError) { | ||
| return "validation_error"; | ||
| } |
There was a problem hiding this comment.
Missing validation field metric
Low Severity
Silenced ValidationErrors emit cli.error.silenced without the structured field attribute. recordSilencedError already preserves resource for ContextError and auth_reason for AuthError, so validation volume cannot be broken down by which input failed after capture is dropped.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f48ea79. Configure here.
This PR addresses CLI-1HQ by silencing
ValidationErrorinstances in Sentry telemetry.Previously,
ValidationErrors, such as those thrown when an empty or whitespace-only issue identifier was provided (e.g., by AI agents callingsentry issue view), were not explicitly handled by theclassifySilenced()function. This led to these user input errors being reported to Sentry as if they were CLI bugs.The fix involves:
"validation_error"to theSilenceReasonunion type.classifySilenced()to return"validation_error"when anerror instanceof ValidationErroris encountered.This change ensures that user-generated validation errors are correctly classified as silenced events, reducing noise in Sentry and accurately reflecting that these are not internal CLI issues.
Fixes CLI-1HQ
Comment
@sentry <feedback>on this PR to have Autofix iterate on the changes.