-
-
Notifications
You must be signed in to change notification settings - Fork 1
feat(feedback): add command to submit CLI feedback #150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Users can now send feedback about the CLI with: sentry feedback <message> The message is sent via Sentry.captureFeedback() to the CLI's own Sentry project for collection. Closes #119
| Sentry.captureFeedback({ message }); | ||
|
|
||
| // Flush to ensure feedback is sent before process exits | ||
| await Sentry.flush(3000); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The Sentry.captureFeedback() call is not awaited, which can lead to unhandled promise rejections and cause user feedback to be silently dropped while incorrectly showing a success message.
Severity: MEDIUM
Suggested Fix
Wrap the Sentry.captureFeedback() call in a try...catch block and await the returned promise. Display the success message only when the promise resolves successfully. In the catch block, log the error and inform the user that the feedback submission failed.
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/feedback.ts#L45-L49
Potential issue: The `Sentry.captureFeedback()` function returns a promise that can
reject if sending the feedback fails due to network errors, ad blockers, or rate
limiting. The current implementation does not `await` this promise or handle potential
rejections. This will cause an unhandled promise rejection if the API call fails.
Additionally, the code always displays a success message to the user, even if the
feedback was not successfully submitted, providing misleading information.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| // Flush to ensure feedback is sent before process exits | ||
| await Sentry.flush(3000); | ||
|
|
||
| stdout.write("Feedback submitted. Thank you!\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Success message shown even when feedback submission fails
Low Severity
The Sentry.flush(3000) return value is ignored. This method returns false when the flush times out or fails (e.g., network issues), but the success message "Feedback submitted. Thank you!" is displayed unconditionally. Since the CLI exits immediately after, undelivered feedback is lost, leaving users with false confirmation their feedback was received.
| stderr.write("Please provide a feedback message.\n"); | ||
| stderr.write("Usage: sentry feedback <message>\n"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing message returns success exit code instead of failure
Low Severity
When no feedback message is provided, the command writes an error to stderr but returns normally, resulting in exit code 0 (success). The codebase pattern uses CliError subclasses (like ValidationError) for error conditions to set non-zero exit codes. Scripts or CI pipelines checking exit codes won't detect this failure.
| // Flush to ensure feedback is sent before process exits | ||
| await Sentry.flush(3000); | ||
|
|
||
| stdout.write("Feedback submitted. Thank you!\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feedback shows success when telemetry is disabled
Low Severity
When SENTRY_CLI_NO_TELEMETRY=1 is set, Sentry is initialized with enabled: false, making Sentry.captureFeedback() a no-op that silently discards the message. The command still prints "Feedback submitted. Thank you!" without checking if Sentry is actually enabled, giving users false confirmation that their feedback was sent.


Summary
Adds a
sentry feedbackcommand that lets users send feedback about the CLI. All text afterfeedbackis joined and sent viaSentry.captureFeedback()to the CLI's Sentry project.Usage
Test Plan
Closes #119