fix(forge): retry 5xx server errors at the HTTP client level - #24
Closed
guyoron1 wants to merge 17 commits into
Closed
fix(forge): retry 5xx server errors at the HTTP client level#24guyoron1 wants to merge 17 commits into
guyoron1 wants to merge 17 commits into
Conversation
Move 5xx retry handling from the higher-level retryOnTransient wrapper (now renamed retryOnRepoRace) down into isRetryable, which is used by do(). This ensures all GitHub API calls automatically retry on transient server errors (500-504), not just the handful of call sites that were wrapped in retryOnTransient. This fixes a 502 Bad Gateway failure in post-review's GetPullRequestHeadSHA, which had no retry coverage because it called get() directly. Rename retryOnTransient to retryOnRepoRace and narrow isTransientStatus to only cover 404 (async repo init) and 409 (branch ref conflict), which are the race conditions that wrapper actually exists for. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
Owner
Author
|
/fs-qf |
Verdict: APPROVED_WITH_FINDINGS (89/100) - 0 critical, 3 major, 4 minor findings - Key findings: metadata type mislabel, missing requirement IDs, epic tracking field mismatch
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
QualityFlow Pipeline Summary
Test Output
Issue: GH-24 Generated by QualityFlow |
Owner
Author
|
/fs-qf |
|
🤖 Finished Review · ✅ Success · Started 3:39 PM UTC · Completed 3:53 PM UTC |
Verdict: APPROVED_WITH_FINDINGS (84/100) - 0 critical, 5 major, 4 minor findings - Key findings: metadata type mislabel, missing requirement IDs, Go version mismatch, requirement summary format, QE kickoff timing
ReviewReason: stale-head The review agent reviewed commit Previous runReviewReason: stale-head The review agent reviewed commit |
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 3:56 PM UTC · Completed 4:08 PM UTC |
Generated Software Test Description (STD) with 34 test scenarios covering retry 5xx server errors at the HTTP client level. - STD YAML: comprehensive test description with all scenarios - Go stubs: 8 test files with 34 PendingIt blocks (Phase 1 design stubs) - Coverage: 15 P0 + 19 P1 scenarios across 8 requirement groups Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/fs-review |
Generated 34 Go test implementations from STD YAML covering: - isRetryable 5xx status code validation (scenarios 1-7) - do() retry behavior and context cancellation (scenarios 8-12) - Double-retry prevention (scenarios 13-15) - retryOnRepoRace scoping (scenarios 16-20) - isTransientStatus boundary tests (scenarios 21-23) - Error message format validation (scenarios 24-26) - File operation integration tests (scenarios 27-30) - Rate limit regression tests (scenarios 31-34) All tests verified to compile and pass against source.
Owner
Author
|
Closing — replacing with fresh upstream PR mirror for QF demo |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upstream PR
Mirror of fullsend-ai/fullsend#2342 by @ralphbean
Summary
retryOnTransientdown intoisRetryableindo(), so all GitHub API calls automatically retry on transient server errorsretryOnTransient→retryOnRepoRaceand narrowsisTransientStatusto only 404/409, reflecting its actual purpose (repo init races and branch ref conflicts)QF Demo Value
Why this is a great QF target:
github_test.go— QF can identify untested pathsMirrored for QualityFlow integration demo — trigger with
/fs-qf