fix: Retry the release POST instead of stranding the tag #44

Merged
ahmad merged 1 commit from feature/40-retry-release-post into main 2026-09-04 19:02:45 +00:00
Owner

Issue

Closes #40

Problem

release/action.yml pushes the tag and then POSTs the release that references it. When Forgejo has not finished indexing that tag the POST returns 500, curl -fsS exits 22, and the job dies after the tag exists — leaving a version with a release missing, publish and deploy skipped, and a re-dispatch that derives nothing because the tag is already there. The images can never be built for that version.

Observed on ahmad/imamah v0.5.0. Replaying the identical call by hand minutes later returned 201.

Solution

The POST retries with exponential backoff (2s, 4s, 8s, 16s; five attempts) on a server fault or a lost connection, and reports the status and the response body when it gives up. curl -f is dropped deliberately — it exits 22 and discards the body, which is why the original incident needed a dig through the job log to discover a plain 500.

Review notes

Two judgement calls the issue does not name, both forced by adding a retry rather than optional:

  • 409 is treated as published, not as a failure. A retry only exists because a request can be lost, and the case where the response is lost rather than the request is exactly when the second attempt finds the release already created. Failing there would make the retry able to undo itself — turning a survivable blip into the stranded tag this issue is about.
  • Only 5xx and 000 retry. 401, 403 and 422 fail immediately with the body. Retrying a bad token five times over thirty seconds adds delay and tells nobody anything.

Verified by extracting the loop and driving a stub curl through status sequences, with sleep stubbed to record the backoff rather than spend it:

201                    1 call,  exit 0, no delay        (timing unchanged)
500,201                2 calls, exit 0, slept 2s
500 forever            5 calls, exit 1, status + body + recovery note
000,409                2 calls, exit 0  "already exists; treating as published"
000,000,201            3 calls, exit 0, slept 2s then 4s
401 / 422              1 call,  exit 1, body shown, no retry
MUTATION max_attempts=1, 500 then 201    exit 1 — the stranded tag returns

That exercise found a real bug in my first version, which is the part worth reading. I had written curl … || echo 000, not noticing that curl -w '%{http_code}' already prints 000 when it never got a status. The two concatenated to 000000, which matched neither 5* nor 000, fell through to the fail-fast branch, and meant a dropped connection — the case most deserving a retry — would never have been retried. It is now || true, which only stops set -e acting on curl's non-zero exit, with a comment saying why an echo there is wrong.

The whole point of this change is surviving a transient failure, so shipping it unable to survive the most transient failure of all would have been quietly useless. It passed every other case before I ran that one.

Risks and trade-offs

A genuinely unreachable Forgejo now costs 30 seconds before the job fails rather than failing at once. That is the trade the issue asks for.

This does not address the ordering — the tag is still pushed before the release is known to be creatable, so any failure after that point still strands a version. A retry covers a transient 500; it does nothing for a runner killed mid-step. #40's notes make that case and it is deliberately left out of scope here, but it is the larger fix and this should not be read as closing it.

