Skip to content

fix: Correct retry logic - #3703

Open
stevehipwell wants to merge 1 commit into
integrations:mainfrom
stevehipwell:fix-retry-transport
Open

stevehipwell wants to merge 1 commit into
integrations:mainfrom
stevehipwell:fix-retry-transport

Conversation

@stevehipwell

Copy link
Copy Markdown
Collaborator

Resolves #ISSUE_NUMBER


Before the change?

  • New client retry logic was incorrect and mutated the request

After the change?

  • New client retry correctly clones the request
  • New client retry correctly resets the body

Pull request checklist

  • Schema migrations have been created if needed (example)
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No

@stevehipwell stevehipwell added this to the v6.14.0 milestone Oct 26, 2026
@stevehipwell
stevehipwell requested a balanced review from Copilot October 26, 2026 21:20
@stevehipwell stevehipwell self-assigned this Oct 26, 2026
@stevehipwell stevehipwell added the Type: Bug Something isn't working as documented label Oct 26, 2026
@github-actions

Copy link
Copy Markdown

👋 Hi, and thank you for this contribution!

This repo is maintained by GitHub and community members on a best-effort basis. We'll get to this as soon as we can.

You can help us prioritize by joining the discussion on open issues and PRs, sharing details on the changes you need, and reviewing other contributions.


🤖 This is an automated message.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Retry preparation can interfere with the previous attempt’s request, and cancellation cleanup needs regression coverage.

2 open issues
What changed in this PR

These provider review instructions are being used.

This PR corrects request handling in the provider’s internal GitHub HTTP client retry transport.

Changes:

  • Copies requests and restores request bodies before retrying.
  • Cleans up retained responses when retries end with an error.
  • Adds body-rewind tests and checks request payloads and attempt counts.
File Description
internal/​ghclient/​retry.go Updates request copying, body replay, and response cleanup.
internal/​ghclient/​retry_test.go Verifies retry counts and request payload preservation.
internal/​ghclient/​body.go Adds a helper to restore request bodies using GetBody.
internal/​ghclient/​body_test.go Tests body restoration and invalid request handling.

Review effort: Balanced

Comment thread internal/ghclient/retry.go
Comment thread internal/ghclient/retry.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Retry attempts lose generated body-replay metadata, and the early-cancellation test introduces a scheduling race.

2 open issues
2 resolved since last review

Review effort: Balanced

Comment thread internal/ghclient/retry.go
Comment thread internal/ghclient/retry_test.go Outdated
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Caller-request preservation needs regression assertions to satisfy the provider's behavior-change coverage requirements.

1 open issue
2 resolved since last review

Review effort: Balanced

Comment thread internal/ghclient/rewind.go
@stevehipwell stevehipwell added the needs-github-review Request a review from GitHub label Oct 27, 2026
@team-review-router
team-review-router Bot requested a review from a team October 27, 2026 17:43
@stevehipwell
stevehipwell requested a review from deiga October 27, 2026 17:43

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-github-review Request a review from GitHub Type: Bug Something isn't working as documented

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants