From 0e80e1fd3dd89b3e87ffce69419056694b37f510 Mon Sep 17 00:00:00 2001 From: Brian Picciano Date: Mon, 14 Jan 2019 23:55:22 -0500 Subject: [PATCH] mlog: remove ErrKV/ErrWithKV, merr.KV/merr.WithValue are used instead --- README.md | 13 ++++++----- mcrypto/pair.go | 18 ++++++++-------- mcrypto/secret.go | 6 +++--- mcrypto/secret_test.go | 6 +++--- mcrypto/sig.go | 8 +++---- mcrypto/sig_test.go | 4 ++-- mcrypto/uuid.go | 6 +++--- mlog/errctx.go | 42 ------------------------------------ mlog/errctx_test.go | 49 ------------------------------------------ mlog/mlog.go | 6 ++++-- 10 files changed, 34 insertions(+), 124 deletions(-) diff --git a/README.md b/README.md index 91c66f9..6bdc6e3 100644 --- a/README.md +++ b/README.md @@ -4,14 +4,13 @@ This is a collection of packages which I use across many of my personal projects. All packages intended to be used start with an `m`, packages not starting with `m` are for internal use within this set of packages. -Other third-party packages which integrate into these: +## Usage notes -* [merry](github.com/ansel1/merry): used by `mlog` to embed KV logging - information into `error` instances, it should be assumed that all errors - returned from these packages are `merry.Error` instances. In cases where a - package has a specific error it might return and which might be checked for a - function to perform that equality check will be supplied as part of the - package. +* In general, all checking of equality of errors, e.g. `err == io.EOF`, done on + errors returned from the packages in this project should be done using + `merr.Equal`, e.g. `merr.Equal(err, io.EOF)`. The `merr` package is used to + wrap errors and embed further metadata in them, like stack traces and so + forth. ## Styleguide diff --git a/mcrypto/pair.go b/mcrypto/pair.go index 651f0ed..26648b5 100644 --- a/mcrypto/pair.go +++ b/mcrypto/pair.go @@ -13,7 +13,7 @@ import ( "math/big" "time" - "github.com/mediocregopher/mediocre-go-lib/mlog" + "github.com/mediocregopher/mediocre-go-lib/merr" ) var ( @@ -58,7 +58,7 @@ func (pk PublicKey) verify(s Signature, r io.Reader) error { return err } if err := rsa.VerifyPSS(&pk.PublicKey, crypto.SHA256, h.Sum(nil), s.sig, nil); err != nil { - return mlog.ErrWithKV(ErrInvalidSig, s) + return merr.WithValue(ErrInvalidSig, "sig", s, true) } return nil } @@ -88,12 +88,12 @@ func (pk *PublicKey) UnmarshalText(b []byte) error { str := string(b) strEnc, ok := stripPrefix(str, pubKeyV0) if !ok || len(strEnc) <= hex.EncodedLen(8) { - return mlog.ErrWithKV(errMalformedPublicKey, mlog.KV{"pubKeyStr": str}) + return merr.WithValue(errMalformedPublicKey, "pubKeyStr", str, true) } b, err := hex.DecodeString(strEnc) if err != nil { - return mlog.ErrWithKV(err, mlog.KV{"pubKeyStr": str}) + return merr.WithValue(err, "pubKeyStr", str, true) } pk.E = int(binary.BigEndian.Uint64(b)) @@ -184,17 +184,17 @@ func (pk *PrivateKey) UnmarshalText(b []byte) error { str := string(b) strEnc, ok := stripPrefix(str, privKeyV0) if !ok { - return mlog.ErrWithKV(errMalformedPrivateKey, mlog.KV{"privKeyStr": str}) + return merr.Wrap(errMalformedPrivateKey) } b, err := hex.DecodeString(strEnc) if err != nil { - return mlog.ErrWithKV(err, mlog.KV{"privKeyStr": str}) + return merr.Wrap(errMalformedPrivateKey) } e, n := binary.Uvarint(b) if n <= 0 { - return mlog.ErrWithKV(errMalformedPrivateKey, mlog.KV{"privKeyStr": str}) + return merr.Wrap(errMalformedPrivateKey) } pk.PublicKey.E = int(e) b = b[n:] @@ -205,7 +205,7 @@ func (pk *PrivateKey) UnmarshalText(b []byte) error { } l, n := binary.Uvarint(b) if n <= 0 { - err = errMalformedPrivateKey + err = merr.Wrap(errMalformedPrivateKey) } b = b[n:] i := new(big.Int) @@ -221,7 +221,7 @@ func (pk *PrivateKey) UnmarshalText(b []byte) error { } if err != nil { - return mlog.ErrWithKV(err, mlog.KV{"privKeyStr": str}) + return err } return nil } diff --git a/mcrypto/secret.go b/mcrypto/secret.go index 1511a90..156e12f 100644 --- a/mcrypto/secret.go +++ b/mcrypto/secret.go @@ -7,7 +7,7 @@ import ( "io" "time" - "github.com/mediocregopher/mediocre-go-lib/mlog" + "github.com/mediocregopher/mediocre-go-lib/merr" ) // Secret contains a set of bytes which are inteded to remain secret within some @@ -76,9 +76,9 @@ func (s Secret) sign(r io.Reader) (Signature, error) { func (s Secret) verify(sig Signature, r io.Reader) error { sigB, err := s.signRaw(r, uint8(len(sig.sig)), sig.salt, sig.t) if err != nil { - return mlog.ErrWithKV(err, sig) + return merr.WithValue(err, "sig", sig, true) } else if !hmac.Equal(sigB, sig.sig) { - return mlog.ErrWithKV(ErrInvalidSig, sig) + return merr.WithValue(ErrInvalidSig, "sig", sig, true) } return nil } diff --git a/mcrypto/secret_test.go b/mcrypto/secret_test.go index 1880197..9734ea5 100644 --- a/mcrypto/secret_test.go +++ b/mcrypto/secret_test.go @@ -4,7 +4,7 @@ import ( . "testing" "time" - "github.com/ansel1/merry" + "github.com/mediocregopher/mediocre-go-lib/merr" "github.com/mediocregopher/mediocre-go-lib/mrand" "github.com/stretchr/testify/assert" ) @@ -43,9 +43,9 @@ func TestSecretSignVerify(t *T) { assert.NotEqual(t, prevSig.String(), thisSigStr) assert.NotEqual(t, prevWeakSig.String(), thisWeakSigStr) err := VerifyString(secret, prevSig, thisStr) - assert.True(t, merry.Is(err, ErrInvalidSig)) + assert.True(t, merr.Equal(err, ErrInvalidSig)) err = VerifyString(secret, prevWeakSig, thisStr) - assert.True(t, merry.Is(err, ErrInvalidSig)) + assert.True(t, merr.Equal(err, ErrInvalidSig)) } prevStr = thisStr prevSig = thisSig diff --git a/mcrypto/sig.go b/mcrypto/sig.go index 42259ed..553ab96 100644 --- a/mcrypto/sig.go +++ b/mcrypto/sig.go @@ -9,7 +9,7 @@ import ( "io" "time" - "github.com/mediocregopher/mediocre-go-lib/mlog" + "github.com/mediocregopher/mediocre-go-lib/merr" ) var ( @@ -66,12 +66,12 @@ func (s *Signature) UnmarshalText(b []byte) error { str := string(b) strEnc, ok := stripPrefix(str, sigV0) if !ok || len(strEnc) < hex.EncodedLen(10) { - return mlog.ErrWithKV(errMalformedSig, mlog.KV{"sigStr": str}) + return merr.WithValue(errMalformedSig, "sigStr", str, true) } b, err := hex.DecodeString(strEnc) if err != nil { - return mlog.ErrWithKV(err, mlog.KV{"sigStr": str}) + return merr.WithValue(err, "sigStr", str, true) } unixNano, b := int64(binary.BigEndian.Uint64(b[:8])), b[8:] @@ -81,7 +81,7 @@ func (s *Signature) UnmarshalText(b []byte) error { if err != nil { return nil } else if len(b) < 1+int(b[0]) { - err = mlog.ErrWithKV(errMalformedSig, mlog.KV{"sigStr": str}) + err = merr.WithValue(errMalformedSig, "sigStr", str, true) return nil } out := b[1 : 1+b[0]] diff --git a/mcrypto/sig_test.go b/mcrypto/sig_test.go index dc60ffe..7a49c5d 100644 --- a/mcrypto/sig_test.go +++ b/mcrypto/sig_test.go @@ -4,7 +4,7 @@ import ( . "testing" "time" - "github.com/ansel1/merry" + "github.com/mediocregopher/mediocre-go-lib/merr" "github.com/mediocregopher/mediocre-go-lib/mrand" "github.com/stretchr/testify/assert" ) @@ -36,7 +36,7 @@ func TestSignerVerifier(t *T) { if prevStr != "" { assert.NotEqual(t, prevSig.String(), thisSigStr) err := VerifyString(secret, prevSig, thisStr) - assert.True(t, merry.Is(err, ErrInvalidSig)) + assert.True(t, merr.Equal(err, ErrInvalidSig)) } prevStr = thisStr prevSig = thisSig diff --git a/mcrypto/uuid.go b/mcrypto/uuid.go index 2773e66..f898899 100644 --- a/mcrypto/uuid.go +++ b/mcrypto/uuid.go @@ -9,7 +9,7 @@ import ( "errors" "time" - "github.com/mediocregopher/mediocre-go-lib/mlog" + "github.com/mediocregopher/mediocre-go-lib/merr" ) var errMalformedUUID = errors.New("malformed UUID string") @@ -68,11 +68,11 @@ func (u *UUID) UnmarshalText(b []byte) error { str := string(b) strEnc, ok := stripPrefix(str, uuidV0) if !ok || len(strEnc) != hex.EncodedLen(16) { - return mlog.ErrWithKV(errMalformedUUID, mlog.KV{"uuidStr": str}) + return merr.WithValue(errMalformedUUID, "uuidStr", str, true) } b, err := hex.DecodeString(strEnc) if err != nil { - return mlog.ErrWithKV(err, mlog.KV{"uuidStr": str}) + return merr.WithValue(err, "uuidStr", str, true) } u.b = b return nil diff --git a/mlog/errctx.go b/mlog/errctx.go index da65c52..db256b4 100644 --- a/mlog/errctx.go +++ b/mlog/errctx.go @@ -2,52 +2,10 @@ package mlog import ( "context" - "fmt" - "path/filepath" - - "github.com/ansel1/merry" ) type kvKey int -// ErrWithKV embeds the merging of a set of KVs into an error, returning a new -// error instance. If the error already has a KV embedded in it then the -// returned error will have the merging of them all, with the given KVs taking -// precedence. -func ErrWithKV(err error, kvs ...KVer) merry.Error { - if err == nil { - return nil - } - merr := merry.WrapSkipping(err, 1) - var kv KV - if exKV := merry.Value(merr, kvKey(0)); exKV != nil { - kv = mergeInto(exKV.(KV), kvs...) - } else { - kv = Merge(kvs...).KV() - } - return merr.WithValue(kvKey(0), kv) -} - -// ErrKV returns a KV which includes the KV embedded in the error by ErrWithKV, -// if any. The KV will also include an "err" field containing the output of -// err.Error(), and an "errSrc" field if the given error is a merry error which -// contains an embedded stacktrace. -func ErrKV(err error) KVer { - var kv KV - if kvi := merry.Value(err, kvKey(0)); kvi != nil { - kv = kvi.(KV) - } else { - kv = KV{} - } - kv["err"] = err.Error() - if fileFull, line := merry.Location(err); fileFull != "" { - file, dir := filepath.Base(fileFull), filepath.Dir(fileFull) - dir = filepath.Base(dir) // only want the first dirname, ie the pkg name - kv["errSrc"] = fmt.Sprintf("%s/%s:%d", dir, file, line) - } - return kv -} - // CtxWithKV embeds a KV into a Context, returning a new Context instance. If // the Context already has a KV embedded in it then the returned error will have // the merging of the two, with the given KVs taking precedence. diff --git a/mlog/errctx_test.go b/mlog/errctx_test.go index 832121b..670a7c6 100644 --- a/mlog/errctx_test.go +++ b/mlog/errctx_test.go @@ -2,60 +2,11 @@ package mlog import ( "context" - "strings" . "testing" - "github.com/ansel1/merry" "github.com/mediocregopher/mediocre-go-lib/mtest/massert" ) -func TestErrKV(t *T) { - assertErrKV := func(err error, exp KV) massert.Assertion { - got := KV(ErrKV(err).KV()) - errSrc := got["errSrc"] - delete(got, "errSrc") - return massert.All( - massert.Not(massert.Nil(errSrc)), - massert.Equal(true, strings.HasPrefix(errSrc.(string), "mlog/errctx_test.go")), - massert.Equal(exp, got), - ) - } - - err := merry.New("foo") - massert.Fatal(t, assertErrKV(err, KV{"err": err.Error()})) - - kv := KV{"a": "a"} - err2 := ErrWithKV(err, kv) - massert.Fatal(t, massert.All( - assertErrKV(err, KV{"err": err.Error()}), - assertErrKV(err2, KV{"err": err.Error(), "a": "a"}), - )) - - // changing the kv now shouldn't do anything - kv["a"] = "b" - massert.Fatal(t, massert.All( - assertErrKV(err, KV{"err": err.Error()}), - assertErrKV(err2, KV{"err": err.Error(), "a": "a"}), - )) - - // a new ErrWithKV shouldn't affect the previous one - err3 := ErrWithKV(err2, KV{"b": "b"}) - massert.Fatal(t, massert.All( - assertErrKV(err, KV{"err": err.Error()}), - assertErrKV(err2, KV{"err": err2.Error(), "a": "a"}), - assertErrKV(err3, KV{"err": err3.Error(), "a": "a", "b": "b"}), - )) - - // make sure precedence works - err4 := ErrWithKV(err3, KV{"b": "bb"}) - massert.Fatal(t, massert.All( - assertErrKV(err, KV{"err": err.Error()}), - assertErrKV(err2, KV{"err": err2.Error(), "a": "a"}), - assertErrKV(err3, KV{"err": err3.Error(), "a": "a", "b": "b"}), - assertErrKV(err4, KV{"err": err4.Error(), "a": "a", "b": "bb"}), - )) -} - func TestCtxKV(t *T) { ctx := context.Background() massert.Fatal(t, massert.Equal(KV{}, CtxKV(ctx))) diff --git a/mlog/mlog.go b/mlog/mlog.go index 6cc5448..f756311 100644 --- a/mlog/mlog.go +++ b/mlog/mlog.go @@ -10,7 +10,7 @@ // Examples: // // Info("Something important has occurred") -// Error("Could not open file", llog.KV{"filename": filename}, llog.ErrKV(err)) +// Error("Could not open file", llog.KV{"filename": filename}, merr.KV(err)) // package mlog @@ -22,6 +22,8 @@ import ( "sort" "strconv" "sync" + + "github.com/mediocregopher/mediocre-go-lib/merr" ) // Truncate is a helper function to truncate a string to a given size. It will @@ -339,7 +341,7 @@ func (l *Logger) Log(msg Message) { } if err := l.h(msg); err != nil { - go l.Error("Logger.Handler returned error", ErrKV(err)) + go l.Error("Logger.Handler returned error", merr.KV(err)) return }