diff --git a/changelog.d/192.md b/changelog.d/192.md index 916fc5e..8851646 100644 --- a/changelog.d/192.md +++ b/changelog.d/192.md @@ -25,6 +25,10 @@ - A removal that changes nothing writes nothing, rather than replacing the set with itself and opening a race for no state change (#192). +- Every failure diagnostic on the forgejo backend names the verb as well as the + path and the status. A read used to say `HTTP 500 from 'repos/…'`, which + cannot be told from a failed write of the same path (#192). + - The diagnostic names what was attempted and that it did not happen, instead of blaming a missing label and telling the operator to bootstrap — a cause it had not established (#192, #101). diff --git a/lib/forge-forgejo.sh b/lib/forge-forgejo.sh index 066ce2c..0583f86 100644 --- a/lib/forge-forgejo.sh +++ b/lib/forge-forgejo.sh @@ -123,7 +123,7 @@ forge_api() { echo "forge_api: request failed: $endpoint" >&2 return 1 fi - forgejo_http_ok "$hdr" "$endpoint" || return 1 + forgejo_http_ok "$hdr" "GET $endpoint" || return 1 if [ "$have_jq" = true ]; then jq -r "$jqexpr" <"$body"; else cat "$body"; fi return 0 fi @@ -140,7 +140,7 @@ forge_api() { echo "forge_api: request failed: $endpoint (page $page)" >&2 return 1 fi - forgejo_http_ok "$hdr" "$endpoint" || return 1 + forgejo_http_ok "$hdr" "GET $endpoint" || return 1 # Re-read on EVERY page, not once (#4712). A board that changes size # under the walk was invisible: page 1 declaring 4 and page 2 declaring @@ -223,9 +223,14 @@ EOF printf '%s\n' "$total" } -# forgejo_http_ok — a non-2xx is named, not +# forgejo_http_ok — a non-2xx is named, not # swallowed. gh exits non-zero on HTTP failure; curl does not without -f, # and -f would throw away the body that says why. +# The second argument carries the VERB as well as the path — "GET repos/…", +# "PUT repos/…". #192's acceptance criterion is that a failure names the verb, +# the path and the status, and reads used to omit the verb: a caller reading +# `HTTP 500 from 'repos/o/r/issues/5'` could not tell a failed read from a +# failed write of the same path (@codex-reviewer-andresmgsl). forgejo_http_ok() { local hdr="$1" endpoint="$2" code code="$(tr -d '\r' <"$hdr" | awk '/^HTTP\// { c = $2 } END { print c }')" diff --git a/test/forge-backends.test.sh b/test/forge-backends.test.sh index 908b5ab..fd44a84 100644 --- a/test/forge-backends.test.sh +++ b/test/forge-backends.test.sh @@ -376,7 +376,11 @@ fail_get() { REPO=o/r forge_issue_edit 5 --remove-label state:old } check "a failed current-label GET refuses, non-zero" 1 "" fail_get -check "...naming the verb, path and status" 1 "500" fail_get +check "...naming the verb, the path AND the status, in one diagnostic" 1 \ + "HTTP 500 from 'GET repos/o/r/issues/5'" fail_get +get_write_count() { : >"$WRITES"; fail_get >/dev/null 2>&1; wc -l <"$WRITES"; } +check "...having written nothing: the read failed before any mutation" 0 "0" \ + get_write_count fail_put() { FAKE_LABELS="$ROSTER" FAKE_LABEL_N=4 \ FAKE_ISSUE_LABELS='[{"name":"state:old","id":11},{"name":"scope:labels","id":13}]' \ @@ -384,7 +388,10 @@ fail_put() { REPO=o/r forge_issue_edit 5 --remove-label state:old } check "a failed replacement PUT refuses, non-zero" 1 "" fail_put -check "...naming the verb, path and status" 1 "500" fail_put +check "...naming the verb, the path AND the status, in one diagnostic" 1 \ + "HTTP 500 from 'PUT repos/o/r/issues/5/labels'" fail_put +put_write_count() { : >"$WRITES"; fail_put >/dev/null 2>&1; wc -l <"$WRITES"; } +check "...having attempted only the one PUT" 0 "1" put_write_count # An ADD-ONLY call keeps the additive POST (ceremony#128): a read-modify-write # there clobbered a label set two seconds after a builder wrote it.