diff --git a/internal/storage/orders.go b/internal/storage/orders.go index 1f2f670..d0c0b27 100644 --- a/internal/storage/orders.go +++ b/internal/storage/orders.go @@ -148,6 +148,12 @@ func validAdvVerdict(status string) bool { var ErrNoSuchAdvOrder = errors.New("orders: no such order") +// ErrBadAdvVerdict is a verdict outside the terminal set. It is kept distinct +// from a storage failure so the web seam can answer 400 (gogobee sent something +// it will never be able to send successfully) rather than parking a perfectly +// resolvable order on a transient database error. +var ErrBadAdvVerdict = errors.New("orders: bad verdict") + // InsertAdvOrder records a fresh, pending order and returns it with a new guid. // The guid is minted here so the owner has a stable reference the instant they // click, before gogobee has heard of it. The caller has already proved the signed- @@ -218,7 +224,7 @@ func PendingAdvOrders(limit int) ([]AdvOrder, error) { // missing row all take one path. func ResolveAdvOrder(guid, status, detail string) (AdvOrder, error) { if !validAdvVerdict(status) { - return AdvOrder{}, fmt.Errorf("orders: bad verdict %q", status) + return AdvOrder{}, fmt.Errorf("%w %q", ErrBadAdvVerdict, status) } now := nowUnix() if _, err := Get().Exec( diff --git a/internal/web/orders.go b/internal/web/orders.go index ff52ca2..f58222c 100644 --- a/internal/web/orders.go +++ b/internal/web/orders.go @@ -336,11 +336,19 @@ func (s *Server) handleAdvOrderVerdict(w http.ResponseWriter, r *http.Request) { http.Error(w, "no such order", http.StatusBadRequest) return } - if err != nil { - slog.Error("orders: resolve", "guid", v.GUID, "status", v.Status, "err", err) + if errors.Is(err, storage.ErrBadAdvVerdict) { + slog.Error("orders: verdict outside the terminal set", "guid", v.GUID, "status", v.Status) http.Error(w, "bad verdict", http.StatusBadRequest) return } + if err != nil { + // A storage failure, not a bad request. 400 here would park a perfectly + // resolvable order forever on a transient database error; 500 gets it + // retried on gogobee's next poll. + slog.Error("orders: resolve", "guid", v.GUID, "status", v.Status, "err", err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } slog.Info("orders: action resolved", "guid", order.GUID, "action", order.Action, "status", order.Status) writeJSON(w, order) } diff --git a/internal/web/push_adventure.go b/internal/web/push_adventure.go index 260d942..0548a49 100644 --- a/internal/web/push_adventure.go +++ b/internal/web/push_adventure.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "log/slog" + "net/url" "strings" "time" @@ -344,13 +345,16 @@ func orPlace(zone string) string { // what they have always done. func advRunOrStoryURL(ev storage.AdvEvent) string { if ev.RunID != "" { - return "/adventure/run/" + ev.RunID + return runReportPath(ev.RunID) } return advStoryURL(ev.GUID) } +// advStoryURL is advPermalink's relative half, and it escapes for the same +// reason: the guid arrives over a wire, and one that grew a slash would send the +// notification somewhere else entirely. func advStoryURL(guid string) string { - return "/adventure/" + guid + return "/adventure/" + url.PathEscape(guid) } // buildAdvPayload renders the notification JSON the service worker expects. The