Skip to content

Commit

Permalink
Merge pull request #295 from onflow/fxamacker/refactor-errors
Browse files Browse the repository at this point in the history
Refactor error handling and recategorize errors
  • Loading branch information
fxamacker authored Mar 23, 2023
2 parents c836704 + 036f757 commit 76ac577
Show file tree
Hide file tree
Showing 16 changed files with 1,497 additions and 574 deletions.
380 changes: 296 additions & 84 deletions array.go

Large diffs are not rendered by default.

127 changes: 74 additions & 53 deletions array_debug.go

Large diffs are not rendered by default.

100 changes: 91 additions & 9 deletions array_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,13 @@ func TestArrayAppendAndGet(t *testing.T) {

storable, err := array.Get(array.Count())
require.Nil(t, storable)
require.Equal(t, 1, errorCategorizationCount(err))

var userError *UserError
var indexOutOfBoundsError *IndexOutOfBoundsError
require.ErrorAs(t, err, &userError)
require.ErrorAs(t, err, &indexOutOfBoundsError)
require.ErrorAs(t, userError, &indexOutOfBoundsError)

verifyArray(t, storage, typeInfo, address, array, values, false)
}
Expand Down Expand Up @@ -316,8 +321,13 @@ func TestArraySetAndGet(t *testing.T) {
v := NewStringValue(randStr(r, 1024))
storable, err := array.Set(array.Count(), v)
require.Nil(t, storable)
require.Equal(t, 1, errorCategorizationCount(err))

var userError *UserError
var indexOutOfBoundsError *IndexOutOfBoundsError
require.ErrorAs(t, err, &userError)
require.ErrorAs(t, err, &indexOutOfBoundsError)
require.ErrorAs(t, userError, &indexOutOfBoundsError)

verifyArray(t, storage, typeInfo, address, array, values, false)
})
Expand Down Expand Up @@ -428,8 +438,13 @@ func TestArrayInsertAndGet(t *testing.T) {

v := NewStringValue(randStr(r, 1024))
err = array.Insert(array.Count()+1, v)
require.Equal(t, 1, errorCategorizationCount(err))

var userError *UserError
var indexOutOfBoundsError *IndexOutOfBoundsError
require.ErrorAs(t, err, &userError)
require.ErrorAs(t, err, &indexOutOfBoundsError)
require.ErrorAs(t, userError, &indexOutOfBoundsError)

verifyArray(t, storage, typeInfo, address, array, values, false)
})
Expand Down Expand Up @@ -609,8 +624,13 @@ func TestArrayRemove(t *testing.T) {

storable, err := array.Remove(array.Count())
require.Nil(t, storable)
require.Equal(t, 1, errorCategorizationCount(err))

var userError *UserError
var indexOutOfBounds *IndexOutOfBoundsError
require.ErrorAs(t, err, &userError)
require.ErrorAs(t, err, &indexOutOfBounds)
require.ErrorAs(t, userError, &indexOutOfBounds)

verifyArray(t, storage, typeInfo, address, array, values, false)
})
Expand Down Expand Up @@ -822,8 +842,12 @@ func TestArrayIterate(t *testing.T) {
i++
return true, nil
})
require.Error(t, err)
require.Equal(t, testErr, err)
// err is testErr wrapped in ExternalError.
require.Equal(t, 1, errorCategorizationCount(err))
var externalError *ExternalError
require.ErrorAs(t, err, &externalError)
require.Equal(t, testErr, externalError.Unwrap())

require.Equal(t, count/2, i)
})
}
Expand All @@ -841,15 +865,23 @@ func testArrayIterateRange(t *testing.T, storage *PersistentSlabStorage, array *
i++
return true, nil
})
require.Equal(t, 1, errorCategorizationCount(err))

var userError *UserError
require.ErrorAs(t, err, &userError)
require.ErrorAs(t, err, &sliceOutOfBoundsError)
require.ErrorAs(t, userError, &sliceOutOfBoundsError)
require.Equal(t, uint64(0), i)

// If endIndex > count, IterateRange returns SliceOutOfBoundsError
err = array.IterateRange(0, count+1, func(v Value) (bool, error) {
i++
return true, nil
})
require.Equal(t, 1, errorCategorizationCount(err))
require.ErrorAs(t, err, &userError)
require.ErrorAs(t, err, &sliceOutOfBoundsError)
require.ErrorAs(t, userError, &sliceOutOfBoundsError)
require.Equal(t, uint64(0), i)

// If startIndex > endIndex, IterateRange returns InvalidSliceIndexError
Expand All @@ -858,7 +890,10 @@ func testArrayIterateRange(t *testing.T, storage *PersistentSlabStorage, array *
i++
return true, nil
})
require.Equal(t, 1, errorCategorizationCount(err))
require.ErrorAs(t, err, &userError)
require.ErrorAs(t, err, &invalidSliceIndexError)
require.ErrorAs(t, userError, &invalidSliceIndexError)
require.Equal(t, uint64(0), i)
}

