standardize panic recovery (#2530)

## Does this PR need a docs update or release note?

- [x]  No 

## Type of change

- [x] 🧹 Tech Debt/Cleanup

## Issue(s)

* #2529

## Test Plan

- [x]  Unit test
This commit is contained in:
Keepers 2023-02-21 11:36:56 -07:00 committed by GitHub
parent cfc2b2a206
commit 533164744b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 137 additions and 37 deletions

View File

@ -0,0 +1,54 @@
package crash
import (
"context"
"fmt"
"runtime"
"runtime/debug"
"github.com/alcionai/clues"
"github.com/alcionai/corso/src/pkg/logger"
)
// Recovery provides a deferrable func that can be called
// to recover from, and log context about, crashes.
// If an error is returned, then a panic recovery occurred.
//
// Call it as follows:
//
// defer func() {
// if crErr := crash.Recovery(ctx, recover()); crErr != nil {
// err = crErr // err needs to be a named return variable
// }
// }()
func Recovery(ctx context.Context, r any) error {
var (
err error
inFile string
)
if r != nil {
if re, ok := r.(error); ok {
err = re
} else if re, ok := r.(string); ok {
err = clues.New(re)
} else {
err = clues.New(fmt.Sprintf("%v", r))
}
_, file, _, ok := runtime.Caller(2)
if ok {
inFile = " in file: " + file
}
err = clues.Wrap(err, "panic recovery"+inFile).
WithClues(ctx).
With("stacktrace", string(debug.Stack()))
logger.Ctx(ctx).
With("err", err).
Errorw("backup panic", clues.InErr(err).Slice()...)
}
return err
}

View File

@ -0,0 +1,62 @@
package crash_test
import (
"testing"
"github.com/alcionai/corso/src/internal/common/crash"
"github.com/alcionai/corso/src/internal/tester"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
)
type CrashTestDummySuite struct {
tester.Suite
}
func TestCrashTestDummySuite(t *testing.T) {
suite.Run(t, &CrashTestDummySuite{Suite: tester.NewUnitSuite(t)})
}
func (suite *CrashTestDummySuite) TestRecovery() {
table := []struct {
name string
v any
expect assert.ErrorAssertionFunc
}{
{
name: "no panic",
v: nil,
expect: assert.NoError,
},
{
name: "error panic",
v: assert.AnError,
expect: assert.Error,
},
{
name: "string panic",
v: "an error",
expect: assert.Error,
},
{
name: "any panic",
v: map[string]string{"error": "yes"},
expect: assert.Error,
},
}
for _, test := range table {
suite.Run(test.name, func() {
t := suite.T()
ctx, flush := tester.NewContext()
defer func() {
test.expect(t, crash.Recovery(ctx, recover()))
flush()
}()
if test.v != nil {
panic(test.v)
}
})
}
}

View File

@ -2,8 +2,6 @@ package operations
import ( import (
"context" "context"
"fmt"
"runtime/debug"
"time" "time"
"github.com/alcionai/clues" "github.com/alcionai/clues"
@ -12,6 +10,7 @@ import (
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/alcionai/corso/src/internal/common" "github.com/alcionai/corso/src/internal/common"
"github.com/alcionai/corso/src/internal/common/crash"
"github.com/alcionai/corso/src/internal/connector" "github.com/alcionai/corso/src/internal/connector"
"github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/connector/support"
"github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/data"
@ -111,22 +110,8 @@ type detailsWriter interface {
// Run begins a synchronous backup operation. // Run begins a synchronous backup operation.
func (op *BackupOperation) Run(ctx context.Context) (err error) { func (op *BackupOperation) Run(ctx context.Context) (err error) {
defer func() { defer func() {
if r := recover(); r != nil { if crErr := crash.Recovery(ctx, recover()); crErr != nil {
var rerr error err = crErr
if re, ok := r.(error); ok {
rerr = re
} else if re, ok := r.(string); ok {
rerr = clues.New(re)
} else {
rerr = clues.New(fmt.Sprintf("%v", r))
}
err = clues.Wrap(rerr, "panic recovery").
WithClues(ctx).
With("stacktrace", string(debug.Stack()))
logger.Ctx(ctx).
With("err", err).
Errorw("backup panic", clues.InErr(err).Slice()...)
} }
}() }()

View File

@ -3,7 +3,6 @@ package operations
import ( import (
"context" "context"
"fmt" "fmt"
"runtime/debug"
"sort" "sort"
"time" "time"
@ -13,6 +12,7 @@ import (
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/alcionai/corso/src/internal/common" "github.com/alcionai/corso/src/internal/common"
"github.com/alcionai/corso/src/internal/common/crash"
"github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/connector/support"
"github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/data"
D "github.com/alcionai/corso/src/internal/diagnostics" D "github.com/alcionai/corso/src/internal/diagnostics"
@ -111,22 +111,8 @@ type restorer interface {
// Run begins a synchronous restore operation. // Run begins a synchronous restore operation.
func (op *RestoreOperation) Run(ctx context.Context) (restoreDetails *details.Details, err error) { func (op *RestoreOperation) Run(ctx context.Context) (restoreDetails *details.Details, err error) {
defer func() { defer func() {
if r := recover(); r != nil { if crErr := crash.Recovery(ctx, recover()); crErr != nil {
var rerr error err = crErr
if re, ok := r.(error); ok {
rerr = re
} else if re, ok := r.(string); ok {
rerr = clues.New(re)
} else {
rerr = clues.New(fmt.Sprintf("%v", r))
}
err = clues.Wrap(rerr, "panic recovery").
WithClues(ctx).
With("stacktrace", string(debug.Stack()))
logger.Ctx(ctx).
With("err", err).
Errorw("backup panic", clues.InErr(err).Slice()...)
} }
}() }()

View File

@ -8,6 +8,7 @@ import (
"github.com/google/uuid" "github.com/google/uuid"
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/alcionai/corso/src/internal/common/crash"
"github.com/alcionai/corso/src/internal/events" "github.com/alcionai/corso/src/internal/events"
"github.com/alcionai/corso/src/internal/kopia" "github.com/alcionai/corso/src/internal/kopia"
"github.com/alcionai/corso/src/internal/model" "github.com/alcionai/corso/src/internal/model"
@ -88,13 +89,19 @@ func Initialize(
acct account.Account, acct account.Account,
s storage.Storage, s storage.Storage,
opts control.Options, opts control.Options,
) (Repository, error) { ) (repo Repository, err error) {
ctx = clues.Add( ctx = clues.Add(
ctx, ctx,
"acct_provider", acct.Provider.String(), "acct_provider", acct.Provider.String(),
"acct_id", acct.ID(), // TODO: pii "acct_id", acct.ID(), // TODO: pii
"storage_provider", s.Provider.String()) "storage_provider", s.Provider.String())
defer func() {
if crErr := crash.Recovery(ctx, recover()); crErr != nil {
err = crErr
}
}()
kopiaRef := kopia.NewConn(s) kopiaRef := kopia.NewConn(s)
if err := kopiaRef.Initialize(ctx); err != nil { if err := kopiaRef.Initialize(ctx); err != nil {
// replace common internal errors so that sdk users can check results with errors.Is() // replace common internal errors so that sdk users can check results with errors.Is()
@ -156,13 +163,19 @@ func Connect(
acct account.Account, acct account.Account,
s storage.Storage, s storage.Storage,
opts control.Options, opts control.Options,
) (Repository, error) { ) (r Repository, err error) {
ctx = clues.Add( ctx = clues.Add(
ctx, ctx,
"acct_provider", acct.Provider.String(), "acct_provider", acct.Provider.String(),
"acct_id", acct.ID(), // TODO: pii "acct_id", acct.ID(), // TODO: pii
"storage_provider", s.Provider.String()) "storage_provider", s.Provider.String())
defer func() {
if crErr := crash.Recovery(ctx, recover()); crErr != nil {
err = crErr
}
}()
// Close/Reset the progress bar. This ensures callers don't have to worry about // Close/Reset the progress bar. This ensures callers don't have to worry about
// their output getting clobbered (#1720) // their output getting clobbered (#1720)
defer observe.Complete() defer observe.Complete()