Skip to content

Commit

Permalink
log: fix call depth for go1.12 panics
Browse files Browse the repository at this point in the history
This PR fixes the following test failure on Linux:

```
$ make test IGNORE_GOVERS=1 PKG=./pkg/util/log
--- FAIL: TestCrashReportingPacket (0.20s)
    --- FAIL: TestCrashReportingPacket/#00 (0.00s)
        crash_reporting_packet_test.go:155: expected crash_reporting_packet_test.go:85: boom, got testing.go:865: boom
    --- FAIL: TestCrashReportingPacket/#1 (0.00s)
        crash_reporting_packet_test.go:155: expected crash_reporting_packet_test.go:93: baam, got testing.go:865: baam
FAIL
```

Interestingly this is not quite the same failure noted in cockroachdb#35792, maybe that's
MacOS specific?

Release note: None
  • Loading branch information
ajwerner committed Mar 26, 2019
1 parent 9fff28a commit 50140b0
Showing 1 changed file with 15 additions and 14 deletions.
29 changes: 15 additions & 14 deletions pkg/util/log/crash_reporting.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,18 +86,25 @@ var (
ReportSensitiveDetails = envutil.EnvOrDefaultBool("COCKROACH_REPORT_SENSITIVE_DETAILS", false)
)

var depthForRecoverAndReportPanic = func() int {
// The call stack here is usually:
// - ReportPanic
// - RecoverAndReport
// - panic.go // not present in go1.12
// - panic()
// so ReportPanic should pop four frames.
if ver := runtime.Version(); strings.HasPrefix(ver, "go1.12") {
return 3
}
return 4
}()

// RecoverAndReportPanic can be invoked on goroutines that run with
// stderr redirected to logs to ensure the user gets informed on the
// real stderr a panic has occurred.
func RecoverAndReportPanic(ctx context.Context, sv *settings.Values) {
if r := recover(); r != nil {
// The call stack here is usually:
// - ReportPanic
// - RecoverAndReport
// - panic.go
// - panic()
// so ReportPanic should pop four frames.
ReportPanic(ctx, sv, r, 4)
ReportPanic(ctx, sv, r, depthForRecoverAndReportPanic)
panic(r)
}
}
Expand All @@ -106,13 +113,7 @@ func RecoverAndReportPanic(ctx context.Context, sv *settings.Values) {
// does not re-panic in Release builds.
func RecoverAndReportNonfatalPanic(ctx context.Context, sv *settings.Values) {
if r := recover(); r != nil {
// The call stack here is usually:
// - ReportPanic
// - RecoverAndReport
// - panic.go
// - panic()
// so ReportPanic should pop four frames.
ReportPanic(ctx, sv, r, 4)
ReportPanic(ctx, sv, r, depthForRecoverAndReportPanic)
if !build.IsRelease() || PanicOnAssertions.Get(sv) {
panic(r)
}
Expand Down

0 comments on commit 50140b0

Please sign in to comment.