### Issue Closes #40 ### Problem `release/action.yml` pushes the tag and then `POST`s the release that references it. When Forgejo has not finished indexing that tag the POST returns 500, `curl -fsS` exits 22, and the job dies **after** the tag exists — leaving a version with a release missing, `publish` and `deploy` skipped, and a re-dispatch that derives nothing because the tag is already there. The images can never be built for that version. Observed on `ahmad/imamah` v0.5.0. Replaying the identical call by hand minutes later returned 201. ### Solution The POST retries with exponential backoff (2s, 4s, 8s, 16s; five attempts) on a server fault or a lost connection, and reports the status *and* the response body when it gives up. `curl -f` is dropped deliberately — it exits 22 and discards the body, which is why the original incident needed a dig through the job log to discover a plain 500. ### Review notes **Two judgement calls the issue does not name**, both forced by adding a retry rather than optional: - **`409` is treated as published, not as a failure.** A retry only exists because a request can be lost, and the case where the *response* is lost rather than the request is exactly when the second attempt finds the release already created. Failing there would make the retry able to undo itself — turning a survivable blip into the stranded tag this issue is about. - **Only `5xx` and `000` retry.** `401`, `403` and `422` fail immediately with the body. Retrying a bad token five times over thirty seconds adds delay and tells nobody anything. **Verified by extracting the loop and driving a stub `curl` through status sequences**, with `sleep` stubbed to record the backoff rather than spend it: ``` 201 1 call, exit 0, no delay (timing unchanged) 500,201 2 calls, exit 0, slept 2s 500 forever 5 calls, exit 1, status + body + recovery note 000,409 2 calls, exit 0 "already exists; treating as published" 000,000,201 3 calls, exit 0, slept 2s then 4s 401 / 422 1 call, exit 1, body shown, no retry MUTATION max_attempts=1, 500 then 201 exit 1 — the stranded tag returns ``` **That exercise found a real bug in my first version**, which is the part worth reading. I had written `curl … || echo 000`, not noticing that `curl -w '%{http_code}'` *already* prints `000` when it never got a status. The two concatenated to `000000`, which matched neither `5*` nor `000`, fell through to the fail-fast branch, and meant a dropped connection — the case most deserving a retry — would never have been retried. It is now `|| true`, which only stops `set -e` acting on curl's non-zero exit, with a comment saying why an `echo` there is wrong. The whole point of this change is surviving a transient failure, so shipping it unable to survive the most transient failure of all would have been quietly useless. It passed every other case before I ran that one. ### Risks and trade-offs A genuinely unreachable Forgejo now costs 30 seconds before the job fails rather than failing at once. That is the trade the issue asks for. This does not address the **ordering** — the tag is still pushed before the release is known to be creatable, so any failure after that point still strands a version. A retry covers a transient 500; it does nothing for a runner killed mid-step. #40's notes make that case and it is deliberately left out of scope here, but it is the larger fix and this should not be read as closing it.
fix: Retry the release POST instead of stranding the tag
All checks were successful
PR / validate (pull_request) Successful in 3m3s
b7d8fdadb7
The tag is pushed before the release is created, so a transient 500
from Forgejo left a version with no release, publish and deploy
skipped, and no way to build images for that tag afterwards.

The POST now retries with exponential backoff on a server fault or a
lost connection, treats 409 as already-published so a retried request
cannot undo itself, and fails fast on a status no retry will fix.
Dropping curl -f keeps the status and body for the failure message.

Closes #40
Author
Owner

validate green on b7d8fda. Verification against each acceptance criterion.

The retry loop was extracted from the action and driven through status sequences with a stub curl, and sleep stubbed to record the backoff rather than spend it.

Given the release POST returns 500, when the step runs, then it retries with backoff before failing the job.

500 forever   5 calls, slept 2s 4s 8s 16s, exit 1

Given a retry then succeeds, when the job finishes, then it is green and publish runs as normal.

500,201       2 calls, slept 2s, "Published release v1.2.3.", exit 0

publish gates on needs.version.outputs.released, which is written before this step, so an exit 0 here is exactly what lets it run — unchanged from before.

Given every retry fails, when the job fails, then the log names the status code and the response body — not only curl: (22).

Creating the release failed after 5 attempts (last status 500).
--- response body ---
{"message":"stub body for status 500"}
Tag v1.2.3 is pushed but has no release. Recover by creating
the release for the existing tag; a re-dispatch will derive nothing.

Given the POST succeeds first time, when the job runs, then behaviour and timing are unchanged.

201           1 call, no sleep, exit 0

Beyond the criteria, because adding a retry forces the questions:

000,409       2 calls, exit 0  "already exists; treating as published"
000,000,201   3 calls, exit 0
401 / 422     1 call,  exit 1, body shown, no retry
MUTATION max_attempts=1, 500 then 201  ->  exit 1, the stranded tag returns

The exercise found a real bug in my first version. I wrote curl … || echo 000 without noticing that curl -w '%{http_code}' already prints 000 when it never received a status. The two concatenated to 000000, matched neither 5* nor 000, and fell through to fail-fast — so a dropped connection, the most transient failure there is, would never have been retried. Now || true, with a comment saying why an echo there is wrong. Every other case passed before I ran that one, which is the argument for running the awkward ones.

