From 86450838feb7e73cc33c84c94e73acb3df840681 Mon Sep 17 00:00:00 2001 From: Steve Willoughby Date: Thu, 5 Sep 2024 12:16:34 -0700 Subject: [PATCH] Fix handling of panic(nil) --- v3/newrelic/internal_txn.go | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/v3/newrelic/internal_txn.go b/v3/newrelic/internal_txn.go index 32060485b..545083ca9 100644 --- a/v3/newrelic/internal_txn.go +++ b/v3/newrelic/internal_txn.go @@ -10,6 +10,7 @@ import ( "net/http" "net/url" "reflect" + "runtime" "runtime/debug" "sync" "time" @@ -445,11 +446,16 @@ func (thd *thread) End(recovered interface{}) error { txn.finished = true - if nil != recovered { - e := txnErrorFromPanic(time.Now(), recovered) - e.Stack = getStackTrace() - thd.noticeErrorInternal(e, nil, false) - log.Println(string(debug.Stack())) + // It used to be the case that panic(nil) would cause recover() to return nil, + // which we test for here. However, that is no longer the case, hence the extra + // check at this point to stop panic(nil) from propagating here. (as of Go 1.21) + if recovered != nil { + if _, isNilPanic := recovered.(*runtime.PanicNilError); !isNilPanic { + e := txnErrorFromPanic(time.Now(), recovered) + e.Stack = getStackTrace() + thd.noticeErrorInternal(e, nil, false) + log.Println(string(debug.Stack())) + } } txn.markEnd(time.Now(), thd.thread) @@ -542,9 +548,12 @@ func (thd *thread) End(recovered interface{}) error { } // Note that if a consumer uses `panic(nil)`, the panic will not - // propagate. - if nil != recovered { - panic(recovered) + // propagate. Update: well, not anymore. Go now returns an actual + // non-nil value in this case. + if recovered != nil { + if _, isNilPanic := recovered.(*runtime.PanicNilError); !isNilPanic { + panic(recovered) + } } return nil