-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Handle errors once #179
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
Merged
Merged
Handle errors once #179
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
6f4c53e
feat: Handle errors once
abhinav aeb880e
Clarify when errors should be matched
abhinav 9114524
caller => callee
abhinav d269146
clarify handle once guidance
abhinav 0cbb0e6
Use caller, callee
abhinav 9ffd194
make style.md
abhinav bf126d6
return and wrap in one item
abhinav e5a06e4
callers may handle
abhinav bde62c8
make style.md
abhinav 0aca677
CHANGELOG
abhinav File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
# Handle Errors Once | ||
|
||
When a caller receives an error from a callee, | ||
it can handle it in a variety of different ways | ||
depending on what it knows about the error. | ||
|
||
These include, but not are limited to: | ||
|
||
- if the callee contract defines specific errors, | ||
matching the error with `errors.Is` or `errors.As` | ||
and handling the branches differently | ||
- if the error is recoverable, | ||
logging the error and degrading gracefully | ||
- if the error represents a domain-specific failure condition, | ||
returning a well-defined error | ||
- returning the error, either [wrapped](error-wrap.md) or verbatim | ||
|
||
Regardless of how the caller handles the error, | ||
it should typically handle each error only once. | ||
The caller should not, for example, log the error and then return it, | ||
because *its* callers may handle the error as well. | ||
|
||
For example, consider the following cases: | ||
|
||
<table> | ||
<thead><tr><th>Description</th><th>Code</th></tr></thead> | ||
<tbody> | ||
<tr><td> | ||
|
||
**Bad**: Log the error and return it | ||
|
||
Callers further up the stack will likely take a similar action with the error. | ||
Doing so causing a lot of noise in the application logs for little value. | ||
|
||
</td><td> | ||
|
||
```go | ||
u, err := getUser(id) | ||
if err != nil { | ||
// BAD: See description | ||
log.Printf("Could not get user %q: %v", id, err) | ||
return err | ||
} | ||
``` | ||
|
||
</td></tr> | ||
<tr><td> | ||
|
||
**Good**: Wrap the error and return it | ||
|
||
Callers further up the stack will handle the error. | ||
Use of `%w` ensures they can match the error with `errors.Is` or `errors.As` | ||
if relevant. | ||
|
||
</td><td> | ||
|
||
```go | ||
u, err := getUser(id) | ||
if err != nil { | ||
return fmt.Errorf("get user %q: %w", id, err) | ||
} | ||
``` | ||
|
||
</td></tr> | ||
<tr><td> | ||
|
||
**Good**: Log the error and degrade gracefully | ||
|
||
If the operation isn't strictly necessary, | ||
we can provide a degraded but unbroken experience | ||
by recovering from it. | ||
|
||
</td><td> | ||
|
||
```go | ||
if err := emitMetrics(); err != nil { | ||
// Failure to write metrics should not | ||
// break the application. | ||
log.Printf("Could not emit metrics: %v", err) | ||
} | ||
|
||
``` | ||
|
||
</td></tr> | ||
<tr><td> | ||
|
||
**Good**: Match the error and degrade gracefully | ||
|
||
If the callee defines a specific error in its contract, | ||
and the failure is recoverable, | ||
match on that error case and degrade gracefully. | ||
For all other cases, wrap the error and return it. | ||
|
||
Callers further up the stack will handle other errors. | ||
|
||
</td><td> | ||
|
||
```go | ||
tz, err := getUserTimeZone(id) | ||
if err != nil { | ||
if errors.Is(err, ErrUserNotFound) { | ||
// User doesn't exist. Use UTC. | ||
tz = time.UTC | ||
} else { | ||
return fmt.Errorf("get user %q: %w", id, err) | ||
} | ||
} | ||
``` | ||
|
||
</td></tr> | ||
</tbody></table> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
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.
(tangentially related)
I often see this with structured logging when you want id as a separate field, but returning an error forces you to flatten the id into the error string, or you have to add an additional return of what fields to log (making it much more complex)
I've been meaning to solve it for a while, and think it can be solved relatively simply,
uber-go/zap#1271