From 70611a1bbe7d618465dca10f112d5a737379cebe Mon Sep 17 00:00:00 2001 From: Mike Fridman Date: Sun, 12 Nov 2023 11:49:36 -0500 Subject: [PATCH 1/2] feat: Gracefully recover Go migrations that panic --- provider_run.go | 8 +++++++- provider_run_test.go | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/provider_run.go b/provider_run.go index e69f94dcd..45992a954 100644 --- a/provider_run.go +++ b/provider_run.go @@ -424,7 +424,13 @@ func runMigration(ctx context.Context, db database.DBTxConn, m *Migration, direc // runGo is a helper function that runs the given Go functions in the given direction. It must only // be called after the migration has been initialized. -func runGo(ctx context.Context, db database.DBTxConn, m *Migration, direction bool) error { +func runGo(ctx context.Context, db database.DBTxConn, m *Migration, direction bool) (retErr error) { + defer func() { + if r := recover(); r != nil { + retErr = fmt.Errorf("panic: %v", r) + } + }() + switch db := db.(type) { case *sql.Conn: return fmt.Errorf("go migrations are not supported with *sql.Conn") diff --git a/provider_run_test.go b/provider_run_test.go index f30e09e22..43c257d26 100644 --- a/provider_run_test.go +++ b/provider_run_test.go @@ -724,6 +724,27 @@ func TestSQLiteSharedCache(t *testing.T) { }) } +func TestGoMigrationPanic(t *testing.T) { + t.Parallel() + + ctx := context.Background() + migration := goose.NewGoMigration( + 1, + &goose.GoFunc{RunTx: func(ctx context.Context, tx *sql.Tx) error { panic("something went wrong") }}, + nil, + ) + p, err := goose.NewProvider(goose.DialectSQLite3, newDB(t), nil, + goose.WithGoMigrations(migration), // Add a Go migration that panics. + ) + check.NoError(t, err) + _, err = p.Up(ctx) + check.HasError(t, err) + check.Contains(t, err.Error(), "panic: something went wrong") + var expected *goose.PartialError + check.Bool(t, errors.As(err, &expected), true) + check.Contains(t, expected.Err.Error(), "panic: something went wrong") +} + func TestCustomStoreTableExists(t *testing.T) { t.Parallel() From b77e95c54374a1f34358d5128b51c635c4bffdf7 Mon Sep 17 00:00:00 2001 From: Mike Fridman Date: Sun, 12 Nov 2023 11:58:01 -0500 Subject: [PATCH 2/2] add stack trace to error output --- provider_run.go | 3 ++- provider_run_test.go | 13 ++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/provider_run.go b/provider_run.go index 45992a954..95b4421ce 100644 --- a/provider_run.go +++ b/provider_run.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io/fs" + "runtime/debug" "sort" "strconv" "strings" @@ -427,7 +428,7 @@ func runMigration(ctx context.Context, db database.DBTxConn, m *Migration, direc func runGo(ctx context.Context, db database.DBTxConn, m *Migration, direction bool) (retErr error) { defer func() { if r := recover(); r != nil { - retErr = fmt.Errorf("panic: %v", r) + retErr = fmt.Errorf("panic: %v\n%s", r, debug.Stack()) } }() diff --git a/provider_run_test.go b/provider_run_test.go index 43c257d26..e6926ea16 100644 --- a/provider_run_test.go +++ b/provider_run_test.go @@ -728,9 +728,16 @@ func TestGoMigrationPanic(t *testing.T) { t.Parallel() ctx := context.Background() + const ( + wantErrString = "panic: runtime error: index out of range [7] with length 0" + ) migration := goose.NewGoMigration( 1, - &goose.GoFunc{RunTx: func(ctx context.Context, tx *sql.Tx) error { panic("something went wrong") }}, + &goose.GoFunc{RunTx: func(ctx context.Context, tx *sql.Tx) error { + var ss []int + _ = ss[7] + return nil + }}, nil, ) p, err := goose.NewProvider(goose.DialectSQLite3, newDB(t), nil, @@ -739,10 +746,10 @@ func TestGoMigrationPanic(t *testing.T) { check.NoError(t, err) _, err = p.Up(ctx) check.HasError(t, err) - check.Contains(t, err.Error(), "panic: something went wrong") + check.Contains(t, err.Error(), wantErrString) var expected *goose.PartialError check.Bool(t, errors.As(err, &expected), true) - check.Contains(t, expected.Err.Error(), "panic: something went wrong") + check.Contains(t, expected.Err.Error(), wantErrString) } func TestCustomStoreTableExists(t *testing.T) {