amadan
offline return: a return request the console cannot read judges nothing (final review F5, console half)
readReturnReport skipped a request whose read failed, so with the newest request unreadable it could take an older request's report for the current one and stage the return. Every request is read or the stage and start doors answer the existing unreadable 502. Witness: TestReturnRequestReadFailureIsUnreadable (one request; two with the newer unreadable and the older answered; at stage and start).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+43
−2
internal/console/offlinereturn.go+7 −2internal/console/offlinereturn_test.go+36 −0
diff --git a/internal/console/offlinereturn.go b/internal/console/offlinereturn.go| index f4c6f19e0..0f822a996 100644 |
| --- a/internal/console/offlinereturn.go |
| +++ b/internal/console/offlinereturn.go |
| @@ -197,7 +197,8 @@ func (s *Server) offlineReturnIndex(ctx context.Context, code, host, returnOf, r |
| // box whose BoxID and InstanceID are the selected target's, and be one a |
| // forward source participant (or, for a forward without participants, a |
| // box in its source region) may write. Any of these failing cannot |
| -// determine the target's facts. |
| +// determine the target's facts; a request that cannot be read is the |
| +// unreadable 502 (the newest request is not known). |
| func (s *Server) readReturnReport(ctx context.Context, host, returnOf string, o *moves.OpenRecord, sel offlineSelection, now time.Time) (*moves.OfflineReport, *offlineRefusal) { |
| unknown := &offlineRefusal{status: http.StatusConflict, msg: couldNotDetermine(factTargetFacts, sel.Target.Label)} |
| prefix := moves.ReturnRequestPrefix(host, returnOf) |
| @@ -206,11 +207,15 @@ func (s *Server) readReturnReport(ctx context.Context, host, returnOf string, o |
| log.Printf("console: offline move %s: the return requests of %s: %v", host, returnOf, err) |
| return nil, &offlineRefusal{status: http.StatusBadGateway, msg: "could not read the move's reports. try again", transient: true} |
| } |
| + // Every request is read, or nothing is judged (final review F5): with |
| + // one unread, which request is newest is not known, and an older |
| + // request's report must never pass for the current one's. |
| var reqs []moves.ReturnRequest |
| for _, k := range keys { |
| raw, _, err := s.cfg.Releases.Get(ctx, k) |
| if err != nil { |
| - continue |
| + log.Printf("console: offline move %s: the return request %s: %v", host, k, err) |
| + return nil, &offlineRefusal{status: http.StatusBadGateway, msg: "could not read the move's reports. try again", transient: true} |
| } |
| if rq, err := moves.ParseReturnRequest(raw, host, returnOf, k); err == nil { |
| reqs = append(reqs, rq) |
diff --git a/internal/console/offlinereturn_test.go b/internal/console/offlinereturn_test.go| index 42fc24e3c..7426658da 100644 |
| --- a/internal/console/offlinereturn_test.go |
| +++ b/internal/console/offlinereturn_test.go |
| @@ -3,6 +3,7 @@ package console |
| import ( |
| "context" |
| "encoding/json" |
| + "errors" |
| "net/http" |
| "testing" |
| "time" |
| @@ -445,3 +446,38 @@ func TestReturnDiskUsesCurrentSourceSize(t *testing.T) { |
| } |
| } |
| } |
| + |
| +// TestReturnRequestReadFailureIsUnreadable (final review F5, console |
| +// half): a return request the console cannot read means the newest request |
| +// is not known, so the stage and start doors judge nothing from the |
| +// requests they could read: the unreadable 502, never an older request's |
| +// report taken for the current one. With one request, and with two whose |
| +// newer one fails to read while the older is answered. |
| +func TestReturnRequestReadFailureIsUnreadable(t *testing.T) { |
| + unreadable := "could not read the move's reports. try again" |
| + for name, two := range map[string]bool{"one request": false, "two requests, the newer unreadable": true} { |
| + for _, door := range []string{"/stage", ""} { |
| + t.Run(name+" at "+map[string]string{"/stage": "stage", "": "start"}[door], func(t *testing.T) { |
| + w := newReturnWorld(t, nil) |
| + older := w.request() |
| + w.report("uk-box", "uk-box", "", older, time.Now(), w.ready()) |
| + failing := older |
| + if two { |
| + failing = w.request() |
| + } |
| + lt := <Store{Store: w.fx.rel} |
| + lt.setHook(func(op, key string) error { |
| + if op == "get" && key == moves.ReturnRequestKey(offlineHost, returnFwd, failing) { |
| + return errors.New("the store is unreachable") |
| + } |
| + return nil |
| + }) |
| + w.fx.srv.cfg.Releases = lt |
| + w.refused(http.MethodPost, door, `{"region":"uk","return_of":"`+returnFwd+`"}`, http.StatusBadGateway, unreadable) |
| + if rec := w.record(); rec.Move != nil { |
| + t.Fatalf("a return judged from a partial read wrote %+v", rec.Move) |
| + } |
| + }) |
| + } |
| + } |
| +} |