Skip to content

feat(pkg/scale): add use of pkg/error Wrap for error handling #2708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 55 commits into from
Oct 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
4724528
add use of pkg/error Wrap for error handling
edwardmack Jul 27, 2022
f0e3fb4
update tests to handle additional error messages
edwardmack Jul 28, 2022
3bbdf88
add error wraping to encode
edwardmack Jul 28, 2022
38a5b41
add pkg/errors to result
edwardmack Jul 28, 2022
27f769c
add use of pkg/errors
edwardmack Jul 28, 2022
ae761a8
update error handling to use fmt.Errorf
edwardmack Aug 2, 2022
0689a34
refactor error handling for encode functions
edwardmack Aug 3, 2022
b710205
refactor errors to use fmt.Errorf
edwardmack Aug 3, 2022
441edf9
address PR comments
edwardmack Aug 4, 2022
05a0630
refactor error wrapping
edwardmack Aug 4, 2022
dcb89f4
WIP address PR comments
edwardmack Aug 5, 2022
6cdbee5
Apply suggestions from code review
edwardmack Aug 5, 2022
52cb66c
address PR comments
edwardmack Aug 5, 2022
e42f66a
refactor error handling
edwardmack Aug 5, 2022
375fcc7
fix spelling error
edwardmack Aug 5, 2022
9658a31
update tests error results check
edwardmack Aug 5, 2022
a3a1c1a
fix lint
edwardmack Aug 5, 2022
c31675c
update test error strings
edwardmack Aug 5, 2022
f798426
update error string in tests
edwardmack Aug 5, 2022
fb4a32b
update test error message string
edwardmack Aug 6, 2022
deb9f77
address merge conflicts
edwardmack Sep 14, 2022
9b5b6f4
add check for sentinal EOF error
edwardmack Sep 15, 2022
e102ecd
correct decode code
edwardmack Sep 15, 2022
7d6101e
add use of pkg/error Wrap for error handling
edwardmack Jul 27, 2022
9e2e515
update tests to handle additional error messages
edwardmack Jul 28, 2022
4a2c7e2
add error wraping to encode
edwardmack Jul 28, 2022
cc9f8a5
add pkg/errors to result
edwardmack Jul 28, 2022
af22699
add use of pkg/errors
edwardmack Jul 28, 2022
c026d58
update error handling to use fmt.Errorf
edwardmack Aug 2, 2022
bd3c307
refactor errors to use fmt.Errorf
edwardmack Aug 3, 2022
74f3b92
address PR comments
edwardmack Aug 4, 2022
ec10128
refactor error wrapping
edwardmack Aug 4, 2022
d244cc9
WIP address PR comments
edwardmack Aug 5, 2022
c33323d
Apply suggestions from code review
edwardmack Aug 5, 2022
ecb9f16
address PR comments
edwardmack Aug 5, 2022
8ed1bc1
update tests error results check
edwardmack Aug 5, 2022
b4dd52c
fix lint
edwardmack Aug 5, 2022
67242e3
update test error strings
edwardmack Aug 5, 2022
a43e623
address merge conflicts
edwardmack Sep 14, 2022
65e22b9
update tests for updated error messages
edwardmack Sep 15, 2022
5a9e674
update error check in test
edwardmack Sep 16, 2022
85329b4
resolve rebase conflicts, address PR comments
edwardmack Sep 19, 2022
e252d48
resolve rebase conflicts
edwardmack Sep 19, 2022
c8a3b68
fix check in decodeUint
edwardmack Sep 20, 2022
049abed
clean up sentinal errors
edwardmack Sep 20, 2022
0d9a34e
address PR comments
edwardmack Sep 26, 2022
b19f590
address PR comments
edwardmack Sep 29, 2022
fe80f3e
add to reading buffer error message
edwardmack Sep 30, 2022
1a6d0a8
add VDTNotSet error
edwardmack Sep 30, 2022
04a6eb1
update tests to conform to updated error messages
edwardmack Oct 3, 2022
af25576
unexport errors for private functions
edwardmack Oct 3, 2022
31b3115
refactor error messages
edwardmack Oct 6, 2022
a110c3c
update tests to conform to error messages
edwardmack Oct 6, 2022
eeb2b77
remove un-needed error prefixes
edwardmack Oct 14, 2022
620dec5
address PR comments
edwardmack Oct 18, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dot/rpc/modules/author_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func TestAuthorModule_HasSessionKeys(t *testing.T) {
req: &HasSessionKeyRequest{"0x01"},
},
exp: false,
expErr: errors.New("unsupported Option value: 4, bytes: [1]"),
expErr: errors.New("unsupported option: value: 4, bytes: [1]"),
},
{
name: "happy path",
Expand Down
13 changes: 8 additions & 5 deletions lib/babe/verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,10 @@ func Test_getAuthorityIndex(t *testing.T) {
expErr: errors.New("first digest item is not pre-runtime digest"),
},
{
name: "Invalid Preruntime Digest Type",
args: args{headerInvalidPre},
expErr: errors.New("cannot decode babe header from pre-digest: EOF, field: 0"),
name: "Invalid Preruntime Digest Type",
args: args{headerInvalidPre},
expErr: errors.New("cannot decode babe header from pre-digest: decoding struct: unmarshalling field at" +
" index 0: EOF"),
},
{
name: "BabePrimaryPreDigest Type",
Expand Down Expand Up @@ -382,7 +383,8 @@ func Test_verifier_verifyPreRuntimeDigest(t *testing.T) {
name: "Invalid PreRuntimeDigest",
verifier: verifier{},
args: args{&types.PreRuntimeDigest{Data: []byte{0}}},
expErr: errors.New("unable to find VaryingDataTypeValue with index: 0"),
expErr: errors.New(
"unable to find VaryingDataTypeValue with index: for key 0"),
},
{
name: "Invalid BlockProducer Index",
Expand Down Expand Up @@ -615,7 +617,8 @@ func Test_verifier_verifyAuthorshipRight(t *testing.T) {
name: "invalid preruntime digest data",
verifier: verifier{},
header: header2,
expErr: errors.New("failed to verify pre-runtime digest: EOF, field: 0"),
expErr: errors.New("failed to verify pre-runtime digest: decoding struct: unmarshalling field at index" +
" 0: EOF"),
},
{
name: "invalid seal length",
Expand Down
4 changes: 2 additions & 2 deletions lib/grandpa/message_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1286,8 +1286,8 @@ func TestService_VerifyBlockJustification(t *testing.T) {
justification: []byte{1, 2, 3},
},
want: nil,
wantErr: errors.New("EOF, field: 0x0000000000000000000000000000000000000000000000000000000000000000, " +
"field: {Hash:0x0000000000000000000000000000000000000000000000000000000000000000 Number:0 Precommits:[]}"),
wantErr: errors.New("decoding struct: unmarshalling field at index 1: decoding struct: unmarshalling" +
" field at index 0: EOF"),
},
"valid justification": {
fields: fields{
Expand Down
3 changes: 2 additions & 1 deletion lib/runtime/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"bytes"
"errors"
"fmt"
"io"

"github.com/ChainSafe/gossamer/pkg/scale"
)
Expand Down Expand Up @@ -69,7 +70,7 @@ func DecodeVersion(encoded []byte) (version Version, err error) {
for _, optionalField := range optionalFields {
err = decoder.Decode(optionalField.value)
if err != nil {
if err.Error() == "EOF" {
if errors.Is(err, io.EOF) {
return version, nil
}
return Version{}, fmt.Errorf("%w %s: %s", ErrDecodingVersionField, optionalField.name, err)
Expand Down
41 changes: 15 additions & 26 deletions pkg/scale/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,24 +60,15 @@ func indirect(dstv reflect.Value) (elem reflect.Value) {
func Unmarshal(data []byte, dst interface{}) (err error) {
dstv := reflect.ValueOf(dst)
if dstv.Kind() != reflect.Ptr || dstv.IsNil() {
err = fmt.Errorf("unsupported dst: %T, must be a pointer to a destination", dst)
err = fmt.Errorf("%w: %T", ErrUnsupportedDestination, dst)
return
}

elem := indirect(dstv)
if err != nil {
return
}

buf := &bytes.Buffer{}
ds := decodeState{}
_, err = buf.Write(data)
if err != nil {
return
}
ds.Reader = buf

err = ds.unmarshal(elem)
ds.Reader = bytes.NewBuffer(data)

err = ds.unmarshal(indirect(dstv))
if err != nil {
return
}
Expand All @@ -93,15 +84,15 @@ type Decoder struct {
func (d *Decoder) Decode(dst interface{}) (err error) {
dstv := reflect.ValueOf(dst)
if dstv.Kind() != reflect.Ptr || dstv.IsNil() {
err = fmt.Errorf("unsupported dst: %T, must be a pointer to a destination", dst)
err = fmt.Errorf("%w: %T", ErrUnsupportedDestination, dst)
return
}

elem := indirect(dstv)
err = d.unmarshal(indirect(dstv))
if err != nil {
return
}
return d.unmarshal(elem)
return nil
}

// NewDecoder is constructor for Decoder
Expand Down Expand Up @@ -160,7 +151,7 @@ func (ds *decodeState) unmarshal(dstv reflect.Value) (err error) {
case reflect.Slice:
err = ds.decodeSlice(dstv)
default:
err = fmt.Errorf("unsupported type: %T", in)
err = fmt.Errorf("%w: %T", ErrUnsupportedType, in)
}
}
return
Expand Down Expand Up @@ -244,7 +235,7 @@ func (ds *decodeState) decodeCustomPrimitive(dstv reflect.Value) (err error) {
break
}
default:
err = fmt.Errorf("unsupported type for custom primitive: %T", in)
err = fmt.Errorf("%w: %T", ErrUnsupportedType, in)
return
}
dstv.Set(temp.Elem().Convert(inType))
Expand Down Expand Up @@ -291,7 +282,7 @@ func (ds *decodeState) decodeResult(dstv reflect.Value) (err error) {
dstv.Set(reflect.ValueOf(res))
default:
bytes, _ := io.ReadAll(ds.Reader)
err = fmt.Errorf("unsupported Result value: %v, bytes: %v", rb, bytes)
err = fmt.Errorf("%w: value: %v, bytes: %v", ErrUnsupportedResult, rb, bytes)
}
return
}
Expand Down Expand Up @@ -324,7 +315,7 @@ func (ds *decodeState) decodePointer(dstv reflect.Value) (err error) {
}
default:
bytes, _ := io.ReadAll(ds.Reader)
err = fmt.Errorf("unsupported Option value: %v, bytes: %v", rb, bytes)
err = fmt.Errorf("%w: value: %v, bytes: %v", errUnsupportedOption, rb, bytes)
}
return
}
Expand Down Expand Up @@ -372,7 +363,7 @@ func (ds *decodeState) decodeVaryingDataType(dstv reflect.Value) (err error) {
vdt := dstv.Interface().(VaryingDataType)
val, ok := vdt.cache[uint(b)]
if !ok {
err = fmt.Errorf("unable to find VaryingDataTypeValue with index: %d", uint(b))
err = fmt.Errorf("%w: for key %d", errUnknownVaryingDataTypeValue, uint(b))
return
}

Expand Down Expand Up @@ -448,8 +439,7 @@ func (ds *decodeState) decodeStruct(dstv reflect.Value) (err error) {
}
err = ds.unmarshal(field)
if err != nil {
err = fmt.Errorf("%s, field: %+v", err, field)
return
return fmt.Errorf("decoding struct: unmarshalling field at index %d: %w", i.fieldIndex, err)
}
}
dstv.Set(temp.Elem())
Expand All @@ -470,7 +460,7 @@ func (ds *decodeState) decodeBool(dstv reflect.Value) (err error) {
case 0x01:
b = true
default:
err = fmt.Errorf("could not decode invalid bool")
err = fmt.Errorf("%w", errDecodeBool)
}
dstv.Set(reflect.ValueOf(b))
return
Expand Down Expand Up @@ -538,7 +528,6 @@ func (ds *decodeState) decodeUint(dstv reflect.Value) (err error) {
}
default:
return fmt.Errorf("%w: %d", ErrCompactUintPrefixUnknown, prefix)

}
}
temp.Elem().Set(reflect.ValueOf(value).Convert(reflect.TypeOf(in)))
Expand Down Expand Up @@ -636,7 +625,7 @@ func (ds *decodeState) decodeBigInt(dstv reflect.Value) (err error) {
buf := make([]byte, byteLen)
_, err = ds.Read(buf)
if err != nil {
err = fmt.Errorf("could not decode invalid big.Int: %v", err)
err = fmt.Errorf("reading bytes: %w", err)
break
}
o := reverseBytes(buf)
Expand Down
15 changes: 9 additions & 6 deletions pkg/scale/encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func (es *encodeState) marshal(in interface{}) (err error) {
case reflect.Slice:
err = es.encodeSlice(in)
default:
err = fmt.Errorf("unsupported type: %T", in)
err = fmt.Errorf("%w: %T", ErrUnsupportedType, in)
}
}
return
Expand Down Expand Up @@ -140,7 +140,7 @@ func (es *encodeState) encodeCustomPrimitive(in interface{}) (err error) {
case reflect.Uint64:
in = reflect.ValueOf(in).Convert(reflect.TypeOf(uint64(0))).Interface()
default:
err = fmt.Errorf("unsupported type for custom primitive: %T", in)
err = fmt.Errorf("%w: %T", ErrUnsupportedCustomPrimitive, in)
return
}
err = es.marshal(in)
Expand All @@ -149,7 +149,7 @@ func (es *encodeState) encodeCustomPrimitive(in interface{}) (err error) {

func (es *encodeState) encodeResult(res Result) (err error) {
if !res.IsSet() {
err = fmt.Errorf("Result is not set: %+v", res)
err = fmt.Errorf("%w: %+v", ErrResultNotSet, res)
return
}

Expand Down Expand Up @@ -230,7 +230,7 @@ func (es *encodeState) encodeArray(in interface{}) (err error) {
func (es *encodeState) encodeBigInt(i *big.Int) (err error) {
switch {
case i == nil:
err = fmt.Errorf("nil *big.Int")
err = fmt.Errorf("%w", errBigIntIsNil)
case i.Cmp(new(big.Int).Lsh(big.NewInt(1), 6)) < 0:
err = binary.Write(es, binary.LittleEndian, uint8(i.Int64()<<2))
case i.Cmp(new(big.Int).Lsh(big.NewInt(1), 14)) < 0:
Expand All @@ -247,6 +247,9 @@ func (es *encodeState) encodeBigInt(i *big.Int) (err error) {
if err == nil {
// write integer itself
err = binary.Write(es, binary.LittleEndian, reverseBytes(i.Bytes()))
if err != nil {
err = fmt.Errorf("writing bytes %s: %w", i, err)
}
}
}
return
Expand Down Expand Up @@ -299,7 +302,7 @@ func (es *encodeState) encodeFixedWidthInt(i interface{}) (err error) {
case uint64:
err = binary.Write(es, binary.LittleEndian, i)
default:
err = fmt.Errorf("could not encode fixed width integer, invalid type: %T", i)
err = fmt.Errorf("invalid type: %T", i)
}
return
}
Expand Down Expand Up @@ -374,7 +377,7 @@ func (es *encodeState) encodeUint(i uint) (err error) {
// encodeUint128 encodes a Uint128
func (es *encodeState) encodeUint128(i *Uint128) (err error) {
if i == nil {
err = fmt.Errorf("uint128 is nil: %v", i)
err = fmt.Errorf("%w", errUint128IsNil)
return
}
err = binary.Write(es, binary.LittleEndian, padBytes(i.Bytes(), binary.LittleEndian))
Expand Down
15 changes: 14 additions & 1 deletion pkg/scale/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,18 @@ package scale
import "errors"

var (
ErrVaryingDataTypeNotSet = errors.New("VaryingDataTypeValue has not been set")
ErrUnsupportedDestination = errors.New("must be a non-nil pointer to a destination")
errDecodeBool = errors.New("invalid byte for bool")
ErrUnsupportedType = errors.New("unsupported type")
ErrUnsupportedResult = errors.New("unsupported result")
errUnsupportedOption = errors.New("unsupported option")
errUnknownVaryingDataTypeValue = errors.New("unable to find VaryingDataTypeValue with index")
errUint128IsNil = errors.New("uint128 in nil")
ErrResultNotSet = errors.New("result not set")
ErrResultAlreadySet = errors.New("result already has an assigned value")
ErrUnsupportedVaryingDataTypeValue = errors.New("unsupported VaryingDataTypeValue")
ErrMustProvideVaryingDataTypeValue = errors.New("must provide at least one VaryingDataTypeValue")
errBigIntIsNil = errors.New("big int is nil")
ErrVaryingDataTypeNotSet = errors.New("varying data type not set")
ErrUnsupportedCustomPrimitive = errors.New("unsupported type for custom primitive")
)
3 changes: 0 additions & 3 deletions pkg/scale/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,10 @@
package scale

import (
"errors"
"fmt"
"reflect"
)

var ErrResultAlreadySet = errors.New("result already has an assigned value")

// ResultMode is the mode the Result is set to
type ResultMode int

Expand Down
2 changes: 1 addition & 1 deletion pkg/scale/uint128.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func (u *Uint128) UnmarshalJSON(data []byte) error {

dec, err := NewUint128(intVal)
if err != nil {
return err
return fmt.Errorf("creating uint128 from big integer: %w", err)
}
u.Upper = dec.Upper
u.Lower = dec.Lower
Expand Down
7 changes: 4 additions & 3 deletions pkg/scale/varying_data_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ func (vdts *VaryingDataTypeSlice) Add(values ...VaryingDataTypeValue) (err error
copied := vdts.VaryingDataType
err = copied.Set(val)
if err != nil {
err = fmt.Errorf("setting VaryingDataTypeValue: %w", err)
return
}
vdts.Types = append(vdts.Types, copied)
Expand All @@ -43,7 +44,7 @@ func mustNewVaryingDataTypeSliceAndSet(vdt VaryingDataType,
values ...VaryingDataTypeValue) (vdts VaryingDataTypeSlice) {
vdts = NewVaryingDataTypeSlice(vdt)
if err := vdts.Add(values...); err != nil {
panic(err)
panic(fmt.Sprintf("adding varying data type value: %s", err))
}
return
}
Expand All @@ -58,7 +59,7 @@ type VaryingDataType struct {
func (vdt *VaryingDataType) Set(value VaryingDataTypeValue) (err error) {
_, ok := vdt.cache[value.Index()]
if !ok {
err = fmt.Errorf("unable to append VaryingDataTypeValue: %T, not in cache", value)
err = fmt.Errorf("%w: %v (%T)", ErrUnsupportedVaryingDataTypeValue, value, value)
return
}
vdt.value = value
Expand All @@ -76,7 +77,7 @@ func (vdt *VaryingDataType) Value() (VaryingDataTypeValue, error) {
// NewVaryingDataType is constructor for VaryingDataType
func NewVaryingDataType(values ...VaryingDataTypeValue) (vdt VaryingDataType, err error) {
if len(values) == 0 {
err = fmt.Errorf("must provide atleast one VaryingDataTypeValue")
err = fmt.Errorf("%w", ErrMustProvideVaryingDataTypeValue)
return
}
vdt.cache = make(map[uint]VaryingDataTypeValue)
Expand Down