adventure: don't strand an order on a database blip, don't misroute a tap

Two things a code review turned up in the W9 seams.

The verdict handler answered 400 for everything ResolveAdvOrder could fail
with, not just a bad verdict. gogobee's contract says a 400 means "park this
row for a human", so a SQLite busy or a disk hiccup permanently stranded an
extract or a bout that was perfectly resolvable. Split the two apart with
ErrBadAdvVerdict: a verdict outside the terminal set is still 400, because
gogobee will never send it successfully, and a genuine storage failure is now
500 and comes back on the next poll.

The push URL builders concatenated the guid and the run id raw, while every
other builder beside them path-escapes because these values arrive over a wire.
A guid carrying a slash sent the notification tap to a different page.
This commit is contained in:
prosolis
2026-07-24 22:36:35 -07:00
parent b07abc1d13
commit 556b9440b8
3 changed files with 23 additions and 5 deletions
+7 -1
View File
@@ -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(
+10 -2
View File
@@ -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)
}
+6 -2
View File
@@ -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