Skip to content

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 10 commits into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
- [Error Types](error-type.md)
- [Error Wrapping](error-wrap.md)
- [Error Naming](error-name.md)
- [Handle Errors Once](error-once.md)
- [Handle Type Assertion Failures](type-assert.md)
- [Don't Panic](panic.md)
- [Use go.uber.org/atomic](atomic.md)
Expand Down
112 changes: 112 additions & 0 deletions src/error-once.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
# 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
- [wrapping the error](error-wrap.md) and returning it
- returning the error as-is

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 will likely take a similar action 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
Comment on lines +41 to +42
Copy link
Contributor

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

}
```

</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>
114 changes: 114 additions & 0 deletions style.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
- [Error Types](#error-types)
- [Error Wrapping](#error-wrapping)
- [Error Naming](#error-naming)
- [Handle Errors Once](#handle-errors-once)
- [Handle Type Assertion Failures](#handle-type-assertion-failures)
- [Don't Panic](#dont-panic)
- [Use go.uber.org/atomic](#use-gouberorgatomic)
Expand Down Expand Up @@ -1016,6 +1017,119 @@ func (e *resolveError) Error() string {
}
```

#### 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
- [wrapping the error](#error-wrapping) and returning it
- returning the error as-is

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 will likely take a similar action 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>

### Handle Type Assertion Failures

The single return value form of a [type assertion](https://golang.org/ref/spec#Type_assertions) will panic on an incorrect
Expand Down