Worth recording against this PR: it would not have prevented today's third stranded version. imamah v0.5.2 was stranded an hour ago because the runner reported a job that had succeeded — its log ends Published release v0.5.2. then Job succeeded — as failure, blocking publish and deploy. Nothing failed for a retry to catch. This change covers a transient 500 and a lost connection; it does not cover a phantom failure, a killed runner, or anything else after the tag is pushed. Only reordering the tag behind the artefacts covers the class, which #40's notes argue and this PR deliberately does not attempt.

`validate` green on `b7d8fda`. Verification against each acceptance criterion. The retry loop was extracted from the action and driven through status sequences with a stub `curl`, and `sleep` stubbed to record the backoff rather than spend it. > Given the release POST returns 500, when the step runs, then it retries with backoff before failing the job. ``` 500 forever 5 calls, slept 2s 4s 8s 16s, exit 1 ``` > Given a retry then succeeds, when the job finishes, then it is green and `publish` runs as normal. ``` 500,201 2 calls, slept 2s, "Published release v1.2.3.", exit 0 ``` `publish` gates on `needs.version.outputs.released`, which is written before this step, so an exit 0 here is exactly what lets it run — unchanged from before. > Given every retry fails, when the job fails, then the log names the status code and the response body — not only `curl: (22)`. ``` Creating the release failed after 5 attempts (last status 500). --- response body --- {"message":"stub body for status 500"} Tag v1.2.3 is pushed but has no release. Recover by creating the release for the existing tag; a re-dispatch will derive nothing. ``` > Given the POST succeeds first time, when the job runs, then behaviour and timing are unchanged. ``` 201 1 call, no sleep, exit 0 ``` **Beyond the criteria**, because adding a retry forces the questions: ``` 000,409 2 calls, exit 0 "already exists; treating as published" 000,000,201 3 calls, exit 0 401 / 422 1 call, exit 1, body shown, no retry MUTATION max_attempts=1, 500 then 201 -> exit 1, the stranded tag returns ``` **The exercise found a real bug in my first version.** I wrote `curl … || echo 000` without noticing that `curl -w '%{http_code}'` already prints `000` when it never received a status. The two concatenated to `000000`, matched neither `5*` nor `000`, and fell through to fail-fast — so a dropped connection, the most transient failure there is, would never have been retried. Now `|| true`, with a comment saying why an `echo` there is wrong. Every other case passed before I ran that one, which is the argument for running the awkward ones. **Worth recording against this PR: it would not have prevented today's third stranded version.** `imamah` v0.5.2 was stranded an hour ago because the runner reported a job that had *succeeded* — its log ends `Published release v0.5.2.` then `Job succeeded` — as `failure`, blocking `publish` and `deploy`. Nothing failed for a retry to catch. This change covers a transient 500 and a lost connection; it does not cover a phantom failure, a killed runner, or anything else after the tag is pushed. Only reordering the tag behind the artefacts covers the class, which #40's notes argue and this PR deliberately does not attempt.
ahmad changed title from WIP: fix: Retry the release POST instead of stranding the tag to fix: Retry the release POST instead of stranding the tag 2026-09-04 18:32:49 +00:00
ahmad_bot approved these changes 2026-09-04 19:02:07 +00:00
ahmad_bot left a comment

Approving. All four criteria hold, and I reproduced your table row for row against a stub curl with sleep recording the backoff rather than spending it:

201                    calls=1 exit=0  no delay          criterion 4: timing unchanged
500,201                calls=2 exit=0  slept 2s          criteria 1 and 2
500 x5                 calls=5 exit=1  2s 4s 8s 16s      criterion 3: status and body reported
000,409                calls=2 exit=0                    "already exists; treating as published"
000,000,201            calls=3 exit=0  slept 2s 4s
401 / 422              calls=1 exit=1  body shown, no retry
MUTATION max_attempts=1, 500 then 201  calls=1 exit=1    the stranded tag returns

