diff --git a/CHANGELOG.md b/CHANGELOG.md index c125bf9..88aef13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ below says plainly whether an upgrade can break a caller. ## [Unreleased] +No signature changes. An upgrade cannot break a caller. + +### Fixed + +- A worker registered with `Go` that panics is now a worker that failed: the + panic becomes its error, `Shutdown` receives it, `Run` returns it, `OnStop` + runs and the stop event carries it, as for a panicking hook. It was the one + user function not called through the recovering wrapper, so its panic took + the process down with no release and no event. + ## [0.16.0] - 2026-09-10 `Binding.Worker` is now `Binding.Go`, after `errgroup.Group.Go` and diff --git a/CLAUDE.md b/CLAUDE.md index dce86c2..4c9a7c5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -421,9 +421,11 @@ binding and cannot protect the inner scope. given gets an error naming `Shutdown`. A hook that passes a context of its own is invisible and waits, which is why the fallback still has to be a bounded wait rather than a promise. -- Every user hook is called through `callHook`, which turns a panic into that - hook's error, and every step is reported through `state.report`, so a - hook that panicked is observed like one that failed. A cancelled +- Every user function is called through `callHook`, the `Go` worker + included, which turns a panic into that hook's error, and every step is + reported through `state.report`, so a hook that panicked is observed like + one that failed. The worker was the one exception until 0.16.1, and its + panic took the process down with no `OnStop` and no event. A cancelled worker's return is dropped only when it says nothing beyond `context.Canceled` (`onlyCancellation` walks the error tree); `errors.Is` matched `errors.Join(ctx.Err(), failure)` and dropped the failure with it diff --git a/lifecycle.go b/lifecycle.go index 8075c61..ce0f82f 100644 --- a/lifecycle.go +++ b/lifecycle.go @@ -226,7 +226,11 @@ func (in *instance) start(ctx context.Context, owner *state) error { hctx := inHook(rctx, owner) go func() { defer close(in.runDone) - err := b.worker(hctx, in.value) + // Through callHook like every other user function: a worker that + // panics is a worker that failed, reported by Stop and received + // by Run, rather than a crash of the process with no OnStop run + // and no event emitted. + err := callHook(b.worker, hctx, in.value) if err == nil { return } diff --git a/worker_test.go b/worker_test.go index 602d428..0a0555e 100644 --- a/worker_test.go +++ b/worker_test.go @@ -262,3 +262,39 @@ func TestWorkerFailureJoinedWithCancellationIsReported(t *testing.T) { t.Fatalf("a bare cancellation is not a failure: %v", err) } } + +// A worker that panics is a worker that failed: the panic becomes its error, +// Shutdown receives it, Run returns it, and OnStop still runs. Called directly +// rather than through callHook, the panic took the process down with no +// OnStop and no event. Found by the supervision-tree question of 2026-09-12 +// and checked against c44198f. +func TestPanickingWorkerIsAFailure(t *testing.T) { + var stops atomic.Int32 + var events []di.Event + s := di.New() + s.Observe(func(ev di.Event) { + if ev.Kind == di.EventStop { + events = append(events, ev) + } + }) + s.Value(&Worker{}).Eager(). + Go(func(context.Context, *Worker) error { panic("consumer exploded") }). + OnStop(func(context.Context, *Worker) error { stops.Add(1); return nil }) + done := make(chan error, 1) + go func() { done <- s.Run(context.Background()) }() + var err error + select { + case err = <-done: + case <-time.After(5 * time.Second): + t.Fatal("a panicking worker did not stop the application") + } + if err == nil || !strings.Contains(err.Error(), "consumer exploded") { + t.Fatalf("Run must report the panic as the worker's failure, got %v", err) + } + if stops.Load() != 1 { + t.Fatalf("OnStop ran %d times, want 1", stops.Load()) + } + if len(events) != 1 || events[0].Err == nil { + t.Fatalf("stop event %+v, want one carrying the failure", events) + } +}