Skip to content

Commit

Permalink
introduce migration error, include stack trace for recovered panics
Browse files Browse the repository at this point in the history
  • Loading branch information
turbolent committed Mar 6, 2024
1 parent 2f8bed4 commit a3de7d7
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 147 deletions.
34 changes: 7 additions & 27 deletions migrations/capcons/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,9 @@ type testMigration struct {
migration string
}

type testMigrationError struct {
storageKey interpreter.StorageKey
storageMapKey interpreter.StorageMapKey
migration string
err error
}

type testMigrationReporter struct {
migrations []testMigration
errors []testMigrationError
errors []error
linkMigrations []testCapConsLinkMigration
pathCapabilityMigrations []testCapConsPathCapabilityMigration
missingCapabilityIDs []testCapConsMissingCapabilityID
Expand All @@ -106,21 +99,8 @@ func (t *testMigrationReporter) Migrated(
)
}

func (t *testMigrationReporter) Error(
storageKey interpreter.StorageKey,
storageMapKey interpreter.StorageMapKey,
migration string,
err error,
) {
t.errors = append(
t.errors,
testMigrationError{
storageKey: storageKey,
storageMapKey: storageMapKey,
migration: migration,
err: err,
},
)
func (t *testMigrationReporter) Error(err error) {
t.errors = append(t.errors, err)
}
func (t *testMigrationReporter) MigratedLink(
accountAddressPath interpreter.AddressPath,
Expand Down Expand Up @@ -252,7 +232,7 @@ func testPathCapabilityValueMigration(
pathLinks []testLink,
accountLinks []interpreter.PathValue,
expectedMigrations []testMigration,
expectedErrors []testMigrationError,
expectedErrors []error,
expectedPathMigrations []testCapConsPathCapabilityMigration,
expectedMissingCapabilityIDs []testCapConsMissingCapabilityID,
setupFunction, checkFunction string,
Expand Down Expand Up @@ -580,7 +560,7 @@ func TestPathCapabilityValueMigration(t *testing.T) {
pathLinks []testLink
accountLinks []interpreter.PathValue
expectedMigrations []testMigration
expectedErrors []testMigrationError
expectedErrors []error
expectedPathMigrations []testCapConsPathCapabilityMigration
expectedMissingCapabilityIDs []testCapConsMissingCapabilityID
borrowShouldFail bool
Expand Down Expand Up @@ -1237,7 +1217,7 @@ func testLinkMigration(
pathLinks []testLink,
accountLinks []interpreter.PathValue,
expectedMigrations []testMigration,
expectedErrors []testMigrationError,
expectedErrors []error,
expectedLinkMigrations []testCapConsLinkMigration,
expectedCyclicLinkErrors []CyclicLinkError,
expectedMissingTargets []interpreter.AddressPath,
Expand Down Expand Up @@ -1381,7 +1361,7 @@ func TestLinkMigration(t *testing.T) {
pathLinks []testLink
accountLinks []interpreter.PathValue
expectedMigrations []testMigration
expectedErrors []testMigrationError
expectedErrors []error
expectedLinkMigrations []testCapConsLinkMigration
expectedCyclicLinkErrors []CyclicLinkError
expectedMissingTargets []interpreter.AddressPath
Expand Down
29 changes: 3 additions & 26 deletions migrations/entitlements/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2987,10 +2987,7 @@ type testReporter struct {
interpreter.StorageKey
interpreter.StorageMapKey
}]struct{}
errors map[struct {
interpreter.StorageKey
interpreter.StorageMapKey
}][]error
errors []error
}

func newTestReporter() *testReporter {
Expand All @@ -2999,10 +2996,6 @@ func newTestReporter() *testReporter {
interpreter.StorageKey
interpreter.StorageMapKey
}]struct{}{},
errors: map[struct {
interpreter.StorageKey
interpreter.StorageMapKey
}][]error{},
}
}

Expand All @@ -3020,24 +3013,8 @@ func (t *testReporter) Migrated(
}] = struct{}{}
}

