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

Remove unwraps via '?' with anyhow crate for example-oauth #2069

Merged
merged 5 commits into from
Jul 4, 2023
Merged

Remove unwraps via '?' with anyhow crate for example-oauth #2069

merged 5 commits into from
Jul 4, 2023

Conversation

rsdlt
Copy link
Contributor

@rsdlt rsdlt commented Jul 2, 2023

Refs: #2039

Motivation

Substitutes unwrap calls with proper error handling via the anyhow crate for example-oauth.

Solution

Substitute unwraps with ? via anyhow. Using if let or match is very verbose and makes the example lose focus. Using ? keeps the example readable, focused and shows better practices.

Validated solution by compiling and testing via CLIENT_ID=REPLACE_ME CLIENT_SECRET=REPLACE_ME cargo run -p example-oauth

Copy link
Member

@davidpdrsn davidpdrsn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this!

In general I think we should also use .context("...")? to make it clear where the error is coming from. Otherwise it can be hard to debug later.

examples/oauth/src/main.rs Outdated Show resolved Hide resolved
examples/oauth/src/main.rs Outdated Show resolved Hide resolved
@rsdlt rsdlt requested a review from davidpdrsn July 2, 2023 22:20
examples/oauth/src/main.rs Outdated Show resolved Hide resolved
examples/oauth/src/main.rs Outdated Show resolved Hide resolved
examples/oauth/src/main.rs Outdated Show resolved Hide resolved
examples/oauth/src/main.rs Outdated Show resolved Hide resolved
@rsdlt rsdlt requested a review from davidpdrsn July 3, 2023 17:36
Copy link
Member

@davidpdrsn davidpdrsn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@davidpdrsn davidpdrsn enabled auto-merge (squash) July 4, 2023 07:33
auto-merge was automatically disabled July 4, 2023 16:05

Head branch was pushed to by a user without write access

@davidpdrsn davidpdrsn enabled auto-merge (squash) July 4, 2023 19:42
@davidpdrsn davidpdrsn merged commit 8cb11e7 into tokio-rs:main Jul 4, 2023
17 checks passed
@rsdlt rsdlt deleted the clean-oauth-anyhow branch July 5, 2023 22:45
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.

2 participants