amadan

carlos / platform Public

offline return: a panic in the detached index cleanup is contained (fix round 4, item 2a)

The return door's detached index cleanup recovers a panic and logs it
with the move id and the forward, as an HTTP handler's panic is
contained: the console stays up, the 202 stands, and the entry is left for
the sweep.

Witness: TestReturnIndexCleanupPanicIsContained (a store that panics on
the index read). RED: the panic took the test binary down
("panic: panicStore: control/offline-moves/@returns/uk/…").
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 1355a2d849bf6da0ecd69fea59c704ef44bbb2e8 parent 7a78272
2 files changed, +52 −0
  • internal/console/apiinstancemove_offline.go +7 −0
  • internal/console/offlinereturn_test.go +45 −0
diff --git a/internal/console/apiinstancemove_offline.go b/internal/console/apiinstancemove_offline.go
index 22c473b66..268e7fd2a 100644
--- a/internal/console/apiinstancemove_offline.go
+++ b/internal/console/apiinstancemove_offline.go
@@ -1017,6 +1017,13 @@ func (s *Server) offlineBegin(w http.ResponseWriter, r *http.Request, d offlineD
go func() {
defer s.detached.Done()
defer ccancel()
+ // Contained as an HTTP handler's panic is: the console stays up
+ // and the entry is left for the sweep (fix round 4).
+ defer func() {
+ if r := recover(); r != nil {
+ log.Printf("console: offline move %s (%s): panic in the return index cleanup of %s: %v (the sweep removes the entry)", id, host, sel.ReturnOf, r)
+ }
+ }()
s.offlineReturnIndexDone(cctx, sel.To, host, sel.ReturnOf)
}()
}
diff --git a/internal/console/offlinereturn_test.go b/internal/console/offlinereturn_test.go
index dc8b77388..419cb8abe 100644
--- a/internal/console/offlinereturn_test.go
+++ b/internal/console/offlinereturn_test.go
@@ -7,6 +7,7 @@ import (
"fmt"
"net/http"
"sort"
+ "strings"
"sync"
"testing"
"time"
@@ -791,3 +792,47 @@ func TestReturnSweepHourlyIsBounded(t *testing.T) {
}
})
}
+
+// panicStore panics on every read of key: a store bug in the cleanup.
+type panicStore struct {
+ release.Store
+ key string
+}
+
+func (p *panicStore) Get(ctx context.Context, key string) ([]byte, string, error) {
+ if key == p.key {
+ panic("panicStore: " + key)
+ }
+ return p.Store.Get(ctx, key)
+}
+
+// TestReturnIndexCleanupPanicIsContained (fix round 4, item 2a): a panic in
+// the detached index cleanup is recovered and logged with the move id, as
+// an HTTP handler's panic is contained: the console survives, the answer
+// stands, and the entry stays for the sweep.
+func TestReturnIndexCleanupPanicIsContained(t *testing.T) {
+ w := newReturnWorld(t, nil)
+ w.report("uk-box", "uk-box", "", w.request(), time.Now(), w.ready())
+ indexKey := moves.ReturnIndexKey("uk", offlineHost, returnFwd)
+ w.fx.srv.cfg.Releases = &panicStore{Store: w.fx.rel, key: indexKey}
+ var logs strings.Builder
+ restore := captureLogTo(&logs)
+ code, out := w.req(http.MethodPost, "/stage", `{"region":"uk","return_of":"`+returnFwd+`"}`)
+ ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+ defer cancel()
+ serr := w.fx.srv.Shutdown(ctx)
+ restore()
+ if code != http.StatusAccepted {
+ t.Fatalf("stage = %d %s", code, out)
+ }
+ if serr != nil {
+ t.Fatal(serr)
+ }
+ id := w.record().Move.ID
+ if !strings.Contains(logs.String(), "panic") || !strings.Contains(logs.String(), id) {
+ t.Fatalf("the cleanup's panic was not logged with the move id %s: %q", id, logs.String())
+ }
+ if _, _, err := w.fx.rel.Get(context.Background(), indexKey); err != nil {
+ t.Fatalf("the entry the sweep owns is gone: %v", err)
+ }
+}