Skip to content
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

feat: Add backtraces to errors #2827

Merged
merged 1 commit into from Oct 7, 2021
Merged

feat: Add backtraces to errors #2827

merged 1 commit into from Oct 7, 2021

Conversation

epage
Copy link
Member

@epage epage commented Oct 6, 2021

This is gated behind the debug feature flag so only explicit debugging
cases pay the build time and runtime costs.

The builder API's stack traces are generally not too interesting. Where
this really helps is with clap_derive. We currently panic on
unexpected conditions which at least gives us a backtrace. We'd like to
turn these into errors but to do so would lose those debuggin
backtraces, which is where this comes in.

This is a part of #2255

This is gated behind the `debug` feature flag so only explicit debugging
cases pay the build time and runtime costs.

The builder API's stack traces are generally not too interesting.  Where
this really helps is with `clap_derive`.  We currently panic on
unexpected conditions which at least gives us a backtrace.  We'd like to
turn these into errors but to do so would lose those debuggin
backtraces, which is where this comes in.

This is a part of clap-rs#2255
@epage
Copy link
Member Author

epage commented Oct 7, 2021

Since the raised concerns are only with implementation details and not the high level idea / design / user-facing aspect, I'm going ahead and merging this and resolving any further issues post-merge.

bors r+

@epage epage added C: alias M-unreviewed Meta: Request for post-merge review. and removed C: alias labels Oct 7, 2021
bors bot added a commit that referenced this pull request Oct 7, 2021
2823: fix(derive): Subcommands not working within macro_rules r=epage a=epage



2824: docs(derive): Fix explanation on optional string list argument r=epage a=epage



2827: feat: Add backtraces to errors r=epage a=epage



Co-authored-by: Ed Page <eopage@gmail.com>
@bors
Copy link
Contributor

bors bot commented Oct 7, 2021

This PR was included in a batch that successfully built, but then failed to merge into master. It will not be retried.

Additional information:

{"message":"Required status check \"CI-PR\" is expected.","documentation_url":"https://docs.github.com/articles/about-protected-branches"}

@epage
Copy link
Member Author

epage commented Oct 7, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Oct 7, 2021

Build succeeded:

@bors bors bot merged commit e3b2392 into clap-rs:master Oct 7, 2021
@epage epage deleted the backtrace branch October 7, 2021 20:22
@pksunkara pksunkara removed the M-unreviewed Meta: Request for post-merge review. label Oct 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants