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 1 commit
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
106 changes: 106 additions & 0 deletions src/error-once.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
# Handle Errors Once

When a function receives an error from a caller,
Copy link
Member

Choose a reason for hiding this comment

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

how does a function receive an error from a caller or am I being stupid here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🤦 oops. yeah, callee.

it can handle it in a variety of different ways.
These include, but not are limited to:

- returning the error
- [wrapping the error](error-wrap.md) and returning that
- logging the error and returning nil
- logging the error and returning a different error
- logging the error and continuing running
- matching the error with `errors.Is` or `errors.As`
and then handling the two error branches differently

In general, a function should handle an error only once.
It should not, for example, log the error and then return it again
because its callers will likely handle the error too.

As demonstrative examples, 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 will likely handle the error as well--likely logging it before doing so
causing a lot of noise in the application logs.

</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 will be able to handle the error,
matching with `errors.Is` or `errors.As` for specific errors.

</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 retrieving the value is not strictly necessary,
we can provide a degraded but unbroken experience
by using a fallback value.

</td><td>

```go
u, err := getUser(id)
if err != nil {
log.Printf("Could not get user %q: %v", id, err)
u = nil
}

```

</td></tr>
<tr><td>

**Good**: Match the error and degrade gracefully

If it's expected for retrieval to fail in specific cases,
match on that error case and degrade gracefully.

For all other cases, wrap the error and return it.
Callers will be able to add their own special handling
if necessary.

</td><td>

```go

u, err := getUser(id)
if err != nil {
if !errors.Is(err, ErrNotFound) {
return fmt.Errorf("get user %q: %w", id, err)
}
log.Printf("User %q does not exist", id)
u = nil
}
```

</td></tr>
</tbody></table>
108 changes: 108 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,113 @@ func (e *resolveError) Error() string {
}
```

#### Handle Errors Once

When a function receives an error from a caller,
it can handle it in a variety of different ways.
These include, but not are limited to:

- returning the error
- [wrapping the error](#error-wrapping) and returning that
- logging the error and returning nil
- logging the error and returning a different error
- logging the error and continuing running
- matching the error with `errors.Is` or `errors.As`
and then handling the two error branches differently

In general, a function should handle an error only once.
It should not, for example, log the error and then return it again
because its callers will likely handle the error too.

As demonstrative examples, 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 will likely handle the error as well--likely logging it before doing so
causing a lot of noise in the application logs.

</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 will be able to handle the error,
matching with `errors.Is` or `errors.As` for specific errors.

</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 retrieving the value is not strictly necessary,
we can provide a degraded but unbroken experience
by using a fallback value.

</td><td>

```go
u, err := getUser(id)
if err != nil {
log.Printf("Could not get user %q: %v", id, err)
u = nil
}

```

</td></tr>
<tr><td>

**Good**: Match the error and degrade gracefully

If it's expected for retrieval to fail in specific cases,
match on that error case and degrade gracefully.

For all other cases, wrap the error and return it.
Callers will be able to add their own special handling
if necessary.

</td><td>

```go

u, err := getUser(id)
if err != nil {
if !errors.Is(err, ErrNotFound) {
return fmt.Errorf("get user %q: %w", id, err)
}
log.Printf("User %q does not exist", id)
u = nil
}
```

</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