Skip to content

fix(github): retry reads that fail with a transient upstream error - #709

Draft
piekstra wants to merge 1 commit into
mainfrom
fix/retry-transient-github-reads
Draft

piekstra wants to merge 1 commit into
mainfrom
fix/retry-transient-github-reads

Conversation

@piekstra

@piekstra piekstra commented Nov 3, 2026

Copy link
Copy Markdown
Member

Summary

A review aborts with exit 5 when any one GitHub read returns a 5xx, for example:

GetFileAtRef: gitprovider: retryable upstream error: github: status 500 (response body redacted)

mapHTTPStatus already classifies 5xx, transport errors and truncated bodies as ErrRetryable, but only the outbox acts on that, for posts. Reads made while planning a review (agent files, diffs, PR metadata) give up on the first failure. A review makes many reads, and GitHub occasionally answers one with a 5xx that the same request returns moments later. One repository failed four reviews in a row this way, each on a different .codereview/agents/** file, while direct fetches of those files returned in under half a second.

doRESTWithToken now retries a GET that fails with ErrRetryable twice, after one and two seconds. Not retried:

  • 429, because GitHub's rate-limit window is longer than these delays;
  • 4xx and validation errors;
  • writes (the PATCH that accepts invitations, and everything on doRESTJSON and GraphQL);
  • any wait after the context is cancelled.

Tests

internal/gitprovider/github/http_retry_test.go covers recovery after 500 and 504, giving up after three attempts, no retry on 404, 429 or 401, no retry for a PATCH, and an early stop when the context is cancelled. Against the previous request code, the recovery and give-up tests fail. make check passes.

Model: Claude Opus 5.5 (1M context, xhigh effort) in Claude Code.

A GET that fails with a 5xx, a transport error or a truncated body is retried twice, after one and two seconds. A review makes many file and API reads, and GitHub occasionally answers one with a 5xx that the same request succeeds on moments later, so a single transient failure aborted the whole run with exit 5. Rate limits (429), client errors and writes are not retried, and the wait ends with the context.

Twapp-Session: a1c84e67-5d09-4d88-9b16-ebfd33557c24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant