From 8aa97edada96def2bef8b89ae77db5fd64bc8f84 Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Mon, 19 Aug 2019 03:58:06 -0500 Subject: [PATCH] gcs: Make error consistent with rest of codebase. This updates the error handling in the gcs package to be consistent with the rest of the code base to provide a proper error type and error codes that can be programmatically detected. This is part of the ongoing process to cleanup and improve the gcs module to the quality level required by consensus code for ultimate inclusion in header commitments. --- gcs/error.go | 69 +++++++++++++++++++++++++++++ gcs/error_test.go | 109 ++++++++++++++++++++++++++++++++++++++++++++++ gcs/gcs.go | 30 +++++-------- gcs/gcs_test.go | 10 ++--- 4 files changed, 194 insertions(+), 24 deletions(-) create mode 100644 gcs/error.go create mode 100644 gcs/error_test.go diff --git a/gcs/error.go b/gcs/error.go new file mode 100644 index 00000000..70e89863 --- /dev/null +++ b/gcs/error.go @@ -0,0 +1,69 @@ +// Copyright (c) 2019 The Decred developers +// Use of this source code is governed by an ISC +// license that can be found in the LICENSE file. + +package gcs + +import ( + "fmt" +) + +// ErrorCode identifies a kind of error. +type ErrorCode int + +// These constants are used to identify a specific RuleError. +const ( + // ErrNTooBig signifies that the filter can't handle N items. + ErrNTooBig ErrorCode = iota + + // ErrPTooBig signifies that the filter can't handle `1/2**P` + // collision probability. + ErrPTooBig + + // ErrMisserialized signifies a filter was misserialized and is missing the + // N and/or P parameters of a serialized filter. + ErrMisserialized + + // numErrorCodes is the maximum error code number used in tests. + numErrorCodes +) + +// Map of ErrorCode values back to their constant names for pretty printing. +var errorCodeStrings = map[ErrorCode]string{ + ErrNTooBig: "ErrNTooBig", + ErrPTooBig: "ErrPTooBig", + ErrMisserialized: "ErrMisserialized", +} + +// 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 identifies a filter-related error. The caller can use type assertions +// to access the ErrorCode field to ascertain the specific reason for the +// failure. +type Error struct { + ErrorCode ErrorCode // Describes the kind of error + Description string // Human readable description of the issue +} + +// Error satisfies the error interface and prints human-readable errors. +func (e Error) Error() string { + return e.Description +} + +// makeError creates an Error given a set of arguments. The error code must +// be one of the error codes provided by this package. +func makeError(c ErrorCode, desc string) Error { + return Error{ErrorCode: c, Description: desc} +} + +// IsError returns whether err is an Error with a matching error code. +func IsErrorCode(err error, c ErrorCode) bool { + e, ok := err.(Error) + return ok && e.ErrorCode == c +} diff --git a/gcs/error_test.go b/gcs/error_test.go new file mode 100644 index 00000000..6e5b63fd --- /dev/null +++ b/gcs/error_test.go @@ -0,0 +1,109 @@ +// Copyright (c) 2019 The Decred developers +// Use of this source code is governed by an ISC +// license that can be found in the LICENSE file. + +package gcs + +import ( + "testing" +) + +// TestErrorCodeStringer tests the stringized output for the ErrorCode type. +func TestErrorCodeStringer(t *testing.T) { + tests := []struct { + in ErrorCode + want string + }{ + {ErrNTooBig, "ErrNTooBig"}, + {ErrPTooBig, "ErrPTooBig"}, + {ErrMisserialized, "ErrMisserialized"}, + {0xffff, "Unknown ErrorCode (65535)"}, + } + + // Detect additional error codes that don't have the stringer added. + if len(tests)-1 != int(numErrorCodes) { + t.Errorf("It appears an error code was added without adding an " + + "associated stringer test") + } + + t.Logf("Running %d tests", len(tests)) + for i, test := range tests { + result := test.in.String() + if result != test.want { + t.Errorf("String #%d\n got: %s want: %s", i, result, test.want) + continue + } + } +} + +// TestError tests the error output for the Error type. +func TestError(t *testing.T) { + tests := []struct { + in Error + want string + }{{ + Error{Description: "duplicate block"}, + "duplicate block", + }, { + Error{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) + continue + } + } +} + +// TestIsErrorCode ensures IsErrorCode works as intended. +func TestIsErrorCode(t *testing.T) { + tests := []struct { + name string + err error + code ErrorCode + want bool + }{{ + name: "ErrNTooBig testing for ErrNTooBig", + err: makeError(ErrNTooBig, ""), + code: ErrNTooBig, + want: true, + }, { + name: "ErrPTooBig testing for ErrPTooBig", + err: makeError(ErrPTooBig, ""), + code: ErrPTooBig, + want: true, + }, { + name: "ErrMisserialized testing for ErrMisserialized", + err: makeError(ErrMisserialized, ""), + code: ErrMisserialized, + want: true, + }, { + name: "ErrNTooBig error testing for ErrPTooBig", + err: makeError(ErrNTooBig, ""), + code: ErrPTooBig, + want: false, + }, { + name: "ErrNTooBig error testing for unknown error code", + err: makeError(ErrNTooBig, ""), + code: 0xffff, + want: false, + }, { + name: "nil error testing for ErrNTooBig", + err: nil, + code: ErrNTooBig, + want: false, + }} + for _, test := range tests { + result := IsErrorCode(test.err, test.code) + if result != test.want { + t.Errorf("%s: unexpected result -- got: %v want: %v", test.name, + result, test.want) + continue + } + } +} diff --git a/gcs/gcs.go b/gcs/gcs.go index c52f293f..d8bc09e4 100644 --- a/gcs/gcs.go +++ b/gcs/gcs.go @@ -1,6 +1,6 @@ // Copyright (c) 2016-2017 The btcsuite developers // Copyright (c) 2016-2017 The Lightning Network Developers -// Copyright (c) 2018 The Decred developers +// Copyright (c) 2018-2019 The Decred developers // Use of this source code is governed by an ISC // license that can be found in the LICENSE file. @@ -8,7 +8,7 @@ package gcs import ( "encoding/binary" - "errors" + "fmt" "math" "sort" "sync" @@ -20,19 +20,6 @@ import ( // Inspired by https://github.com/rasky/gcs -var ( - // ErrNTooBig signifies that the filter can't handle N items. - ErrNTooBig = errors.New("N does not fit in uint32") - - // ErrPTooBig signifies that the filter can't handle `1/2**P` - // collision probability. - ErrPTooBig = errors.New("P is too large") - - // ErrMisserialized signifies a filter was misserialized and is missing the - // N and/or P parameters of a serialized filter. - ErrMisserialized = errors.New("misserialized filter") -) - // KeySize is the size of the byte array required for key material for the // SipHash keyed hash function. const KeySize = 16 @@ -66,10 +53,13 @@ func NewFilter(P uint8, key [KeySize]byte, data [][]byte) (*Filter, error) { // build the filter, and make sure our parameters will fit the hash // function we're using. if len(data) > math.MaxInt32 { - return nil, ErrNTooBig + str := fmt.Sprintf("unable to create filter with %d entries greater "+ + "than max allowed %d", len(data), math.MaxInt32) + return nil, makeError(ErrNTooBig, str) } if P > 32 { - return nil, ErrPTooBig + str := fmt.Sprintf("P value of %d is greater than max allowed 32", P) + return nil, makeError(ErrPTooBig, str) } // Create the filter object and insert metadata. @@ -141,7 +131,8 @@ func NewFilter(P uint8, key [KeySize]byte, data [][]byte) (*Filter, error) { func FromBytes(N uint32, P uint8, d []byte) (*Filter, error) { // Basic sanity check. if P > 32 { - return nil, ErrPTooBig + str := fmt.Sprintf("P value of %d is greater than max allowed 32", P) + return nil, makeError(ErrPTooBig, str) } // Save the filter data internally as n + filter bytes @@ -165,7 +156,8 @@ func FromNBytes(P uint8, d []byte) (*Filter, error) { if len(d) >= 4 { n = binary.BigEndian.Uint32(d[:4]) } else if len(d) < 4 && len(d) != 0 { - return nil, ErrMisserialized + str := "number of items serialization missing" + return nil, makeError(ErrMisserialized, str) } f := &Filter{ diff --git a/gcs/gcs_test.go b/gcs/gcs_test.go index 45896225..3b70b025 100644 --- a/gcs/gcs_test.go +++ b/gcs/gcs_test.go @@ -327,20 +327,20 @@ func TestFilterCorners(t *testing.T) { const largeP = 33 var key [KeySize]byte _, err := NewFilter(largeP, key, nil) - if err != ErrPTooBig { + if !IsErrorCode(err, ErrPTooBig) { t.Fatalf("did not receive expected err for P too big -- got %v, want %v", err, ErrPTooBig) } _, err = FromBytes(0, largeP, nil) - if err != ErrPTooBig { + if !IsErrorCode(err, ErrPTooBig) { t.Fatalf("did not receive expected err for P too big -- got %v, want %v", err, ErrPTooBig) } // Attempt to decode a filter without the N value serialized properly. _, err = FromNBytes(20, []byte{0x00}) - if err != ErrMisserialized { - t.Fatalf("did not receive expected err -- got %v, want %v", - err, ErrMisserialized) + if !IsErrorCode(err, ErrMisserialized) { + t.Fatalf("did not receive expected err -- got %v, want %v", err, + ErrMisserialized) } }