Skip to content

Commit

Permalink
chore(allsrv): add github.com/jsteenb2/errors module to improve error…
Browse files Browse the repository at this point in the history
… handling

The addition of this module gives us radically improved error handling and
logging. We now have the ability to tie into the std lib errors.Is/As
functionality instead of writing it ourselves. Our `ErrKinds` are now
useful for an entire domain.

We've added a touch more structure to our error handling with the new
module. With this new structure we can improve our logging once again.
The better your error handling is, the better your logging will get.

Here's an example pulled from the service create foo with exists test:

```json
{
  "time": "2024-07-05T22:56:16.976262-05:00",
  "level": "ERROR",
  "msg": "failed to create foo",
  "input_name": "existing-foo",
  "input_note": "new note",
  "took_ms": "0s",
  "err": "foo exists",
  "err_fields": {
    "sqlite_err_code": "constraint failed",
    "sqlite_err_extended_code": "constraint failed",
    "sqlite_system_errno": "errno 0",
    "err_kind": "exists",
    "stack_trace": [
      "github.com/jsteenb2/mess/allsrv/svc.go:97[(*Service).CreateFoo]",
      "github.com/jsteenb2/mess/allsrv/db_sqlite.go:38[(*sqlDB).CreateFoo]",
      "github.com/jsteenb2/mess/allsrv/db_sqlite.go:96[(*sqlDB).exec]"
    ]
  }
}
```

As your system gets more and more complex, your errors are capable of
extending not support additional details. The extensibility is amazing
for a growing codebase.

Refs: [errors module](https://github.com/jsteenb2/errors)
  • Loading branch information
jsteenb2 committed Jul 10, 2024
1 parent ddafbe2 commit baf5cd9
Show file tree
Hide file tree
Showing 13 changed files with 140 additions and 135 deletions.
6 changes: 4 additions & 2 deletions allsrv/allsrvtesting/service_inmem.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ import (
)

func NewInmemSVC(t *testing.T, opts SVCTestOpts) allsrv.SVC {
db := new(allsrv.InmemDB)
opts.PrepDB(t, db)
return NewSVC(t, new(allsrv.InmemDB), opts)
}

func NewSVC(t *testing.T, db allsrv.DB, opts SVCTestOpts) allsrv.SVC {
opts.PrepDB(t, db)
var svc allsrv.SVC = allsrv.NewService(db, opts.SVCOpts...)
svc = allsrv.SVCLogging(newTestLogger(t))(svc)
svc = allsrv.ObserveSVC(metrics.Default())(svc)
Expand Down
19 changes: 10 additions & 9 deletions allsrv/allsrvtesting/test_suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"
"time"

"github.com/jsteenb2/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -107,7 +108,7 @@ func testSVCCreate(t *testing.T, initFn SVCInitFn) {
},
want: func(t *testing.T, _ allsrv.Foo, insertErr error) {
require.Error(t, insertErr)
assert.True(t, allsrv.IsExistsErr(insertErr))
assert.True(t, errors.Is(insertErr, allsrv.ErrKindExists), errors.Fields(insertErr))
},
},
{
Expand All @@ -120,7 +121,7 @@ func testSVCCreate(t *testing.T, initFn SVCInitFn) {
},
want: func(t *testing.T, _ allsrv.Foo, insertErr error) {
require.Error(t, insertErr)
assert.True(t, allsrv.IsInvalidErr(insertErr))
assert.True(t, errors.Is(insertErr, allsrv.ErrKindInvalid))
},
},
}
Expand Down Expand Up @@ -202,7 +203,7 @@ func testSVCRead(t *testing.T, initFn SVCInitFn) {
},
want: func(t *testing.T, got allsrv.Foo, readErr error) {
require.Error(t, readErr)
assert.True(t, allsrv.IsInvalidErr(readErr), "got_err="+readErr.Error())
assert.True(t, errors.Is(readErr, allsrv.ErrKindInvalid), "got_err="+readErr.Error())
},
},
{
Expand All @@ -212,7 +213,7 @@ func testSVCRead(t *testing.T, initFn SVCInitFn) {
},
want: func(t *testing.T, got allsrv.Foo, readErr error) {
require.Error(t, readErr)
assert.True(t, allsrv.IsNotFoundErr(readErr))
assert.True(t, errors.Is(readErr, allsrv.ErrKindNotFound))
},
},
}
Expand Down Expand Up @@ -338,7 +339,7 @@ func testSVCUpdate(t *testing.T, initFn SVCInitFn) {
},
want: func(t *testing.T, updatedFoo allsrv.Foo, updErr error) {
require.Error(t, updErr)
assert.True(t, allsrv.IsNotFoundErr(updErr))
assert.True(t, errors.Is(updErr, allsrv.ErrKindNotFound))
},
},
{
Expand All @@ -355,7 +356,7 @@ func testSVCUpdate(t *testing.T, initFn SVCInitFn) {
},
want: func(t *testing.T, updatedFoo allsrv.Foo, updErr error) {
require.Error(t, updErr)
assert.True(t, allsrv.IsExistsErr(updErr))
assert.True(t, errors.Is(updErr, allsrv.ErrKindExists))
},
},
}
Expand Down Expand Up @@ -401,7 +402,7 @@ func testSVCDel(t *testing.T, initFn SVCInitFn) {

_, err := svc.ReadFoo(context.TODO(), "9000")
require.Error(t, err)
assert.True(t, allsrv.IsNotFoundErr(err))
assert.True(t, errors.Is(err, allsrv.ErrKindNotFound), errors.Fields(err))
},
},
{
Expand All @@ -411,7 +412,7 @@ func testSVCDel(t *testing.T, initFn SVCInitFn) {
},
want: func(t *testing.T, svc allsrv.SVC, delErr error) {
require.Error(t, delErr)
assert.True(t, allsrv.IsNotFoundErr(delErr))
assert.True(t, errors.Is(delErr, allsrv.ErrKindNotFound))
},
},
{
Expand All @@ -421,7 +422,7 @@ func testSVCDel(t *testing.T, initFn SVCInitFn) {
},
want: func(t *testing.T, svc allsrv.SVC, delErr error) {
require.Error(t, delErr)
assert.True(t, allsrv.IsInvalidErr(delErr), "got_err="+delErr.Error())
assert.True(t, errors.Is(delErr, allsrv.ErrKindInvalid), "got_err="+delErr.Error())
},
},
}
Expand Down
25 changes: 17 additions & 8 deletions allsrv/client_http.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"io"
"net/http"
"time"

