You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by ha...@apache.org on 2026/08/09 07:28:11 UTC
(skywalking-banyandb) branch panic-metric-grpc created (now 2c362da58)
This is an automated email from the ASF dual-hosted git repository.
hanahmily pushed a change to branch panic-metric-grpc
in repository https://gitbox.apache.org/repos/asf/skywalking-banyandb.git
at 2c362da58 fix(observability): count panics recovered by the gRPC interceptors
This branch includes the following new commits:
new 2c362da58 fix(observability): count panics recovered by the gRPC interceptors
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(skywalking-banyandb) 01/01: fix(observability): count panics recovered by the gRPC interceptors
Posted by ha...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
hanahmily pushed a commit to branch panic-metric-grpc
in repository https://gitbox.apache.org/repos/asf/skywalking-banyandb.git
commit 2c362da585097d1778df9950a9fca04c9b511ea9
Author: Gao Hongtao <ha...@gmail.com>
AuthorDate: Sun Aug 9 07:27:59 2026 +0000
fix(observability): count panics recovered by the gRPC interceptors
Each gRPC server hand-rolled a recovery handler that only logged the
panic and returned codes.Internal, so panics on a request path reached
none of panicdiag's signals: not banyandb_panic_total, not the crash
reporters, not the artifact writer. During a production incident one
data node logged 737 recovered request-path panics while
banyandb_panic_total stayed at 4 -- the only goroutine panics, which
route through run.Go. The panics worth alerting on were the ones
nothing counted.
Extract the signal-emitting body of WithRecovery so a panic recovered
elsewhere can reuse it, expose it as RecoverExternal, and replace the
four copied handlers with panicdiag.GRPCRecoveryHandler labelled per
server. The stack is captured inside the interceptor's recovering
defer, so artifacts keep pointing at the panic site.
---
CHANGES.md | 1 +
banyand/liaison/grpc/server.go | 11 +-
banyand/metadata/schema/schemaserver/service.go | 13 +-
banyand/property/gossip/service.go | 13 +-
banyand/queue/sub/server.go | 13 +-
pkg/panicdiag/grpcrecovery.go | 47 +++++
pkg/panicdiag/grpcrecovery_test.go | 153 ++++++++++++++++
pkg/panicdiag/recovery.go | 222 ++++++++++++++----------
8 files changed, 340 insertions(+), 133 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 8bd6049e1..c4e7285c7 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -156,6 +156,7 @@ Release Notes.
- Enable periodic health checks on the queue client (`--<prefix>-client-health-check-interval`, default `10s`), evicting dead data nodes proactively.
- Build the plugin `.so` with `-tags slim` to match the host binary's ABI. Without this, `pkg/pool` has build-tag-dependent code (`tracker.go` vs `tracker_stub.go`) and the plugin-sidecar E2E failed with "plugin was built with a different version of package `pkg/pool`".
- Make the trace merge loop survive a persistently failing merge instead of destroying the node. A production cluster wedged when a few unreadable parts made every merge selection fail: each failed attempt leaked its partially-written output directory (the block writer creates it before the merge runs), which exhausted the volume's inodes at 8% byte usage; the resulting disk-full panic then killed the merge lane workers, which are recovered but never respawned, so the dispatcher blocked [...]
+- Report panics recovered by the gRPC servers through `panicdiag`, so they reach `banyandb_panic_total{component}`, the crash reporters and the artifact writer. Each server (`queue/sub`, `liaison/grpc`, `property/gossip`, `metadata/schema/schemaserver`) hand-rolled a recovery handler that only logged and returned `codes.Internal`, bypassing every panicdiag signal: during a production incident one data node logged 737 recovered request-path panics while `banyandb_panic_total` stayed at 4 [...]
### Document
diff --git a/banyand/liaison/grpc/server.go b/banyand/liaison/grpc/server.go
index cc5c928cc..f4cef153a 100644
--- a/banyand/liaison/grpc/server.go
+++ b/banyand/liaison/grpc/server.go
@@ -22,7 +22,6 @@ import (
"context"
"net"
"path/filepath"
- "runtime/debug"
"strconv"
"strings"
"time"
@@ -551,15 +550,9 @@ func (s *server) Serve() run.StopNotify {
}
s.log.Info().Str("authConfigFile", s.authConfigFile).Msg("Starting auth config file monitoring")
}
- grpcPanicRecoveryHandler := func(ctx context.Context, p any) (err error) {
- breadcrumbs := panicdiag.BreadcrumbsFromContext(ctx)
- stages := make([]string, len(breadcrumbs))
- for idx, bc := range breadcrumbs {
- stages[idx] = bc.Stage
- }
- s.log.Error().Interface("panic", p).Str("stack", string(debug.Stack())).Strs("breadcrumbs", stages).Msg("recovered from panic")
+ grpcPanicRecoveryHandler := func(ctx context.Context, p any) error {
s.metrics.totalPanic.Inc(1)
- return status.Errorf(codes.Internal, "%s", p)
+ return panicdiag.GRPCRecoveryHandler(s.log, "grpc.liaison")(ctx, p)
}
streamChain := []grpclib.StreamServerInterceptor{
diff --git a/banyand/metadata/schema/schemaserver/service.go b/banyand/metadata/schema/schemaserver/service.go
index 4024077ff..bd2180e7f 100644
--- a/banyand/metadata/schema/schemaserver/service.go
+++ b/banyand/metadata/schema/schemaserver/service.go
@@ -23,7 +23,6 @@ import (
"fmt"
"net"
"path/filepath"
- "runtime/debug"
"strconv"
"sync"
"time"
@@ -32,11 +31,9 @@ import (
grpc_validator "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/validator"
"github.com/pkg/errors"
grpclib "google.golang.org/grpc"
- "google.golang.org/grpc/codes"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/health"
"google.golang.org/grpc/health/grpc_health_v1"
- "google.golang.org/grpc/status"
"github.com/apache/skywalking-banyandb/api/common"
databasev1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/database/v1"
@@ -300,15 +297,7 @@ func (s *server) PreRun(ctx context.Context) error {
opts = []grpclib.ServerOption{grpclib.Creds(creds)}
}
}
- grpcPanicRecoveryHandler := func(ctx context.Context, p any) (err error) {
- breadcrumbs := panicdiag.BreadcrumbsFromContext(ctx)
- stages := make([]string, len(breadcrumbs))
- for idx, bc := range breadcrumbs {
- stages[idx] = bc.Stage
- }
- s.l.Error().Interface("panic", p).Str("stack", string(debug.Stack())).Strs("breadcrumbs", stages).Msg("recovered from panic")
- return status.Errorf(codes.Internal, "%s", p)
- }
+ grpcPanicRecoveryHandler := panicdiag.GRPCRecoveryHandler(s.l, "grpc.schema-server")
streamChain := []grpclib.StreamServerInterceptor{
panicdiag.BreadcrumbStreamInterceptor(),
recovery.StreamServerInterceptor(recovery.WithRecoveryHandlerContext(grpcPanicRecoveryHandler)),
diff --git a/banyand/property/gossip/service.go b/banyand/property/gossip/service.go
index b6706a93f..80477a0f7 100644
--- a/banyand/property/gossip/service.go
+++ b/banyand/property/gossip/service.go
@@ -21,7 +21,6 @@ import (
"context"
"fmt"
"net"
- "runtime/debug"
"strconv"
"sync"
"time"
@@ -30,9 +29,7 @@ import (
grpc_validator "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/validator"
"github.com/pkg/errors"
grpclib "google.golang.org/grpc"
- "google.golang.org/grpc/codes"
"google.golang.org/grpc/credentials"
- "google.golang.org/grpc/status"
"github.com/apache/skywalking-banyandb/api/common"
databasev1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/database/v1"
@@ -238,15 +235,7 @@ func (s *service) Serve(closer *run.Closer) {
if s.tls {
opts = []grpclib.ServerOption{grpclib.Creds(s.creds)}
}
- grpcPanicRecoveryHandler := func(ctx context.Context, p any) (err error) {
- breadcrumbs := panicdiag.BreadcrumbsFromContext(ctx)
- stages := make([]string, len(breadcrumbs))
- for idx, bc := range breadcrumbs {
- stages[idx] = bc.Stage
- }
- s.log.Error().Interface("panic", p).Str("stack", string(debug.Stack())).Strs("breadcrumbs", stages).Msg("recovered from panic")
- return status.Errorf(codes.Internal, "%s", p)
- }
+ grpcPanicRecoveryHandler := panicdiag.GRPCRecoveryHandler(s.log, "grpc.property-gossip")
streamChain := []grpclib.StreamServerInterceptor{
panicdiag.BreadcrumbStreamInterceptor(),
recovery.StreamServerInterceptor(recovery.WithRecoveryHandlerContext(grpcPanicRecoveryHandler)),
diff --git a/banyand/queue/sub/server.go b/banyand/queue/sub/server.go
index d79494c5a..97ecab701 100644
--- a/banyand/queue/sub/server.go
+++ b/banyand/queue/sub/server.go
@@ -21,7 +21,6 @@ import (
"context"
"net"
"net/http"
- "runtime/debug"
"strconv"
"sync"
"time"
@@ -32,12 +31,10 @@ import (
"github.com/grpc-ecosystem/grpc-gateway/v2/runtime"
"github.com/pkg/errors"
grpclib "google.golang.org/grpc"
- "google.golang.org/grpc/codes"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/health"
"google.golang.org/grpc/health/grpc_health_v1"
- "google.golang.org/grpc/status"
"github.com/apache/skywalking-banyandb/api/common"
clusterv1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/cluster/v1"
@@ -265,15 +262,7 @@ func (s *server) Serve() run.StopNotify {
opts = []grpclib.ServerOption{grpclib.Creds(creds)}
}
}
- grpcPanicRecoveryHandler := func(ctx context.Context, p any) (err error) {
- breadcrumbs := panicdiag.BreadcrumbsFromContext(ctx)
- stages := make([]string, len(breadcrumbs))
- for idx, bc := range breadcrumbs {
- stages[idx] = bc.Stage
- }
- s.log.Error().Interface("panic", p).Str("stack", string(debug.Stack())).Strs("breadcrumbs", stages).Msg("recovered from panic")
- return status.Errorf(codes.Internal, "%s", p)
- }
+ grpcPanicRecoveryHandler := panicdiag.GRPCRecoveryHandler(s.log, "grpc.queue-sub")
streamChain := []grpclib.StreamServerInterceptor{
panicdiag.BreadcrumbStreamInterceptor(),
diff --git a/pkg/panicdiag/grpcrecovery.go b/pkg/panicdiag/grpcrecovery.go
new file mode 100644
index 000000000..a258ca90e
--- /dev/null
+++ b/pkg/panicdiag/grpcrecovery.go
@@ -0,0 +1,47 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you under
+// the Apache License, Version 2.0 (the "License"); you may
+// not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package panicdiag
+
+import (
+ "context"
+ "runtime/debug"
+
+ "google.golang.org/grpc/codes"
+ "google.golang.org/grpc/status"
+
+ "github.com/apache/skywalking-banyandb/pkg/logger"
+)
+
+// GRPCRecoveryHandler returns a handler for the gRPC recovery interceptors that reports
+// the panic through panicdiag before converting it to a codes.Internal error. Servers
+// previously hand-rolled a handler that only logged, which left every panic recovered on
+// a request path out of banyandb_panic_total, out of the crash reporters and without an
+// artifact -- the panics most likely to matter were the ones nothing counted.
+//
+// The returned function is deliberately untyped so this package does not depend on the
+// gRPC middleware; it satisfies recovery.RecoveryHandlerFuncContext directly:
+//
+// recovery.WithRecoveryHandlerContext(panicdiag.GRPCRecoveryHandler(log, "grpc.liaison"))
+func GRPCRecoveryHandler(log *logger.Logger, component string) func(context.Context, any) error {
+ return func(ctx context.Context, panicValue any) error {
+ // Captured here, inside the interceptor's recovering defer, so the stack still
+ // describes the panic site rather than the recovery machinery.
+ RecoverExternal(ctx, RecoveryOptions{Logger: log, Component: component}, nil, panicValue, debug.Stack())
+ return status.Errorf(codes.Internal, "%s", panicValue)
+ }
+}
diff --git a/pkg/panicdiag/grpcrecovery_test.go b/pkg/panicdiag/grpcrecovery_test.go
new file mode 100644
index 000000000..bfefe25c5
--- /dev/null
+++ b/pkg/panicdiag/grpcrecovery_test.go
@@ -0,0 +1,153 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you under
+// the Apache License, Version 2.0 (the "License"); you may
+// not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package panicdiag
+
+import (
+ "context"
+ "sync"
+ "testing"
+
+ "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/recovery"
+ "github.com/stretchr/testify/require"
+ "google.golang.org/grpc/codes"
+ "google.golang.org/grpc/status"
+
+ "github.com/apache/skywalking-banyandb/pkg/meter"
+)
+
+type recordingReporter struct {
+ results []RecoveryResult
+ mu sync.Mutex
+}
+
+func (r *recordingReporter) report() Reporter {
+ return func(_ context.Context, result RecoveryResult) {
+ r.mu.Lock()
+ defer r.mu.Unlock()
+ r.results = append(r.results, result)
+ }
+}
+
+func (r *recordingReporter) recorded() []RecoveryResult {
+ r.mu.Lock()
+ defer r.mu.Unlock()
+ return append([]RecoveryResult(nil), r.results...)
+}
+
+// setDefaultPanicCounterForTest installs a counter for the duration of one test and
+// restores the previous registration, so tests never leak process-wide state.
+func setDefaultPanicCounterForTest(t *testing.T, counter meter.Counter) {
+ t.Helper()
+ previous := defaultPanicCounterPtr.Load()
+ SetDefaultPanicCounter(counter)
+ t.Cleanup(func() { defaultPanicCounterPtr.Store(previous) })
+}
+
+func setDefaultReporterForTest(t *testing.T, reporter Reporter) {
+ t.Helper()
+ previous := defaultReporterPtr.Load()
+ SetDefaultReporter(reporter)
+ t.Cleanup(func() { defaultReporterPtr.Store(previous) })
+}
+
+// TestGRPCRecoveryHandlerCountsPanic is the regression guard for the gap this handler
+// closes: a panic recovered by the gRPC interceptor used to be logged only, so it never
+// reached banyandb_panic_total or the crash reporters.
+func TestGRPCRecoveryHandlerCountsPanic(t *testing.T) {
+ counter := &fakeCounter{}
+ setDefaultPanicCounterForTest(t, counter)
+ reporter := &recordingReporter{}
+ setDefaultReporterForTest(t, reporter.report())
+
+ handler := GRPCRecoveryHandler(nil, "grpc.test")
+ err := handler(context.Background(), "boom")
+
+ require.Error(t, err)
+ require.Equal(t, codes.Internal, status.Code(err))
+ require.Contains(t, err.Error(), "boom")
+
+ require.Equal(t, 1, counter.calls, "the recovered panic must reach the panic counter")
+ require.Equal(t, []string{"grpc.test"}, counter.lastLabels, "the component label must identify the server")
+ recorded := reporter.recorded()
+ require.Len(t, recorded, 1, "the recovered panic must reach the crash reporters")
+ require.Equal(t, "boom", recorded[0].Record.PanicValue)
+}
+
+// TestGRPCRecoveryHandlerKeepsPanicSiteStack asserts the recorded stack describes where
+// the panic happened rather than the recovery machinery: the handler is invoked from the
+// interceptor's recovering defer, so the panicking frame is still on the stack.
+func TestGRPCRecoveryHandlerKeepsPanicSiteStack(t *testing.T) {
+ counter := &fakeCounter{}
+ setDefaultPanicCounterForTest(t, counter)
+ reporter := &recordingReporter{}
+ setDefaultReporterForTest(t, reporter.report())
+
+ handler := GRPCRecoveryHandler(nil, "grpc.test")
+ err := callThroughRecoveringDefer(handler)
+
+ require.Error(t, err)
+ recorded := reporter.recorded()
+ require.Len(t, recorded, 1)
+ require.Contains(t, recorded[0].Record.GoroutineStack, "panicInNestedFrame",
+ "the artifact stack must still contain the panicking frame")
+}
+
+// callThroughRecoveringDefer mimics the grpc-middleware recovery interceptor: recover in
+// a defer and hand the value to the handler while the panicking frames are still live.
+func callThroughRecoveringDefer(handler func(context.Context, any) error) (err error) {
+ defer func() {
+ if p := recover(); p != nil {
+ err = handler(context.Background(), p)
+ }
+ }()
+ panicInNestedFrame()
+ return nil
+}
+
+func panicInNestedFrame() {
+ panic("nested boom")
+}
+
+// TestRecoverExternalSuppressesHookPanic asserts a broken hook cannot turn panicdiag's
+// own reporting into a second panic escaping into the caller's recovery path.
+func TestRecoverExternalSuppressesHookPanic(t *testing.T) {
+ setDefaultReporterForTest(t, func(_ context.Context, _ RecoveryResult) { panic("reporter is broken") })
+ require.NotPanics(t, func() {
+ RecoverExternal(context.Background(), RecoveryOptions{Component: "grpc.test"}, nil, "boom", nil)
+ })
+}
+
+// TestRecoverExternalIgnoresNilPanic guards the no-panic path: handlers must be able to
+// call it unconditionally without recording phantom panics.
+func TestRecoverExternalIgnoresNilPanic(t *testing.T) {
+ counter := &fakeCounter{}
+ setDefaultPanicCounterForTest(t, counter)
+ result := RecoverExternal(context.Background(), RecoveryOptions{Component: "grpc.test"}, nil, nil, nil)
+ require.Nil(t, result.Record)
+ require.Zero(t, counter.calls)
+}
+
+// TestGRPCRecoveryHandlerSatisfiesInterceptorType pins the deliberately untyped return
+// to the middleware's expectation. The package avoids importing the middleware so the
+// dependency stays at the wiring sites; this assignment keeps that decision honest by
+// failing here, once, rather than at four servers if the signature ever drifts.
+func TestGRPCRecoveryHandlerSatisfiesInterceptorType(t *testing.T) {
+ var handler recovery.RecoveryHandlerFuncContext = GRPCRecoveryHandler(nil, "grpc.test")
+ require.NotNil(t, handler)
+ _ = recovery.WithRecoveryHandlerContext(handler)
+}
diff --git a/pkg/panicdiag/recovery.go b/pkg/panicdiag/recovery.go
index 538bea37e..6ca8a95b0 100644
--- a/pkg/panicdiag/recovery.go
+++ b/pkg/panicdiag/recovery.go
@@ -65,98 +65,94 @@ func WithRecovery(ctx context.Context, opts RecoveryOptions, reporter Reporter,
if panicValue == nil {
return
}
+ result, artifactDone := emitRecovery(ctx, log, opts, reporter, panicValue, debug.Stack())
+ outcome.Panicked = true
+ outcome.PanicValue = panicValue
+ outcome.Result = result
+ outcome.ArtifactDone = artifactDone
+ }()
- record := &PanicRecord{
- OccurredAt: time.Now().UTC(),
- Component: opts.Component,
- PanicValue: fmt.Sprint(panicValue),
- Recovered: true,
- GoroutineStack: string(debug.Stack()),
- Breadcrumbs: BreadcrumbsFromContext(ctx),
- ProcessMetadata: cloneStringMap(opts.ProcessMetadata),
- }
+ fn(&ctx)
+ return
+}
- incPanicCounter(log, opts.Counter, opts.Component)
+// emitRecovery fires every signal panicdiag owes a recovered panic: the panic counter,
+// the structured log, the reporters, the abort hook and the artifact. It is shared by
+// WithRecovery and by RecoverExternal so a panic recovered elsewhere (a gRPC recovery
+// interceptor, for example) is observed identically to one recovered here. stack is the
+// caller's captured goroutine stack: callers recover during the panic's unwind and so
+// still hold the stack of the panic site, which is worth far more than a synthetic one
+// captured after the fact.
+func emitRecovery(ctx context.Context, log *logger.Logger, opts RecoveryOptions, reporter Reporter,
+ panicValue any, stack []byte,
+) (RecoveryResult, chan struct{}) {
+ record := &PanicRecord{
+ OccurredAt: time.Now().UTC(),
+ Component: opts.Component,
+ PanicValue: fmt.Sprint(panicValue),
+ Recovered: true,
+ GoroutineStack: string(stack),
+ Breadcrumbs: BreadcrumbsFromContext(ctx),
+ ProcessMetadata: cloneStringMap(opts.ProcessMetadata),
+ }
- artifactRoot := opts.ArtifactRoot
- if artifactRoot == "" {
- artifactRoot = DefaultArtifactRoot()
- }
+ incPanicCounter(log, opts.Counter, opts.Component)
- var (
- stateDump any
- stateErr error
- hasStateDump bool
- )
- if opts.StateDumper != nil && artifactRoot != "" {
- hasStateDump = true
- safeCall(log, "state-dumper", func() {
- stateDump, stateErr = opts.StateDumper.DumpState(ctx)
- })
- }
+ artifactRoot := opts.ArtifactRoot
+ if artifactRoot == "" {
+ artifactRoot = DefaultArtifactRoot()
+ }
- var artifactDir string
- if artifactRoot != "" {
- artifactDir = NewArtifactWriter(artifactRoot).ArtifactDirPath(record)
- }
+ var (
+ stateDump any
+ stateErr error
+ hasStateDump bool
+ )
+ if opts.StateDumper != nil && artifactRoot != "" {
+ hasStateDump = true
+ safeCall(log, "state-dumper", func() {
+ stateDump, stateErr = opts.StateDumper.DumpState(ctx)
+ })
+ }
- stages := make([]string, len(record.Breadcrumbs))
- for idx, bc := range record.Breadcrumbs {
- stages[idx] = bc.Stage
- }
- log.Error().
- Str("component", opts.Component).
- Str("panic", record.PanicValue).
- Strs("breadcrumbs", stages).
- Str("stack", record.GoroutineStack).
- Str("artifact_dir", artifactDir).
- Msg("recovered panic")
-
- recoveryResult := RecoveryResult{
- Record: record,
- ArtifactDir: artifactDir,
- }
- outcome.Panicked = true
- outcome.PanicValue = panicValue
- outcome.Result = recoveryResult
- callReporter(ctx, log, reporter, recoveryResult)
- callDefaultAbort(ctx, log, recoveryResult)
+ var artifactDir string
+ if artifactRoot != "" {
+ artifactDir = NewArtifactWriter(artifactRoot).ArtifactDirPath(record)
+ }
- // Signal artifact completion for callers such as GoOrDie.
- artifactDone := make(chan struct{})
- outcome.ArtifactDone = artifactDone
+ stages := make([]string, len(record.Breadcrumbs))
+ for idx, bc := range record.Breadcrumbs {
+ stages[idx] = bc.Stage
+ }
+ log.Error().
+ Str("component", opts.Component).
+ Str("panic", record.PanicValue).
+ Strs("breadcrumbs", stages).
+ Str("stack", record.GoroutineStack).
+ Str("artifact_dir", artifactDir).
+ Msg("recovered panic")
+
+ recoveryResult := RecoveryResult{
+ Record: record,
+ ArtifactDir: artifactDir,
+ }
+ callReporter(ctx, log, reporter, recoveryResult)
+ callDefaultAbort(ctx, log, recoveryResult)
- if artifactRoot == "" {
- // No artifact will be written.
- close(artifactDone)
- return
- }
- if sink := defaultArtifactSinkPtr.Load(); sink != nil {
- // Clone so the async worker never mutates caller-visible state.
- sinkRecord := *record
- sinkRecord.StateDump = nil
- sinkJob := ArtifactJob{
- Record: &sinkRecord,
- RootDir: artifactRoot,
- StateDump: stateDump,
- StateErr: stateErr,
- StateLimit: opts.StateLimitBytes,
- Logger: log,
- HasStateDump: hasStateDump,
- Done: artifactDone,
- }
- if !sink.Submit(sinkJob) {
- log.Warn().Str("component", opts.Component).
- Int("queue_depth", sink.QueueDepth()).
- Msg("artifact sink saturated; dropping artifact")
- // Submit did not take ownership of Done.
- close(artifactDone)
- }
- return
- }
- // Sync fallback preserves caller-visible StateDump status.
- writeArtifactSync(ArtifactJob{
- Record: record,
+ // Signal artifact completion for callers such as GoOrDie.
+ artifactDone := make(chan struct{})
+
+ if artifactRoot == "" {
+ // No artifact will be written.
+ close(artifactDone)
+ return recoveryResult, artifactDone
+ }
+ if sink := defaultArtifactSinkPtr.Load(); sink != nil {
+ // Clone so the async worker never mutates caller-visible state.
+ sinkRecord := *record
+ sinkRecord.StateDump = nil
+ sinkJob := ArtifactJob{
+ Record: &sinkRecord,
RootDir: artifactRoot,
StateDump: stateDump,
StateErr: stateErr,
@@ -164,11 +160,61 @@ func WithRecovery(ctx context.Context, opts RecoveryOptions, reporter Reporter,
Logger: log,
HasStateDump: hasStateDump,
Done: artifactDone,
- })
- }()
+ }
+ if !sink.Submit(sinkJob) {
+ log.Warn().Str("component", opts.Component).
+ Int("queue_depth", sink.QueueDepth()).
+ Msg("artifact sink saturated; dropping artifact")
+ // Submit did not take ownership of Done.
+ close(artifactDone)
+ }
+ return recoveryResult, artifactDone
+ }
+ // Sync fallback preserves caller-visible StateDump status.
+ writeArtifactSync(ArtifactJob{
+ Record: record,
+ RootDir: artifactRoot,
+ StateDump: stateDump,
+ StateErr: stateErr,
+ StateLimit: opts.StateLimitBytes,
+ Logger: log,
+ HasStateDump: hasStateDump,
+ Done: artifactDone,
+ })
+ return recoveryResult, artifactDone
+}
- fn(&ctx)
- return
+// RecoverExternal fires panicdiag's standard signals for a panic that the caller has
+// already recovered itself, so recovery paths outside WithRecovery -- notably the gRPC
+// servers' recovery interceptors -- are not invisible to the panic counter, the crash
+// reporters and the artifact writer. Call it from inside the recovering defer (or a
+// handler it invokes) so stack still describes the panic site. It never panics: every
+// hook it runs is isolated, and a failure inside panicdiag itself falls back to stderr.
+//
+//nolint:contextcheck // a nil caller context has no parent to inherit from; background is the safe root fallback
+func RecoverExternal(ctx context.Context, opts RecoveryOptions, reporter Reporter, panicValue any, stack []byte) (result RecoveryResult) {
+ if panicValue == nil {
+ return result
+ }
+ if ctx == nil {
+ ctx = context.Background()
+ }
+ log := opts.Logger
+ if log == nil {
+ log = logger.GetLogger("panicdiag")
+ }
+ defer func() {
+ // Mirrors WithRecovery's second-tier net: a panic raised while emitting the
+ // signals must never escape into the caller's own recovery path.
+ if rec := recover(); rec != nil {
+ fmt.Fprintf(os.Stderr, "panicdiag: panic in external recovery (suppressed): %v\n%s\n", rec, debug.Stack())
+ }
+ }()
+ if len(stack) == 0 {
+ stack = debug.Stack()
+ }
+ result, _ = emitRecovery(ctx, log, opts, reporter, panicValue, stack)
+ return result
}
// GoWithRecovery starts fn in a goroutine protected by WithRecovery. Use