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.
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
Handled unhandled exception to show bug report message #4565
Handled unhandled exception to show bug report message #4565
Changes from all commits
dba1b68
0588f77
7b54934
014eeb0
e9cd81f
32d13cd
e152124
ff52be6
6389ed0
afb4e70
aed9dea
009d89b
1e5a225
1704fc5
2581d59
d540499
569d52c
c7c6a66
f979349
bcd4466
caedfb6
d651acc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Why did this change from
ex.exit_code
to1
?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.
click.BadOptionUsage
and other click exception caught here come with exitcode=2 by default. In our Telemetry, they were, however, classified as 255 (as we didn't catch any them). Today, we only have 1 and 255 exitcodes in our Telemetry. We should revisit the exitcode meaning someday. But for now, re-classifying them as exitcode 1 seems more correct to me.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.
But why not use whatever actual exit code is thrown, instead of only either 1 or 255?
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.
For now I don't want to introduce new exitcode (e.g. 2) into Telemetry, until we have clearer understandings about how we want to use the exitcode. Today,
click.BadOptionUsage
andclick.UsageError
(maybe a couple other user-fixable exceptions as well) go into Telemetry with exitcode 255, which is certainly incorrect. I just want to fix this first.