func (t *testReporter) Error(
storageKey interpreter.StorageKey,
storageMapKey interpreter.StorageMapKey,
_ string,
err error,
) {
key := struct {
interpreter.StorageKey
interpreter.StorageMapKey
}{
StorageKey: storageKey,
StorageMapKey: storageMapKey,
}

t.errors[key] = append(
t.errors[key],
err,
)
func (t *testReporter) Error(err error) {
t.errors = append(t.errors, err)
}

func TestRehash(t *testing.T) {
Expand Down
94 changes: 62 additions & 32 deletions migrations/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
package migrations

import (
goRuntime "runtime"
"fmt"
"runtime/debug"

"github.com/onflow/cadence/runtime"
"github.com/onflow/cadence/runtime/common"
Expand Down Expand Up @@ -143,28 +144,28 @@ func (m *StorageMigration) MigrateNestedValue(
) (migratedValue interpreter.Value) {

defer func() {
// Here it catches the panics that may occur at the framework level,
// Here it catches the panics that may occur at the framework level,
// even before going to each individual migration. e.g: iterating the dictionary for elements.
//
// There is a similar recovery at the `StorageMigration.migrate()` method,
// which handles panics from each individual migrations (e.g: capcon migration, static type migration, etc.).

if r := recover(); r != nil {
switch r := r.(type) {
case goRuntime.Error:
panic(r)

case error:
if reporter != nil {
reporter.Error(
storageKey,
storageMapKey,
"StorageMigration",
r,
)
}
default:
panic(r)
err, ok := r.(error)
if !ok {
err = fmt.Errorf("%v", r)
}

Check warning on line 157 in migrations/migration.go

View check run for this annotation

Codecov / codecov/patch

migrations/migration.go#L154-L157

Added lines #L154 - L157 were not covered by tests

err = StorageMigrationError{
StorageKey: storageKey,
StorageMapKey: storageMapKey,
Migration: "StorageMigration",
Err: err,
Stack: debug.Stack(),
}

if reporter != nil {
reporter.Error(err)

Check warning on line 168 in migrations/migration.go

View check run for this annotation

Codecov / codecov/patch

migrations/migration.go#L159-L168

Added lines #L159 - L168 were not covered by tests
}
}
}()
Expand Down Expand Up @@ -379,12 +380,16 @@ func (m *StorageMigration) MigrateNestedValue(

if err != nil {
if reporter != nil {
reporter.Error(
storageKey,
storageMapKey,
migration.Name(),
err,
)
if _, ok := err.(StorageMigrationError); !ok {
err = StorageMigrationError{
StorageKey: storageKey,
StorageMapKey: storageMapKey,
Migration: migration.Name(),
Err: err,
}
}

reporter.Error(err)
}
continue
}
Expand Down Expand Up @@ -425,12 +430,34 @@ func (m *StorageMigration) MigrateNestedValue(

}

type StorageMigrationError struct {
StorageKey interpreter.StorageKey
StorageMapKey interpreter.StorageMapKey
Migration string
Err error
Stack []byte
}

func (e StorageMigrationError) Error() string {
return fmt.Sprintf(
"failed to perform migration %s for %s, %s: %s\n%s",
e.Migration,
e.StorageKey,
e.StorageMapKey,
e.Err.Error(),
e.Stack,
)
}

func (m *StorageMigration) migrate(
migration ValueMigration,
storageKey interpreter.StorageKey,
storageMapKey interpreter.StorageMapKey,
value interpreter.Value,
) (converted interpreter.Value, err error) {
) (
converted interpreter.Value,
err error,
) {

// Handles panics from each individual migrations (e.g: capcon migration, static type migration, etc.).
// So even if one migration panics, others could still run (i.e: panics are caught inside the loop).
Expand All @@ -439,15 +466,18 @@ func (m *StorageMigration) migrate(
// They are caught at `StorageMigration.MigrateNestedValue()`.
defer func() {
if r := recover(); r != nil {
switch r := r.(type) {
case goRuntime.Error:
panic(r)

case error:
err = r
var ok bool
err, ok = r.(error)
if !ok {
err = fmt.Errorf("%v", r)
}

Check warning on line 473 in migrations/migration.go

View check run for this annotation

Codecov / codecov/patch

migrations/migration.go#L472-L473

Added lines #L472 - L473 were not covered by tests

default:
panic(r)
err = StorageMigrationError{
StorageKey: storageKey,
StorageMapKey: storageMapKey,
Migration: migration.Name(),
Err: err,
Stack: debug.Stack(),
}
}
}()
Expand Down
7 changes: 1 addition & 6 deletions migrations/migration_reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,5 @@ type Reporter interface {
storageMapKey interpreter.StorageMapKey,
migration string,
)
Error(
storageKey interpreter.StorageKey,
storageMapKey interpreter.StorageMapKey,
migration string,
err error,
)
Error(err error)
}
Loading

0 comments on commit a3de7d7

Please sign in to comment.