86 lines
4.1 KiB
Markdown
86 lines
4.1 KiB
Markdown
# PR#155: A commit status lost on a dead connection is sent once more
|
|
|
|
> **WARNING:** This document describes only the change applied in this PR.
|
|
> It may already be outdated once the next PR is merged.
|
|
> Historic PR-documentation is not maintained along with new PRs — treat it as a snapshot, not as current documentation.
|
|
|
|
## Problem
|
|
|
|
werkcrappit PR#68 stayed unmerged although werkrevisor gave GREEN and scheduled the merge.
|
|
Its review build `claude-code/demo-links@ai-review` succeeded on 2026-10-12 at 04:44:31 UTC, after 34 s.
|
|
Gitea got "build queued" and "build running" for `ai-review/gate` on the head, but never the final `success`.
|
|
The combined status stayed `pending`, so Gitea never merged.
|
|
|
|
The journal of mih09 holds the cause, at the very moment the build ended:
|
|
|
|
Could not publish Gitea status for 4a3109bc…: I/O error on POST request for
|
|
"https://git.javagil.de/api/v1/repos/mi/werkcrappit/statuses/4a3109bc…": HTTP/1.1 header parser received no bytes
|
|
|
|
`GiteaClient` keeps one JDK `HttpClient` and with it a pool of open connections.
|
|
A request answered by no byte at all is what a pooled connection looks like that the forge's proxy had closed meanwhile.
|
|
The JDK client never repeats a POST by itself, and `publishStatus` tried exactly once and only logged the failure.
|
|
Nothing sends a lost status later, so one dropped connection kept a required check pending for good.
|
|
|
|
## Non-Goals
|
|
|
|
- No reconciliation of statuses already lost: a restart of the build sends them again.
|
|
- No retry of an HTTP error answer: the forge decided, and the same request would get the same answer.
|
|
- No change of the connection pool or of its keep-alive time: the proxy's own time is not known to Werkator.
|
|
|
|
## The Scenarios
|
|
|
|
### Feature: commit statuses reach the forge
|
|
|
|
#### Scenario#155.01: A status whose request is answered by nothing is sent once more!
|
|
|
|
So that one dead pooled connection does not leave a required check pending for good.
|
|
|
|
- **Given** Werkator publishes a commit status
|
|
- **and** the first request is answered by nothing at all
|
|
- **When** that request fails with an I/O error
|
|
- **Then** Werkator sends the same status once more, half a second later
|
|
- **and** the status arrives
|
|
|
|
##### Verified by
|
|
|
|
- [GiteaClientIntegrationTest: a request answered by nothing is sent once more, so the status still arrives](../../src/test/kotlin/de/hoennig/werkator/gitea/GiteaClientIntegrationTest.kt)
|
|
|
|
#### Scenario#155.02: A second request answered by nothing ends the attempts!
|
|
|
|
So that a forge that cannot be reached costs at most two requests per status.
|
|
|
|
- **Given** Werkator publishes a commit status
|
|
- **and** every request is answered by nothing at all
|
|
- **When** the second request fails with an I/O error too
|
|
- **Then** Werkator sends no third request
|
|
- **and** logs a warning
|
|
|
|
##### Verified by
|
|
|
|
- [GiteaClientIntegrationTest: a second request answered by nothing gives up after it](../../src/test/kotlin/de/hoennig/werkator/gitea/GiteaClientIntegrationTest.kt)
|
|
|
|
#### Scenario#155.03: A status the forge answered with an HTTP error is not sent again!
|
|
|
|
So that a status the forge refused is not repeated: the same request would get the same answer.
|
|
|
|
- **Given** Werkator publishes a commit status
|
|
- **When** Gitea answers with HTTP 500
|
|
- **Then** Werkator sends no second request
|
|
- **and** logs a warning
|
|
|
|
##### Verified by
|
|
|
|
- [GiteaClientIntegrationTest: an HTTP error answer is not sent again](../../src/test/kotlin/de/hoennig/werkator/gitea/GiteaClientIntegrationTest.kt)
|
|
|
|
## The Solution
|
|
|
|
- `publishStatus` catches `ResourceAccessException`, Spring's I/O error, apart from the other client errors.
|
|
`retryStatus` waits 500 ms, logs the retry and sends the same body once more.
|
|
- Sending a status twice is harmless: Gitea records the same state again.
|
|
- The POST itself moved into `postStatus`, so both attempts share it and each method stays small.
|
|
- The retry waits on the thread that publishes the status, and only after an I/O error.
|
|
For "build queued" that is the watcher's thread, which then polls half a second later.
|
|
- WireMock's `Fault.EMPTY_RESPONSE` stands for the dead connection in the tests.
|
|
- Werkbaum: a node under `#pr`.
|
|
- Version 1.41.6, with an entry in the release notes.
|