"github.com/jsteenb2/errors"
)

type ClientHTTP struct {
Expand Down Expand Up @@ -130,7 +131,7 @@ func doReq[Attr Attrs](c *http.Client, req *http.Request) (Data[Attr], error) {
return *new(Data[Attr]), errs[0]
}
if len(errs) > 1 {
return *new(Data[Attr]), errors.Join(errs...)
return *new(Data[Attr]), errors.Join(errs)
}

if respBody.Data == nil {
Expand Down Expand Up @@ -176,14 +177,22 @@ func toFoo(d Data[ResourceFooAttrs]) Foo {
}

func toErr(respErr RespErr) error {
out := Err{
Type: respErr.Code,
Msg: respErr.Msg,
}
errFn := InternalErr
switch respErr.Code {
case errCodeExist:
errFn = ExistsErr
case errCodeInvalid:
errFn = InvalidErr
case errCodeNotFound:
errFn = NotFoundErr
case errCodeUnAuthed:
errFn = unauthedErr
}
var fields []any
if respErr.Source != nil {
out.Fields = append(out.Fields, "err_source", *respErr.Source)
fields = append(fields, "err_source", *respErr.Source)
}
return out
return errFn(respErr.Msg, fields...)
}

func toTime(in string) time.Time {
Expand Down
31 changes: 21 additions & 10 deletions allsrv/db_sqlite.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ package allsrv
import (
"context"
"database/sql"
"errors"
"sync"
"time"

sq "github.com/Masterminds/squirrel"
"github.com/jmoiron/sqlx"
"github.com/jsteenb2/errors"
"github.com/mattn/go-sqlite3"
)

Expand All @@ -34,7 +34,7 @@ func (s *sqlDB) CreateFoo(ctx context.Context, f Foo) error {
Values(f.ID, f.Name, f.Note, f.CreatedAt, f.UpdatedAt)

_, err := s.exec(ctx, sb)
return err
return errors.Wrap(err)
}

func (s *sqlDB) ReadFoo(ctx context.Context, id string) (Foo, error) {
Expand All @@ -49,7 +49,7 @@ func (s *sqlDB) ReadFoo(ctx context.Context, id string) (Foo, error) {
if errors.Is(err, sql.ErrNoRows) {
return Foo{}, NotFoundErr("foo not found for id: " + id)
}
return Foo{}, err
return Foo{}, errors.Wrap(err, errSQLiteFields(err))
}

out := Foo{
Expand All @@ -70,19 +70,19 @@ func (s *sqlDB) UpdateFoo(ctx context.Context, f Foo) error {
Set("note", f.Note).
Set("updated_at", f.UpdatedAt).
Where(sq.Eq{"id": f.ID})

return s.update(ctx, sb)
return errors.Wrap(s.update(ctx, sb))
}

func (s *sqlDB) DelFoo(ctx context.Context, id string) error {
err := s.update(ctx, s.sq.Delete("foos").Where(sq.Eq{"id": id}))
return err
return errors.Wrap(err)
}

func (s *sqlDB) exec(ctx context.Context, sqlizer sq.Sqlizer) (sql.Result, error) {
query, args, err := sqlizer.ToSql()
if err != nil {
return nil, err
return nil, errors.Wrap(err)
}

s.mu.Lock()
Expand All @@ -92,16 +92,16 @@ func (s *sqlDB) exec(ctx context.Context, sqlizer sq.Sqlizer) (sql.Result, error
if sqErr := new(sqlite3.Error); errors.As(err, sqErr) {
switch sqErr.Code {
case sqlite3.ErrConstraint:
return nil, ExistsErr("foo exists")
return nil, ExistsErr("foo exists", errSQLiteFields(err))
}
}
return res, err
return res, errors.Wrap(err, errSQLiteFields(err))
}

func (s *sqlDB) update(ctx context.Context, sqlizer sq.Sqlizer) error {
res, err := s.exec(ctx, sqlizer)
if err != nil {
return err
return errors.Wrap(err, errSQLiteFields(err))
}

n, err := res.RowsAffected()
Expand All @@ -119,3 +119,14 @@ type entFoo struct {
CreatedAt time.Time `db:"created_at"`
UpdatedAt time.Time `db:"updated_at"`
}

func errSQLiteFields(err error) []errors.KV {
if sqErr := new(sqlite3.Error); errors.As(err, sqErr) {
return errors.KVs(
"sqlite_err_code", sqErr.Code,
"sqlite_err_extended_code", sqErr.ExtendedCode,
"sqlite_system_errno", sqErr.SystemErrno,
)
}
return nil
}
14 changes: 8 additions & 6 deletions allsrv/db_sqlite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,16 @@ import (
)

func TestSQLite(t *testing.T) {
testDB(t, func(t *testing.T) allsrv.DB {
db := newSQLiteInmem(t)
t.Cleanup(func() {
assert.NoError(t, db.Close())
})
testDB(t, newSQLiteDB)
}

return allsrv.NewSQLiteDB(db)
func newSQLiteDB(t *testing.T) allsrv.DB {
t.Helper()
db := newSQLiteInmem(t)
t.Cleanup(func() {
assert.NoError(t, db.Close())
})
return allsrv.NewSQLiteDB(db)
}

func newSQLiteInmem(t *testing.T) *sqlx.DB {
Expand Down
10 changes: 5 additions & 5 deletions allsrv/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ package allsrv_test

import (
"context"
"errors"
"sync"
"testing"
"time"


"github.com/jsteenb2/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -167,7 +167,7 @@ func testDBCreateFoo(t *testing.T, initFn dbInitFn) {
},
want: func(t *testing.T, db allsrv.DB, insertErr error) {
require.Error(t, insertErr)
assert.True(t, allsrv.IsExistsErr(insertErr))
assert.True(t, errors.Is(insertErr, allsrv.ErrKindExists))
},
},
}
Expand Down Expand Up @@ -406,7 +406,7 @@ func testDBUpdateFoo(t *testing.T, initFn dbInitFn) {
},
want: func(t *testing.T, db allsrv.DB, updateErr error) {
require.Error(t, updateErr)
assert.True(t, allsrv.IsNotFoundErr(updateErr))
assert.True(t, errors.Is(updateErr, allsrv.ErrKindNotFound))
},
},
}
Expand Down Expand Up @@ -497,7 +497,7 @@ func testDBDeleteFoo(t *testing.T, initFn dbInitFn) {
inputs: inputs{id: "1"},
want: func(t *testing.T, db allsrv.DB, delErr error) {
require.Error(t, delErr)
assert.True(t, allsrv.IsNotFoundErr(delErr))
assert.True(t, errors.Is(delErr, allsrv.ErrKindNotFound))
},
},
}
Expand Down
Loading

0 comments on commit baf5cd9

Please sign in to comment.