Handle errors once (#179)

Adds new guidance on how handling errors once
rather than double handling, e.g. log-and-return.

Resolves #65

CHANGELOG.md

@@ -1,3 +1,7 @@
+# 2023-04-13
+
+- Errors: Add guidance on handling errors only once.
+
 # 2023-03-03
 
 - Receivers and Interfaces: Clarify subtlety with pointer receivers and values.

src/SUMMARY.md

@@ -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)

src/error-once.md

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

style.md

@@ -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)
@@ -1016,6 +1017,118 @@ 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
+- returning the error, either [wrapped](#error-wrapping) 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>
+
 ### 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