From e4e085c5e4ce5bc994513ad877ee78c9b3a5f926 Mon Sep 17 00:00:00 2001 From: Donald Adu-Poku Date: Tue, 15 Dec 2020 21:53:48 +0000 Subject: [PATCH] internal/ticketdb: update error types. This updates the ticketdb error types to leverage go 1.13 errors.Is/As functionality as well as conform to the error infrastructure best practices. --- .../stake/internal/ticketdb/chainio_test.go | 64 +++----- blockchain/stake/internal/ticketdb/error.go | 75 ++++----- .../stake/internal/ticketdb/error_test.go | 143 ++++++++++++++---- 3 files changed, 157 insertions(+), 125 deletions(-) diff --git a/blockchain/stake/internal/ticketdb/chainio_test.go b/blockchain/stake/internal/ticketdb/chainio_test.go index 7096643b00..8dd530a1eb 100644 --- a/blockchain/stake/internal/ticketdb/chainio_test.go +++ b/blockchain/stake/internal/ticketdb/chainio_test.go @@ -107,28 +107,21 @@ func TestDbInfoDeserializeErrors(t *testing.T) { tests := []struct { name string serialized []byte - errCode ErrorCode + err error }{ { name: "short read", serialized: hexToBytes("0000"), - errCode: ErrDatabaseInfoShortRead, + err: ErrDatabaseInfoShortRead, }, } - var ticketDBErr DBError for _, test := range tests { // Ensure the expected error type is returned. _, err := deserializeDatabaseInfo(test.serialized) - if !errors.As(err, &ticketDBErr) { - t.Errorf("couldn't convert deserializeDatabaseInfo error "+ - "to ticket db error (err: %v)", err) - continue - } - if ticketDBErr.GetCode() != test.errCode { + if !errors.Is(err, test.err) { t.Errorf("deserializeDatabaseInfo (%s): expected error type "+ - "does not match - got %v, want %v", test.name, - ticketDBErr.ErrorCode, test.errCode) + "does not match - got %v, want %v", test.name, err, test.err) continue } } @@ -200,28 +193,21 @@ func TestBestChainStateDeserializeErrors(t *testing.T) { tests := []struct { name string serialized []byte - errCode ErrorCode + err error }{ { name: "short read", serialized: hexToBytes("0000"), - errCode: ErrChainStateShortRead, + err: ErrChainStateShortRead, }, } - var ticketDBErr DBError for _, test := range tests { // Ensure the expected error type is returned. _, err := deserializeBestChainState(test.serialized) - if !errors.As(err, &ticketDBErr) { - t.Errorf("couldn't convert deserializeBestChainState error "+ - "to ticket db error (err: %v)", err) - continue - } - if ticketDBErr.GetCode() != test.errCode { + if !errors.Is(err, test.err) { t.Errorf("deserializeBestChainState (%s): expected error type "+ - "does not match - got %v, want %v", test.name, - ticketDBErr.ErrorCode, test.errCode) + "does not match - got %v, want %v", test.name, err, test.err) continue } } @@ -293,33 +279,26 @@ func TestBlockUndoDataDeserializingErrors(t *testing.T) { tests := []struct { name string serialized []byte - errCode ErrorCode + err error }{ { name: "short read", serialized: hexToBytes("00"), - errCode: ErrUndoDataShortRead, + err: ErrUndoDataShortRead, }, { name: "bad size", serialized: hexToBytes("00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"), - errCode: ErrUndoDataCorrupt, + err: ErrUndoDataCorrupt, }, } - var ticketDBErr DBError for _, test := range tests { // Ensure the expected error type is returned. _, err := deserializeBlockUndoData(test.serialized) - if !errors.As(err, &ticketDBErr) { - t.Errorf("couldn't convert deserializeBlockUndoData error "+ - "to ticket db error (err: %v)", err) - continue - } - if ticketDBErr.GetCode() != test.errCode { + if !errors.Is(err, test.err) { t.Errorf("deserializeBlockUndoData (%s): expected error type "+ - "does not match - got %v, want %v", test.name, - ticketDBErr.ErrorCode, test.errCode) + "does not match - got %v, want %v", test.name, err, test.err) continue } } @@ -382,33 +361,26 @@ func TestTicketHashesDeserializingErrors(t *testing.T) { tests := []struct { name string serialized []byte - errCode ErrorCode + err error }{ { name: "short read", serialized: hexToBytes("00"), - errCode: ErrTicketHashesShortRead, + err: ErrTicketHashesShortRead, }, { name: "bad size", serialized: hexToBytes("00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"), - errCode: ErrTicketHashesCorrupt, + err: ErrTicketHashesCorrupt, }, } - var ticketDBErr DBError for _, test := range tests { // Ensure the expected error type is returned. _, err := deserializeTicketHashes(test.serialized) - if !errors.As(err, &ticketDBErr) { - t.Errorf("couldn't convert deserializeTicketHashes error "+ - "to ticket db error (err: %v)", err) - continue - } - if ticketDBErr.GetCode() != test.errCode { + if !errors.Is(err, test.err) { t.Errorf("deserializeTicketHashes (%s): expected error type "+ - "does not match - got %v, want %v", test.name, - ticketDBErr.ErrorCode, test.errCode) + "does not match - got %v, want %v", test.name, err, test.err) continue } } diff --git a/blockchain/stake/internal/ticketdb/error.go b/blockchain/stake/internal/ticketdb/error.go index eb3844b58c..0b8002e711 100644 --- a/blockchain/stake/internal/ticketdb/error.go +++ b/blockchain/stake/internal/ticketdb/error.go @@ -4,79 +4,60 @@ package ticketdb -import ( - "fmt" -) - -// ErrorCode identifies a kind of error. -type ErrorCode int +// ErrorKind identifies a kind of error. It has full support for errors.Is +// and errors.As, so the caller can directly check against an error kind +// when determining the reason for an error. +type ErrorKind string -// These constants are used to identify a specific RuleError. +// These constants are used to identify a specific DBError. const ( // ErrUndoDataShortRead indicates that the given undo serialized data // was took small. - ErrUndoDataShortRead = iota + ErrUndoDataShortRead = ErrorKind("ErrUndoDataShortRead") // ErrUndoDataNoEntries indicates that the data for undoing ticket data // in a serialized entry was corrupt. - ErrUndoDataCorrupt + ErrUndoDataCorrupt = ErrorKind("ErrUndoDataCorrupt") // ErrTicketHashesShortRead indicates that the given ticket hashes // serialized data was took small. - ErrTicketHashesShortRead + ErrTicketHashesShortRead = ErrorKind("ErrTicketHashesShortRead") // ErrTicketHashesCorrupt indicates that the data for ticket hashes // in a serialized entry was corrupt. - ErrTicketHashesCorrupt + ErrTicketHashesCorrupt = ErrorKind("ErrTicketHashesCorrupt") // ErrUninitializedBucket indicates that a database bucket was not // initialized and therefore could not be written to or read from. - ErrUninitializedBucket + ErrUninitializedBucket = ErrorKind("ErrUninitializedBucket") // ErrMissingKey indicates that a key was not found in a bucket. - ErrMissingKey + ErrMissingKey = ErrorKind("ErrMissingKey") // ErrChainStateShortRead indicates that the given chain state data // was too small. - ErrChainStateShortRead + ErrChainStateShortRead = ErrorKind("ErrChainStateShortRead") // ErrDatabaseInfoShortRead indicates that the given database information // was too small. - ErrDatabaseInfoShortRead + ErrDatabaseInfoShortRead = ErrorKind("ErrDatabaseInfoShortRead") // ErrLoadAllTickets indicates that there was an error loading the tickets // from the database, presumably at startup. - ErrLoadAllTickets + ErrLoadAllTickets = ErrorKind("ErrLoadAllTickets") ) -// Map of ErrorCode values back to their constant names for pretty printing. -var errorCodeStrings = map[ErrorCode]string{ - ErrUndoDataShortRead: "ErrUndoDataShortRead", - ErrUndoDataCorrupt: "ErrUndoDataCorrupt", - ErrTicketHashesShortRead: "ErrTicketHashesShortRead", - ErrTicketHashesCorrupt: "ErrTicketHashesCorrupt", - ErrUninitializedBucket: "ErrUninitializedBucket", - ErrMissingKey: "ErrMissingKey", - ErrChainStateShortRead: "ErrChainStateShortRead", - ErrDatabaseInfoShortRead: "ErrDatabaseInfoShortRead", - ErrLoadAllTickets: "ErrLoadAllTickets", -} - -// String returns the ErrorCode as a human-readable name. -func (e ErrorCode) String() string { - if s := errorCodeStrings[e]; s != "" { - return s - } - return fmt.Sprintf("Unknown ErrorCode (%d)", int(e)) +// Error satisfies the error interface and prints human-readable errors. +func (e ErrorKind) Error() string { + return string(e) } -// DBError identifies an error in the stake database for tickets. -// The caller can use type assertions to determine if a failure was -// specifically due to a rule violation and access the ErrorCode field to -// ascertain the specific reason for the rule violation. +// DBError identifies an error related to the stake database for tickets. It +// has full support for errors.Is and errors.As, so the caller can ascertain +// the specific reason for the error by checking the underlying error. type DBError struct { - ErrorCode ErrorCode // Describes the kind of error - Description string // Human readable description of the issue + Description string + Err error } // Error satisfies the error interface and prints human-readable errors. @@ -84,12 +65,12 @@ func (e DBError) Error() string { return e.Description } -// GetCode satisfies the error interface and prints human-readable errors. -func (e DBError) GetCode() ErrorCode { - return e.ErrorCode +// Unwrap returns the underlying wrapped error. +func (e DBError) Unwrap() error { + return e.Err } -// DBError creates an DBError given a set of arguments. -func ticketDBError(c ErrorCode, desc string) DBError { - return DBError{ErrorCode: c, Description: desc} +// ticketDBError creates an DBError given a set of arguments. +func ticketDBError(kind ErrorKind, desc string) DBError { + return DBError{Err: kind, Description: desc} } diff --git a/blockchain/stake/internal/ticketdb/error_test.go b/blockchain/stake/internal/ticketdb/error_test.go index 41f8431cb4..bb6a4cb4e0 100644 --- a/blockchain/stake/internal/ticketdb/error_test.go +++ b/blockchain/stake/internal/ticketdb/error_test.go @@ -3,63 +3,142 @@ // Use of this source code is governed by an ISC // license that can be found in the LICENSE file. -package ticketdb_test +package ticketdb import ( + "errors" + "io" "testing" - - "github.com/decred/dcrd/blockchain/stake/v4/internal/ticketdb" ) -// TestErrorCodeStringer tests the stringized output for the ErrorCode type. -func TestErrorCodeStringer(t *testing.T) { +// TestErrorKindStringer tests the stringized output for the ErrorKind type. +func TestErrorKindStringer(t *testing.T) { tests := []struct { - in ticketdb.ErrorCode + in ErrorKind want string }{ - {ticketdb.ErrUndoDataShortRead, "ErrUndoDataShortRead"}, - {ticketdb.ErrUndoDataCorrupt, "ErrUndoDataCorrupt"}, - {ticketdb.ErrTicketHashesShortRead, "ErrTicketHashesShortRead"}, - {ticketdb.ErrTicketHashesCorrupt, "ErrTicketHashesCorrupt"}, - {ticketdb.ErrUninitializedBucket, "ErrUninitializedBucket"}, - {ticketdb.ErrMissingKey, "ErrMissingKey"}, - {ticketdb.ErrChainStateShortRead, "ErrChainStateShortRead"}, - {ticketdb.ErrDatabaseInfoShortRead, "ErrDatabaseInfoShortRead"}, - {ticketdb.ErrLoadAllTickets, "ErrLoadAllTickets"}, - {0xffff, "Unknown ErrorCode (65535)"}, + {ErrUndoDataShortRead, "ErrUndoDataShortRead"}, + {ErrUndoDataCorrupt, "ErrUndoDataCorrupt"}, + {ErrTicketHashesShortRead, "ErrTicketHashesShortRead"}, + {ErrTicketHashesCorrupt, "ErrTicketHashesCorrupt"}, + {ErrUninitializedBucket, "ErrUninitializedBucket"}, + {ErrMissingKey, "ErrMissingKey"}, + {ErrChainStateShortRead, "ErrChainStateShortRead"}, + {ErrDatabaseInfoShortRead, "ErrDatabaseInfoShortRead"}, + {ErrLoadAllTickets, "ErrLoadAllTickets"}, } t.Logf("Running %d tests", len(tests)) for i, test := range tests { - result := test.in.String() + result := test.in.Error() if result != test.want { - t.Errorf("String #%d\n got: %s want: %s", i, result, - test.want) + t.Errorf("#%d: got: %s want: %s", i, result, test.want) continue } } } -// TestRuleError tests the error output for the RuleError type. -func TestRuleError(t *testing.T) { +// TestDBError tests the error output for the DBError type. +func TestDBError(t *testing.T) { tests := []struct { - in ticketdb.DBError + in DBError want string - }{ - {ticketdb.DBError{Description: "duplicate block"}, - "duplicate block", - }, - {ticketdb.DBError{Description: "human-readable error"}, - "human-readable error", - }, - } + }{{ + DBError{Description: "duplicate block"}, + "duplicate block", + }, { + DBError{Description: "human-readable error"}, + "human-readable error", + }} t.Logf("Running %d tests", len(tests)) for i, test := range tests { result := test.in.Error() if result != test.want { - t.Errorf("Error #%d\n got: %s want: %s", i, result, - test.want) + t.Errorf("#%d: got: %s want: %s", i, result, test.want) + continue + } + } +} + +// TestErrorKindIsAs ensures both ErrorKind and Error can be identified as being +// a specific error kind via errors.Is and unwrapped via errors.As. +func TestErrorKindIsAs(t *testing.T) { + tests := []struct { + name string + err error + target error + wantMatch bool + wantAs ErrorKind + }{{ + name: "ErrUndoDataShortRead == ErrUndoDataShortRead", + err: ErrUndoDataShortRead, + target: ErrUndoDataShortRead, + wantMatch: true, + wantAs: ErrUndoDataShortRead, + }, { + name: "DBError.ErrUndoDataShortRead == ErrUndoDataShortRead", + err: ticketDBError(ErrUndoDataShortRead, ""), + target: ErrUndoDataShortRead, + wantMatch: true, + wantAs: ErrUndoDataShortRead, + }, { + name: "DBError.ErrUndoDataShortRead == DBError.ErrUndoDataShortRead", + err: ticketDBError(ErrUndoDataShortRead, ""), + target: ticketDBError(ErrUndoDataShortRead, ""), + wantMatch: true, + wantAs: ErrUndoDataShortRead, + }, { + name: "ErrUndoDataShortRead != ErrUndoDataCorrupt", + err: ErrUndoDataShortRead, + target: ErrUndoDataCorrupt, + wantMatch: false, + wantAs: ErrUndoDataShortRead, + }, { + name: "DBError.ErrUndoDataShortRead != ErrUndoDataCorrupt", + err: ticketDBError(ErrUndoDataShortRead, ""), + target: ErrUndoDataCorrupt, + wantMatch: false, + wantAs: ErrUndoDataShortRead, + }, { + name: "ErrUndoDataShortRead != DBError.ErrUndoDataCorrupt", + err: ErrUndoDataShortRead, + target: ticketDBError(ErrUndoDataCorrupt, ""), + wantMatch: false, + wantAs: ErrUndoDataShortRead, + }, { + name: "DBError.ErrUndoDataShortRead != DBError.ErrUndoDataCorrupt", + err: ticketDBError(ErrUndoDataShortRead, ""), + target: ticketDBError(ErrUndoDataCorrupt, ""), + wantMatch: false, + wantAs: ErrUndoDataShortRead, + }, { + name: "DBError.ErrUndoDataShortRead != io.EOF", + err: ticketDBError(ErrUndoDataShortRead, ""), + target: io.EOF, + wantMatch: false, + wantAs: ErrUndoDataShortRead, + }} + + for _, test := range tests { + // Ensure the error matches or not depending on the expected result. + result := errors.Is(test.err, test.target) + if result != test.wantMatch { + t.Errorf("%s: incorrect error identification -- got %v, want %v", + test.name, result, test.wantMatch) + continue + } + + // Ensure the underlying error kind can be unwrapped is and is the + // expected kind. + var kind ErrorKind + if !errors.As(test.err, &kind) { + t.Errorf("%s: unable to unwrap to error kind", test.name) + continue + } + if !errors.Is(kind, test.wantAs) { + t.Errorf("%s: unexpected unwrapped error kind -- got %v, want %v", + test.name, kind, test.wantAs) continue } }