From 897c0f8a074f73a03c8e0b3c33f739b48981adf2 Mon Sep 17 00:00:00 2001 From: Keepers Date: Fri, 21 Apr 2023 09:53:10 -0600 Subject: [PATCH] fix the file/line output on panic recovery (#3190) #### Does this PR need a docs update or release note? - [x] :no_entry: No #### Type of change - [x] :bug: Bugfix #### Test Plan - [x] :muscle: Manual - [x] :zap: Unit test --- src/internal/common/crash/crash.go | 54 ++++++++++++++++--------- src/internal/common/crash/crash_test.go | 2 +- src/internal/operations/backup.go | 2 +- src/internal/operations/restore.go | 2 +- src/pkg/repository/repository.go | 4 +- 5 files changed, 40 insertions(+), 24 deletions(-) diff --git a/src/internal/common/crash/crash.go b/src/internal/common/crash/crash.go index a35b93f87..05a5baf2d 100644 --- a/src/internal/common/crash/crash.go +++ b/src/internal/common/crash/crash.go @@ -5,6 +5,7 @@ import ( "fmt" "runtime" "runtime/debug" + "strings" "github.com/alcionai/clues" @@ -22,31 +23,46 @@ import ( // err = crErr // err needs to be a named return variable // } // }() -func Recovery(ctx context.Context, r any) error { +func Recovery(ctx context.Context, r any, namespace string) error { var ( err error inFile string + j int ) - 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(3) - if ok { - inFile = " in file: " + file - } - - err = clues.Wrap(err, "panic recovery"+inFile). - WithClues(ctx). - With("stacktrace", string(debug.Stack())) - logger.CtxErr(ctx, err).Error("backup panic") + if r == nil { + return 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)) + } + + for i := 1; i < 10; i++ { + _, file, line, ok := runtime.Caller(i) + if j > 0 { + if strings.Contains(file, "panic.go") { + j = 0 + } else { + inFile = fmt.Sprintf(": file %s - line %d", file, line) + break + } + } + + // skip the location where Recovery() gets called. + if j == 0 && ok && !strings.Contains(file, "panic.go") && !strings.Contains(file, "crash.go") { + j++ + } + } + + err = clues.Wrap(err, "panic recovery"+inFile). + WithClues(ctx). + With("stacktrace", string(debug.Stack())) + logger.CtxErr(ctx, err).Error(namespace + " panic") + return err } diff --git a/src/internal/common/crash/crash_test.go b/src/internal/common/crash/crash_test.go index 09a6559b9..375e6932e 100644 --- a/src/internal/common/crash/crash_test.go +++ b/src/internal/common/crash/crash_test.go @@ -52,7 +52,7 @@ func (suite *CrashTestDummySuite) TestRecovery() { ctx, flush := tester.NewContext() defer func() { - err := crash.Recovery(ctx, recover()) + err := crash.Recovery(ctx, recover(), "test") test.expect(t, err, clues.ToCore(err)) flush() }() diff --git a/src/internal/operations/backup.go b/src/internal/operations/backup.go index ac185d3d0..366aab60f 100644 --- a/src/internal/operations/backup.go +++ b/src/internal/operations/backup.go @@ -115,7 +115,7 @@ type backupStats struct { // Run begins a synchronous backup operation. func (op *BackupOperation) Run(ctx context.Context) (err error) { defer func() { - if crErr := crash.Recovery(ctx, recover()); crErr != nil { + if crErr := crash.Recovery(ctx, recover(), "backup"); crErr != nil { err = crErr } }() diff --git a/src/internal/operations/restore.go b/src/internal/operations/restore.go index f11b3e56b..b5d3caf64 100644 --- a/src/internal/operations/restore.go +++ b/src/internal/operations/restore.go @@ -104,7 +104,7 @@ type restoreStats struct { // Run begins a synchronous restore operation. func (op *RestoreOperation) Run(ctx context.Context) (restoreDetails *details.Details, err error) { defer func() { - if crErr := crash.Recovery(ctx, recover()); crErr != nil { + if crErr := crash.Recovery(ctx, recover(), "restore"); crErr != nil { err = crErr } }() diff --git a/src/pkg/repository/repository.go b/src/pkg/repository/repository.go index b18488d3f..f3db097c3 100644 --- a/src/pkg/repository/repository.go +++ b/src/pkg/repository/repository.go @@ -115,7 +115,7 @@ func Initialize( "storage_provider", s.Provider.String()) defer func() { - if crErr := crash.Recovery(ctx, recover()); crErr != nil { + if crErr := crash.Recovery(ctx, recover(), "repo init"); crErr != nil { err = crErr } }() @@ -189,7 +189,7 @@ func Connect( "storage_provider", s.Provider.String()) defer func() { - if crErr := crash.Recovery(ctx, recover()); crErr != nil { + if crErr := crash.Recovery(ctx, recover(), "repo connect"); crErr != nil { err = crErr } }()