Expand Down Expand Up @@ -984,11 +1019,15 @@ func TestArrayIterateRange(t *testing.T) {
i++
return true, nil
})
require.Error(t, err)
require.Equal(t, testErr, err)
// err is testErr wrapped in ExternalError.
require.Equal(t, 1, errorCategorizationCount(err))
var externalError *ExternalError
require.ErrorAs(t, err, &externalError)
require.Equal(t, testErr, externalError.Unwrap())
require.Equal(t, count/2, i)
})
}

func TestArrayRootStorageID(t *testing.T) {
SetThreshold(256)
defer SetThreshold(1024)
Expand Down Expand Up @@ -1767,24 +1806,44 @@ func TestEmptyArray(t *testing.T) {

t.Run("get", func(t *testing.T) {
s, err := array.Get(0)
require.Error(t, err, IndexOutOfBoundsError{})
require.Equal(t, 1, errorCategorizationCount(err))
var userError *UserError
var indexOutOfBoundsError *IndexOutOfBoundsError
require.ErrorAs(t, err, &userError)
require.ErrorAs(t, err, &indexOutOfBoundsError)
require.ErrorAs(t, userError, &indexOutOfBoundsError)
require.Nil(t, s)
})

t.Run("set", func(t *testing.T) {
s, err := array.Set(0, Uint64Value(0))
require.Error(t, err, IndexOutOfBoundsError{})
require.Equal(t, 1, errorCategorizationCount(err))
var userError *UserError
var indexOutOfBoundsError *IndexOutOfBoundsError
require.ErrorAs(t, err, &userError)
require.ErrorAs(t, err, &indexOutOfBoundsError)
require.ErrorAs(t, userError, &indexOutOfBoundsError)
require.Nil(t, s)
})

t.Run("insert", func(t *testing.T) {
err := array.Insert(1, Uint64Value(0))
require.Error(t, err, IndexOutOfBoundsError{})
require.Equal(t, 1, errorCategorizationCount(err))
var userError *UserError
var indexOutOfBoundsError *IndexOutOfBoundsError
require.ErrorAs(t, err, &userError)
require.ErrorAs(t, err, &indexOutOfBoundsError)
require.ErrorAs(t, userError, &indexOutOfBoundsError)
})

t.Run("remove", func(t *testing.T) {
s, err := array.Remove(0)
require.Error(t, err, IndexOutOfBoundsError{})
require.Equal(t, 1, errorCategorizationCount(err))
var userError *UserError
var indexOutOfBoundsError *IndexOutOfBoundsError
require.ErrorAs(t, err, &userError)
require.ErrorAs(t, err, &indexOutOfBoundsError)
require.ErrorAs(t, userError, &indexOutOfBoundsError)
require.Nil(t, s)
})

Expand Down Expand Up @@ -1922,7 +1981,12 @@ func TestArrayStoredValue(t *testing.T) {

verifyArray(t, storage, typeInfo, address, array2, values, false)
} else {
require.Error(t, err)
require.Equal(t, 1, errorCategorizationCount(err))
var fatalError *FatalError
var notValueError *NotValueError
require.ErrorAs(t, err, &fatalError)
require.ErrorAs(t, err, &notValueError)
require.ErrorAs(t, fatalError, &notValueError)
require.Nil(t, value)
}
}
Expand Down Expand Up @@ -2510,3 +2574,21 @@ func TestArraySlabDump(t *testing.T) {
require.Equal(t, want, dumps)
})
}

