Skip to content

Commit eb2d91c

Browse files
Automaatbartsmykla
andcommitted
docs(madr): add 094 modern error handling strategy
Addresses inconsistency between MADR 073 (stdlib) and codebase (pkg/errors). Recommends migrating to cockroachdb/errors for stack traces + performance. Co-authored-by: Bart Smykla <[email protected]> Signed-off-by: Marcin Skalski <[email protected]>
1 parent 64d373a commit eb2d91c

File tree

1 file changed

+127
-0
lines changed

1 file changed

+127
-0
lines changed
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
# Modern Go Error Handling Strategy
2+
3+
* Status: accepted
4+
* Supersedes: [MADR 073 - Error Wrapping](073-error-wrapping.md)
5+
6+
## Context and Problem Statement
7+
8+
Inconsistent error handling in Kuma:
9+
- [MADR 073](073-error-wrapping.md) recommends `fmt.Errorf` + `%w` (stdlib)
10+
- Codebase uses `github.com/pkg/errors` (30+ usages in `pkg/`)
11+
- `pkg/errors` archived 2024 - no maintenance/security updates
12+
13+
Requirements: stack traces, best performance, `errors.Is/As` support, no timeline pressure.
14+
15+
## Design
16+
17+
### Option 1: Standard Library (`fmt.Errorf` + `%w`)
18+
19+
Current MADR 073 approach.
20+
21+
```go
22+
var ErrNotFound = errors.New("not found")
23+
return fmt.Errorf("resource %w: type=%q", ErrNotFound, rt)
24+
```
25+
26+
**Pros:** No deps, Go team maintained, stdlib compatible
27+
**Cons:** No stack traces, 5x slower with `errors.Is()`
28+
29+
### Option 2: Keep `github.com/pkg/errors`
30+
31+
Status quo.
32+
33+
**Pros:** No migration, stack traces
34+
**Cons:** Archived (security risk), 5x perf degradation, author abandoned
35+
36+
### Option 3: Migrate to `github.com/cockroachdb/errors`
37+
38+
Drop-in replacement, actively maintained.
39+
40+
```go
41+
import "github.com/cockroachdb/errors"
42+
return errors.Wrapf(err, "sync failed: zone=%s", zone)
43+
```
44+
45+
**Pros:** Stack traces, better perf, drop-in replacement, maintained, stdlib compatible
46+
**Cons:** External dep, migration effort
47+
48+
### Option 4: Custom Hybrid
49+
50+
Build custom error types with manual stack capture.
51+
52+
**Pros:** Full control
53+
**Cons:** High effort, maintenance burden, reinventing wheel
54+
55+
## Security implications and review
56+
57+
- `pkg/errors`: No security updates, CVE risk
58+
- `cockroachdb/errors`: Active maintenance, production-hardened
59+
60+
## Reliability implications
61+
62+
- CP handles many DPs, error handling in xDS hot paths
63+
- `pkg/errors` + `errors.Is()` = 5x slowdown
64+
- Stack traces essential for multi-zone debugging
65+
- KDS error propagation benefits from stack context
66+
67+
## Implications for Kong Mesh
68+
69+
Inherits Kuma error handling. Migration synchronizable.
70+
71+
## Decision
72+
73+
**Migrate to `github.com/cockroachdb/errors`**
74+
75+
Rationale: Stack traces + performance + maintained + drop-in replacement + stdlib compatible.
76+
77+
### Migration
78+
79+
Gradual, no timeline:
80+
1. `go get github.com/cockroachdb/errors`
81+
2. Update imports: `github.com/pkg/errors``github.com/cockroachdb/errors`
82+
3. Convert `fmt.Errorf``errors.Wrap` for stack traces
83+
4. Configure golangci-lint to enforce error handling patterns (errorlint, wrapcheck)
84+
85+
### Usage
86+
87+
```go
88+
// Sentinel errors (from MADR 073)
89+
var ErrNotFound = errors.New("not found")
90+
91+
// Construction
92+
func ErrorResourceNotFound(rt, name, mesh string) error {
93+
return errors.Wrapf(ErrNotFound, "resource: type=%q name=%q mesh=%q", rt, name, mesh)
94+
}
95+
96+
// Checking
97+
if errors.Is(err, ErrNotFound) { /* handle */ }
98+
```
99+
100+
## Notes
101+
102+
Alternative: `gitlab.com/tozd/go/errors` (less adoption)
103+
Stack overhead: negligible, only on error creation
104+
105+
## References
106+
107+
### Go Error Handling
108+
109+
- [Best Practices - JetBrains](https://www.jetbrains.com/guide/go/tutorials/handle_errors_in_go/best_practices/)
110+
- [Working with Errors - Go Blog](https://go.dev/blog/go1.13-errors)
111+
- [Error Wrapping - Bitfield](https://bitfieldconsulting.com/posts/wrapping-errors)
112+
113+
### Sentinel vs Custom Types
114+
115+
- [Go Error Handling Techniques](https://arashtaher.wordpress.com/2024/09/05/go-error-handling-techniques-exploring-sentinel-errors-custom-types-and-client-facing-errors/)
116+
- [Sentinel vs Custom - alesr](https://alesr.github.io/posts/go-errors/)
117+
118+
### fmt.Errorf vs pkg/errors
119+
120+
- [Wrapf vs Errorf - Stack Overflow](https://stackoverflow.com/questions/61933650/whats-the-difference-between-errors-wrapf-errors-errorf-and-fmt-errorf)
121+
- [Can stdlib replace pkg/errors?](https://blog.dharnitski.com/2019/09/09/go-errors-are-not-pkg-errors/)
122+
123+
### Performance & Alternatives
124+
125+
- [errors.Is() 500% slowdown](https://www.dolthub.com/blog/2024-05-31-benchmarking-go-error-handling/)
126+
- [CockroachDB errors library](https://dr-knz.net/cockroachdb-errors-everyday.html)
127+
- [cockroachdb/errors docs](https://pkg.go.dev/github.com/cockroachdb/errors)

0 commit comments

Comments
 (0)