Five attempts, four sleeps, doubling from 2s — the arithmetic matches what the loop claims.

The bug you found is real, and I confirmed the mechanism against a live curl rather than taking it on trust:

curl -sS -o /dev/null -w '%{http_code}' <unreachable>          -> '000'      (3 chars)
same with `|| echo 000`                                        -> '000000'   (6 chars)
'000000' against the case: matches neither 5* nor 000          -> falls through to fail-fast

So the first version would have failed immediately on a dropped connection — the single most transient failure there is, and the one a retry exists for. Finding that by running the case rather than reading the branch is the difference between a retry that works and one that reads as though it does; it would have passed every other row.

Both judgement calls are right, and neither is optional once you add a retry. Treating 409 as published is what stops the retry undoing itself when the response is lost rather than the request — the exact scenario the 000 path creates. And restricting retries to 5xx/000 means a bad token fails in one call with its body rather than after thirty seconds of pointless backoff. Dropping curl -f is the same instinct: exit 22 with the body discarded is precisely why the original incident needed a dig through the job log.

The recovery note on final failure — that the tag exists, that a re-dispatch will derive nothing, and that the fix is to create the release for the existing tag — puts the knowledge the incident produced where the next person will be standing when they need it.

Saying this does not fix the ordering is the right call and worth keeping visible. The tag is still pushed before the release is known creatable, so a runner killed mid-step still strands a version. A retry covers a transient 500 and nothing else. #40's notes make that case; this closing it should not be read as that being solved.

Merging moves v3, so every repository's next release picks this up. Approval does not merge.

Approving. All four criteria hold, and I reproduced your table row for row against a stub `curl` with `sleep` recording the backoff rather than spending it: ``` 201 calls=1 exit=0 no delay criterion 4: timing unchanged 500,201 calls=2 exit=0 slept 2s criteria 1 and 2 500 x5 calls=5 exit=1 2s 4s 8s 16s criterion 3: status and body reported 000,409 calls=2 exit=0 "already exists; treating as published" 000,000,201 calls=3 exit=0 slept 2s 4s 401 / 422 calls=1 exit=1 body shown, no retry MUTATION max_attempts=1, 500 then 201 calls=1 exit=1 the stranded tag returns ``` Five attempts, four sleeps, doubling from 2s — the arithmetic matches what the loop claims. **The bug you found is real, and I confirmed the mechanism against a live `curl` rather than taking it on trust:** ``` curl -sS -o /dev/null -w '%{http_code}' <unreachable> -> '000' (3 chars) same with `|| echo 000` -> '000000' (6 chars) '000000' against the case: matches neither 5* nor 000 -> falls through to fail-fast ``` So the first version would have failed immediately on a dropped connection — the single most transient failure there is, and the one a retry exists for. Finding that by running the case rather than reading the branch is the difference between a retry that works and one that reads as though it does; it would have passed every other row. **Both judgement calls are right, and neither is optional once you add a retry.** Treating `409` as published is what stops the retry undoing itself when the *response* is lost rather than the request — the exact scenario the `000` path creates. And restricting retries to `5xx`/`000` means a bad token fails in one call with its body rather than after thirty seconds of pointless backoff. Dropping `curl -f` is the same instinct: exit 22 with the body discarded is precisely why the original incident needed a dig through the job log. The recovery note on final failure — that the tag exists, that a re-dispatch will derive nothing, and that the fix is to create the release for the existing tag — puts the knowledge the incident produced where the next person will be standing when they need it. **Saying this does not fix the ordering is the right call and worth keeping visible.** The tag is still pushed before the release is known creatable, so a runner killed mid-step still strands a version. A retry covers a transient 500 and nothing else. #40's notes make that case; this closing it should not be read as that being solved. Merging moves `v3`, so every repository's next release picks this up. Approval does not merge.
ahmad merged commit 6ebe5524fd into main 2026-09-04 19:02:45 +00:00
ahmad deleted branch feature/40-retry-release-post 2026-09-04 19:02:49 +00:00
Sign in to join this conversation.
No description provided.