func errorCategorizationCount(err error) int {
var fatalError *FatalError
var userError *UserError
var externalError *ExternalError

count := 0
if errors.As(err, &fatalError) {
count++
}
if errors.As(err, &userError) {
count++
}
if errors.As(err, &externalError) {
count++
}
return count
}
46 changes: 35 additions & 11 deletions basicarray.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ func newBasicArrayDataSlabFromData(
for i := 0; i < int(elemCount); i++ {
storable, err := decodeStorable(cborDec, StorageIDUndefined)
if err != nil {
return nil, NewDecodingError(err)
// Wrap err as external error (if needed) because err is returned by StorableDecoder callback.
return nil, wrapErrorfAsExternalErrorIfNeeded(err, "failed to decode array element")
}
elements[i] = storable
}
Expand Down Expand Up @@ -115,7 +116,8 @@ func (a *BasicArrayDataSlab) Encode(enc *Encoder) error {
for i := 0; i < len(a.elements); i++ {
err := a.elements[i].Encode(enc)
if err != nil {
return NewEncodingError(err)
// Wrap err as external error (if needed) because err is returned by Storable interface.
return wrapErrorfAsExternalErrorIfNeeded(err, "failed to encode array element")
}
}
err = enc.CBOR.Flush()
Expand Down Expand Up @@ -155,7 +157,8 @@ func (a *BasicArrayDataSlab) Set(storage SlabStorage, index uint64, v Storable)

err := storage.Store(a.header.id, a)
if err != nil {
return err
// Wrap err as external error (if needed) because err is returned by SlabStorage interface.
return wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to store slab %s", a.header.id))
}

return nil
Expand All @@ -179,7 +182,8 @@ func (a *BasicArrayDataSlab) Insert(storage SlabStorage, index uint64, v Storabl

err := storage.Store(a.header.id, a)
if err != nil {
return err
// Wrap err as external error (if needed) because err is returned by SlabStorage interface.
return wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to store slab %s", a.header.id))
}

return nil
Expand Down Expand Up @@ -207,7 +211,8 @@ func (a *BasicArrayDataSlab) Remove(storage SlabStorage, index uint64) (Storable

err := storage.Store(a.header.id, a)
if err != nil {
return nil, err
// Wrap err as external error (if needed) because err is returned by SlabStorage interface.
return nil, wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to store slab %s", a.header.id))
}

return v, nil
Expand Down Expand Up @@ -252,7 +257,8 @@ func (a *BasicArrayDataSlab) BorrowFromRight(_ Slab) error {
func NewBasicArray(storage SlabStorage, address Address) (*BasicArray, error) {
sID, err := storage.GenerateStorageID(address)
if err != nil {
return nil, err
// Wrap err as external error (if needed) because err is returned by SlabStorage interface.
return nil, wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to generate storage ID for address 0x%x", address))
}

root := &BasicArrayDataSlab{
Expand Down Expand Up @@ -282,7 +288,8 @@ func NewBasicArrayWithRootID(storage SlabStorage, id StorageID) (*BasicArray, er
}
slab, found, err := storage.Retrieve(id)
if err != nil {
return nil, err
// Wrap err as external error (if needed) because err is returned by SlabStorage interface.
return nil, wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to retrieve slab %s", id))
}
if !found {
return nil, NewSlabNotFoundErrorf(id, "BasicArray slab not found")
Expand All @@ -297,38 +304,55 @@ func NewBasicArrayWithRootID(storage SlabStorage, id StorageID) (*BasicArray, er
func (a *BasicArray) Get(index uint64) (Value, error) {
storable, err := a.root.Get(a.storage, index)
if err != nil {
// Don't need to wrap error as external error because err is already categorized by BasicArrayDataSlab.Get().
return nil, err
}
return storable.StoredValue(a.storage)
value, err := storable.StoredValue(a.storage)
if err != nil {
// Wrap err as external error (if needed) because err is returned by Storable interface.
return nil, wrapErrorfAsExternalErrorIfNeeded(err, "failed to get storable's stored value")
}
return value, nil
}

func (a *BasicArray) Set(index uint64, v Value) error {
storable, err := v.Storable(a.storage, a.Address(), MaxInlineArrayElementSize)
if err != nil {
return err
// Wrap err as external error (if needed) because err is returned by Value interface.
return wrapErrorfAsExternalErrorIfNeeded(err, "failed to get value's storable")
}
// Don't need to wrap error as external error because err is already categorized by BasicArrayDataSlab.Set().
return a.root.Set(a.storage, index, storable)
}

func (a *BasicArray) Append(v Value) error {
index := uint64(a.root.header.count)
// Don't need to wrap error as external error because err is already categorized by BasicArray.Insert().
return a.Insert(index, v)
}

func (a *BasicArray) Insert(index uint64, v Value) error {
storable, err := v.Storable(a.storage, a.Address(), MaxInlineArrayElementSize)
if err != nil {
return err
// Wrap err as external error (if needed) because err is returned by Value interface.
return wrapErrorfAsExternalErrorIfNeeded(err, "failed to get value's storable")
}
// Don't need to wrap error as external error because err is already categorized by BasicArrayDataSlab.Insert().
return a.root.Insert(a.storage, index, storable)
}

func (a *BasicArray) Remove(index uint64) (Value, error) {
storable, err := a.root.Remove(a.storage, index)
if err != nil {
// Don't need to wrap error as external error because err is already categorized by BasicArrayDataSlab.Remove().
return nil, err
}
return storable.StoredValue(a.storage)
value, err := storable.StoredValue(a.storage)
if err != nil {
// Wrap err as external error (if needed) because err is returned by Storable interface.
return nil, wrapErrorfAsExternalErrorIfNeeded(err, "failed to get storable's stored value")
}
return value, nil
}

func (a *BasicArray) Count() uint64 {
Expand Down
2 changes: 1 addition & 1 deletion cmd/stress/storable.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ func (v StringValue) Storable(storage atree.SlabStorage, address atree.Address,
// Create StorableSlab
id, err := storage.GenerateStorageID(address)
if err != nil {
return nil, atree.NewStorageError(err)
return nil, err
}

slab := &atree.StorableSlab{
Expand Down
3 changes: 2 additions & 1 deletion encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ func DecodeSlab(
cborDec := decMode.NewByteStreamDecoder(data[versionAndFlagSize:])
storable, err := decodeStorable(cborDec, id)
if err != nil {
return nil, NewDecodingError(err)
// Wrap err as external error (if needed) because err is returned by StorableDecoder callback.
return nil, wrapErrorfAsExternalErrorIfNeeded(err, "failed to decode slab storable")
}
return StorableSlab{
StorageID: id,
Expand Down
Loading

0 comments on commit 76ac577

Please sign in to comment.