Skip to content

Commit

Permalink
chore(allsrv): add structured errors
Browse files Browse the repository at this point in the history
Structured errors are so incredibly helpful for service owners. With
structured errors, you're able to enrich the context of your failures
without having to add DEBUG log after DEBUG log, to provide that context.

This marks the end of the "training wheels" part of the workshop. Now that
we have a somewhat solid foundation to build on, we can start to make
informed decisions that help us survive the only constant in software
engineering... changing business requirements >_<!

We still have a number of concerns that we can't address within the
the existing API deisgn... but now we have the tools to inform
our server evolution. GEDIUP!
  • Loading branch information
jsteenb2 committed Jul 5, 2024
1 parent 5844301 commit 98c8963
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 11 deletions.
9 changes: 4 additions & 5 deletions allsrv/db_inmem.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package allsrv

import (
"context"
"errors"
"sync"
)

Expand All @@ -18,7 +17,7 @@ func (db *InmemDB) CreateFoo(_ context.Context, f Foo) error {

for _, existing := range db.m {
if f.Name == existing.Name {
return errors.New("foo " + f.Name + " exists") // 8)
return ExistsErr("foo "+f.Name+" exists", "name", f.Name, "existing_foo_id", existing.ID) // 8)
}
}

Expand All @@ -36,7 +35,7 @@ func (db *InmemDB) ReadFoo(_ context.Context, id string) (Foo, error) {
return f, nil
}
}
return Foo{}, errors.New("foo not found for id: " + id) // 8)
return Foo{}, NotFoundErr("foo not found for id: "+id, "id", id) // 8)
}

func (db *InmemDB) UpdateFoo(_ context.Context, f Foo) error {
Expand All @@ -49,7 +48,7 @@ func (db *InmemDB) UpdateFoo(_ context.Context, f Foo) error {
return nil
}
}
return errors.New("foo not found for id: " + f.ID) // 8)
return NotFoundErr("foo not found for id: "+f.ID, "id", f.ID) // 8)
}

func (db *InmemDB) DelFoo(_ context.Context, id string) error {
Expand All @@ -62,5 +61,5 @@ func (db *InmemDB) DelFoo(_ context.Context, id string) error {
return nil // 13)
}
}
return errors.New("foo not found for id: " + id) // 8)
return NotFoundErr("foo not found for id: "+id, "id", id) // 8)
}
10 changes: 5 additions & 5 deletions allsrv/db_inmem_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestInmemDB(t *testing.T) {
// this is pretty gross, we're matching against a raw error/text value
// any change in the error message means we have to update tests too
wantErr := errors.New("foo collision exists")
assert.Equal(t, wantErr, err)
assert.Equal(t, wantErr.Error(), err.Error())
})
})

Expand Down Expand Up @@ -131,7 +131,7 @@ func TestInmemDB(t *testing.T) {
// this is pretty gross, we're matching against a raw error/text value
// any change in the error message means we have to update tests too
want := errors.New("foo not found for id: 1")
assert.Equal(t, want, err)
assert.Equal(t, want.Error(), err.Error())
})
})

Expand Down Expand Up @@ -201,7 +201,7 @@ func TestInmemDB(t *testing.T) {
// this is pretty gross, we're matching against a raw error/text value
// any change in the error message means we have to update tests too
want := errors.New("foo not found for id: 1")
assert.Equal(t, want, err)
assert.Equal(t, want.Error(), err.Error())
})
})

Expand All @@ -224,7 +224,7 @@ func TestInmemDB(t *testing.T) {
// this is pretty gross, we're matching against a raw error/text value
// any change in the error message means we have to update tests too
want := errors.New("foo not found for id: 1")
assert.Equal(t, want, err)
assert.Equal(t, want.Error(), err.Error())
})

t.Run("with concurrent valid foo creates should pass", func(t *testing.T) {
Expand Down Expand Up @@ -267,7 +267,7 @@ func TestInmemDB(t *testing.T) {
// this is pretty gross, we're matching against a raw error/text value
// any change in the error message means we have to update tests too
want := errors.New("foo not found for id: 1")
assert.Equal(t, want, err)
assert.Equal(t, want.Error(), err.Error())
})
})
}
38 changes: 38 additions & 0 deletions allsrv/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package allsrv

const (
errTypeExists = "exists"
errTypeNotFound = "not found"
)

// Err provides a lightly structured error that we can attach behavior. Additionally,
// the use of fields makes it possible for us to enrich our logging infra without
// blowing up the message cardinality.
type Err struct {
Type string
Msg string
Fields []any
}

// Error returns the error message.
func (e Err) Error() string {
return e.Msg
}

// ExistsErr creates an exists error.
func ExistsErr(msg string, fields ...any) error {
return Err{
Type: errTypeExists,
Msg: msg,
Fields: fields,
}
}

// NotFoundErr creates a not found error.
func NotFoundErr(msg string, fields ...any) error {
return Err{
Type: errTypeNotFound,
Msg: msg,
Fields: fields,
}
}
2 changes: 1 addition & 1 deletion allsrv/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
7) Server only works with HTTP
a) what happens when we want to support grpc? thrift? other protocol?
b) this setup often leads to copy pasta/weak abstractions that tend to leak
8) Errors are opaque and limited
8) Errors are opaque and limited
9) API is very bare bones
a) there is nothing actionable, so how does the consumer know to handle the error?
b) if the APIs evolve, how does the consumer distinguish between old and new?
Expand Down

0 comments on commit 98c8963

Please sign in to comment.