From ae369798046a10c90c00c22736551c747a933f80 Mon Sep 17 00:00:00 2001 From: Kaloyan Raev Date: Wed, 26 Jun 2019 10:38:07 +0300 Subject: [PATCH] Take advantage of Is and IsFunc from zeebo/errs (#2310) --- go.mod | 2 +- go.sum | 4 +++- internal/dbutil/pgutil/db.go | 3 +-- internal/dbutil/sqliteutil/db.go | 3 +-- internal/errs2/ignore.go | 19 +----------------- internal/errs2/rpc.go | 17 ++++++++++++++++ pkg/audit/verifier.go | 34 +++++++++----------------------- pkg/audit/verifier_test.go | 22 ++++++--------------- 8 files changed, 39 insertions(+), 65 deletions(-) create mode 100644 internal/errs2/rpc.go diff --git a/go.mod b/go.mod index e2cdc3e0d..f891f3c4b 100644 --- a/go.mod +++ b/go.mod @@ -103,7 +103,7 @@ require ( github.com/vivint/infectious v0.0.0-20190108171102-2455b059135b github.com/yuin/gopher-lua v0.0.0-20180918061612-799fa34954fb // indirect github.com/zeebo/admission v0.0.0-20180821192747-f24f2a94a40c - github.com/zeebo/errs v1.2.0 + github.com/zeebo/errs v1.2.1-0.20190617123220-06a113fed680 github.com/zeebo/float16 v0.1.0 // indirect github.com/zeebo/incenc v0.0.0-20180505221441-0d92902eec54 // indirect github.com/zeebo/structs v1.0.1 diff --git a/go.sum b/go.sum index 34561e409..7a2d5cba0 100644 --- a/go.sum +++ b/go.sum @@ -381,6 +381,8 @@ github.com/zeebo/errs v1.1.1 h1:Cs5Noqj/tj3Ql/hLkD9WdumKlssx/IN2zr7CRGNOKZA= github.com/zeebo/errs v1.1.1/go.mod h1:Yj8dHrUQwls1bF3dr/vcSIu+qf4mI7idnTcHfoACc6I= github.com/zeebo/errs v1.2.0 h1:Tk8UszIOLEjtx6DWnvfmMJe6N8q7vu03Bj95HMWDUkc= github.com/zeebo/errs v1.2.0/go.mod h1:sgbWHsvVuTPHcqJJGQ1WhI5KbWlHYz+2+2C/LSEtCw4= +github.com/zeebo/errs v1.2.1-0.20190617123220-06a113fed680 h1:nLsPSuW22yF+MmUX0sFaTfnMKL83XuHtx8B1YvNtYys= +github.com/zeebo/errs v1.2.1-0.20190617123220-06a113fed680/go.mod h1:sgbWHsvVuTPHcqJJGQ1WhI5KbWlHYz+2+2C/LSEtCw4= github.com/zeebo/float16 v0.1.0 h1:kRqxv5og6z1emEyz5FpW0/BVHe5VfxEAw6b1ljCZlUc= github.com/zeebo/float16 v0.1.0/go.mod h1:fssGvvXu+XS8MH57cKmyrLB/cqioYeYX/2mXCN3a5wo= github.com/zeebo/incenc v0.0.0-20180505221441-0d92902eec54 h1:+cwNE5KJ3pika4HuzmDHkDlK5myo0G9Sv+eO7WWxnUQ= @@ -509,4 +511,4 @@ gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= honnef.co/go/tools v0.0.0-20180728063816-88497007e858/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= -honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= \ No newline at end of file +honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/internal/dbutil/pgutil/db.go b/internal/dbutil/pgutil/db.go index 4828beaa6..b75039957 100644 --- a/internal/dbutil/pgutil/db.go +++ b/internal/dbutil/pgutil/db.go @@ -13,7 +13,6 @@ import ( "storj.io/storj/internal/dbutil" "storj.io/storj/internal/dbutil/dbschema" - "storj.io/storj/internal/errs2" ) // DB is postgres database with schema @@ -126,7 +125,7 @@ func CheckApplicationName(s string) (r string) { // IsConstraintError checks if given error is about constraint violation func IsConstraintError(err error) bool { - return errs2.IsFunc(err, func(err error) bool { + return errs.IsFunc(err, func(err error) bool { if e, ok := err.(*pq.Error); ok { if e.Code.Class() == "23" { return true diff --git a/internal/dbutil/sqliteutil/db.go b/internal/dbutil/sqliteutil/db.go index 660115fee..44fa01287 100644 --- a/internal/dbutil/sqliteutil/db.go +++ b/internal/dbutil/sqliteutil/db.go @@ -11,7 +11,6 @@ import ( "github.com/zeebo/errs" "storj.io/storj/internal/dbutil/dbschema" - "storj.io/storj/internal/errs2" ) // LoadSchemaFromSQL inserts script into connstr and loads schema. @@ -81,7 +80,7 @@ func QueryData(db dbschema.Queryer, schema *dbschema.Schema) (*dbschema.Data, er // IsConstraintError checks if given error is about constraint violation func IsConstraintError(err error) bool { - return errs2.IsFunc(err, func(err error) bool { + return errs.IsFunc(err, func(err error) bool { if e, ok := err.(sqlite3.Error); ok { if e.Code == sqlite3.ErrConstraint { return true diff --git a/internal/errs2/ignore.go b/internal/errs2/ignore.go index 31d2c11ef..6a4cbd14a 100644 --- a/internal/errs2/ignore.go +++ b/internal/errs2/ignore.go @@ -13,7 +13,7 @@ import ( // IgnoreCanceled returns nil, when the operation was about canceling. func IgnoreCanceled(err error) error { - if IsFunc(err, func(err error) bool { + if errs.IsFunc(err, func(err error) bool { return err == context.Canceled || err == grpc.ErrServerStopped || err == http.ErrServerClosed @@ -23,20 +23,3 @@ func IgnoreCanceled(err error) error { return err } - -// IsFunc checks whether any of the underlying errors matches the func -func IsFunc(err error, is func(err error) bool) bool { - if err == nil { - return is(err) - } - for { - if is(err) { - return true - } - unwrapped := errs.Unwrap(err) - if unwrapped == nil || unwrapped == err { - return false - } - err = unwrapped - } -} diff --git a/internal/errs2/rpc.go b/internal/errs2/rpc.go new file mode 100644 index 000000000..060b65c16 --- /dev/null +++ b/internal/errs2/rpc.go @@ -0,0 +1,17 @@ +// Copyright (C) 2019 Storj Labs, Inc. +// See LICENSE for copying information. + +package errs2 + +import ( + "github.com/zeebo/errs" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// IsRPC checks if err contains an RPC error with the given status code. +func IsRPC(err error, code codes.Code) bool { + return errs.IsFunc(err, func(err error) bool { + return status.Code(err) == code + }) +} diff --git a/pkg/audit/verifier.go b/pkg/audit/verifier.go index f60c1c8a7..63bcbdb90 100644 --- a/pkg/audit/verifier.go +++ b/pkg/audit/verifier.go @@ -13,9 +13,9 @@ import ( "github.com/zeebo/errs" "go.uber.org/zap" "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" monkit "gopkg.in/spacemonkeygo/monkit.v2" + "storj.io/storj/internal/errs2" "storj.io/storj/internal/memory" "storj.io/storj/pkg/auth/signing" "storj.io/storj/pkg/identity" @@ -120,17 +120,13 @@ func (verifier *Verifier) Verify(ctx context.Context, stripe *Stripe, skip map[s continue } if transport.Error.Has(share.Error) { - if errs.IsFunc(share.Error, func(err error) bool { - return err == context.DeadlineExceeded - }) { + if errs.Is(share.Error, context.DeadlineExceeded) { // dial timeout offlineNodes = append(offlineNodes, share.NodeID) verifier.log.Debug("Verify: dial timeout (offline)", zap.Stringer("Node ID", share.NodeID), zap.Error(share.Error)) continue } - if errs.IsFunc(share.Error, func(err error) bool { - return status.Code(err) == codes.Unknown - }) { + if errs2.IsRPC(share.Error, codes.Unknown) { // dial failed -- offline node offlineNodes = append(offlineNodes, share.NodeID) verifier.log.Debug("Verify: dial failed (offline)", zap.Stringer("Node ID", share.NodeID), zap.Error(share.Error)) @@ -141,18 +137,14 @@ func (verifier *Verifier) Verify(ctx context.Context, stripe *Stripe, skip map[s verifier.log.Debug("Verify: unknown transport error (contained)", zap.Stringer("Node ID", share.NodeID), zap.Error(share.Error)) } - if errs.IsFunc(share.Error, func(err error) bool { - return status.Code(err) == codes.NotFound - }) { + if errs2.IsRPC(share.Error, codes.NotFound) { // missing share failedNodes = append(failedNodes, share.NodeID) verifier.log.Debug("Verify: piece not found (audit failed)", zap.Stringer("Node ID", share.NodeID), zap.Error(share.Error)) continue } - if errs.IsFunc(share.Error, func(err error) bool { - return status.Code(err) == codes.DeadlineExceeded - }) { + if errs2.IsRPC(share.Error, codes.DeadlineExceeded) { // dial successful, but download timed out containedNodes[pieceNum] = share.NodeID verifier.log.Debug("Verify: download timeout (contained)", zap.Stringer("Node ID", share.NodeID), zap.Error(share.Error)) @@ -358,17 +350,13 @@ func (verifier *Verifier) Reverify(ctx context.Context, stripe *Stripe) (report // analyze the error from GetShare if err != nil { if transport.Error.Has(err) { - if errs.IsFunc(err, func(err error) bool { - return err == context.DeadlineExceeded - }) { + if errs.Is(err, context.DeadlineExceeded) { // dial timeout ch <- result{nodeID: piece.NodeId, status: offline} verifier.log.Debug("Reverify: dial timeout (offline)", zap.Stringer("Node ID", piece.NodeId), zap.Error(err)) return } - if errs.IsFunc(err, func(err error) bool { - return status.Code(err) == codes.Unknown - }) { + if errs2.IsRPC(err, codes.Unknown) { // dial failed -- offline node verifier.log.Debug("Reverify: dial failed (offline)", zap.Stringer("Node ID", piece.NodeId), zap.Error(err)) ch <- result{nodeID: piece.NodeId, status: offline} @@ -380,18 +368,14 @@ func (verifier *Verifier) Reverify(ctx context.Context, stripe *Stripe) (report return } - if errs.IsFunc(err, func(err error) bool { - return status.Code(err) == codes.NotFound - }) { + if errs2.IsRPC(err, codes.NotFound) { // missing share ch <- result{nodeID: piece.NodeId, status: failed} verifier.log.Debug("Reverify: piece not found (audit failed)", zap.Stringer("Node ID", piece.NodeId), zap.Error(err)) return } - if errs.IsFunc(err, func(err error) bool { - return status.Code(err) == codes.DeadlineExceeded - }) { + if errs2.IsRPC(err, codes.DeadlineExceeded) { // dial successful, but download timed out ch <- result{nodeID: piece.NodeId, status: contained, pendingAudit: pending} verifier.log.Debug("Reverify: download timeout (contained)", zap.Stringer("Node ID", piece.NodeId), zap.Error(err)) diff --git a/pkg/audit/verifier_test.go b/pkg/audit/verifier_test.go index 3681b1d62..2a6e1ec4f 100644 --- a/pkg/audit/verifier_test.go +++ b/pkg/audit/verifier_test.go @@ -14,8 +14,8 @@ import ( "github.com/zeebo/errs" "go.uber.org/zap" "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" + "storj.io/storj/internal/errs2" "storj.io/storj/internal/memory" "storj.io/storj/internal/testcontext" "storj.io/storj/internal/testplanet" @@ -134,12 +134,8 @@ func TestDownloadSharesOfflineNode(t *testing.T) { for _, share := range shares { if share.NodeID == stoppedNodeID { assert.True(t, transport.Error.Has(share.Error), "unexpected error: %+v", share.Error) - assert.False(t, errs.IsFunc(share.Error, func(err error) bool { - return err == context.DeadlineExceeded - }), "unexpected error: %+v", share.Error) - assert.True(t, errs.IsFunc(share.Error, func(err error) bool { - return status.Code(err) == codes.Unknown - }), "unexpected error: %+v", share.Error) + assert.False(t, errs.Is(share.Error, context.DeadlineExceeded), "unexpected error: %+v", share.Error) + assert.True(t, errs2.IsRPC(share.Error, codes.Unknown), "unexpected error: %+v", share.Error) } else { assert.NoError(t, share.Error) } @@ -199,9 +195,7 @@ func TestDownloadSharesMissingPiece(t *testing.T) { require.NoError(t, err) for _, share := range shares { - assert.True(t, errs.IsFunc(share.Error, func(err error) bool { - return status.Code(err) == codes.NotFound - }), "unexpected error: %+v", share.Error) + assert.True(t, errs2.IsRPC(share.Error, codes.NotFound), "unexpected error: %+v", share.Error) } }) } @@ -278,9 +272,7 @@ func TestDownloadSharesDialTimeout(t *testing.T) { for _, share := range shares { assert.True(t, transport.Error.Has(share.Error), "unexpected error: %+v", share.Error) - assert.True(t, errs.IsFunc(share.Error, func(err error) bool { - return err == context.DeadlineExceeded - }), "unexpected error: %+v", share.Error) + assert.True(t, errs.Is(share.Error, context.DeadlineExceeded), "unexpected error: %+v", share.Error) } }) } @@ -358,9 +350,7 @@ func TestDownloadSharesDownloadTimeout(t *testing.T) { require.NoError(t, err) for _, share := range shares { - assert.True(t, errs.IsFunc(share.Error, func(err error) bool { - return status.Code(err) == codes.DeadlineExceeded - }), "unexpected error: %+v", share.Error) + assert.True(t, errs2.IsRPC(share.Error, codes.DeadlineExceeded), "unexpected error: %+v", share.Error) assert.False(t, transport.Error.Has(share.Error), "unexpected error: %+v", share.Error) } })