fix(span): Use ResolutionError for missing spans#1236
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.
Reviewed by Cursor Bugbot for commit cef347d. Configure here.
| if (results.length === 0) { | ||
| const idList = formatIdList(spanIds); | ||
| throw new ValidationError( | ||
| throw new ResolutionError( |
There was a problem hiding this comment.
Wrong ResolutionError constructor call
High Severity
ResolutionError expects separate resource, headline, and hint arguments, but this call passes a single message string as if it were ValidationError. At runtime that yields a broken message with undefined for the headline and hint, so the missing-span error path the PR aims to fix does not produce a usable resolution error.
Reviewed by Cursor Bugbot for commit cef347d. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| throw new ResolutionError( | ||
| spanIds.length === 1 | ||
| ? `No span found with ID "${spanIds[0]}" in trace ${traceId}.` | ||
| : `No spans found with any of the following IDs in trace ${traceId}:\n${idList}` |
There was a problem hiding this comment.
Bug: The ResolutionError constructor is called with only one argument instead of the required three, which will result in a garbled error message containing the literal string "undefined".
Severity: MEDIUM
Suggested Fix
Refactor the ResolutionError call to provide the three required arguments: resource, headline, and hint. The existing message string should be broken down and passed as separate arguments to the constructor to ensure the error message is formatted correctly.
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/commands/span/view.ts#L416-L419
Potential issue: The `ResolutionError` constructor is being called with a single string
argument, but its signature requires three mandatory arguments: `resource`, `headline`,
and `hint`. Because only one argument is provided, the `headline` and `hint` parameters
will be `undefined`. When the error message is constructed, it will include the literal
string "undefined", resulting in a garbled and unhelpful message for the user, such as
`"No span found... undefined.\n\nTry:\n undefined"`.
Did we get this right? 👍 / 👎 to inform future reviews.
Previously, when a requested span ID could not be found within a given trace, the CLI would throw a
ValidationError.This commit corrects the error type to
ResolutionErrorinsrc/commands/span/view.ts.ValidationErroris intended for invalid user input, whereas a missing resource (like a span not found in a valid trace) is a resolution failure, whichResolutionErroris designed to represent, aligning with CLI error categorization guidelines.Fixes CLI-1Q4
Comment
@sentry <feedback>on this PR to have Autofix iterate on the changes.