feat: RetryAttempt in PrepareRetry hook

[avmnu-sng]

Adding the retryAttempt counter, which refers to the current retry attempt to enable the server and client to make an informed decision to process the requests. For example,

• The client can hit a fallback host (server) based on the retry attempt.
• The client can send the retry count in the header, and the server can process the request in any special way.

[sonamtenzin2]

Question: Can you elaborate the scenario where we will need to know the current trial count ?

[avmnu-sng]

Hi, I have the following scenarios:

• We can send this value in the header, X-RetryableHTTP-Retry, and the client can use this information for adaptive throttling, logging, or retrying these requests differently.
• Based on the retry count, we can mutate the host (a fallback URL, similar to DLQ concepts), etc.

There might be more use cases, but I opened this PR because of the above two scenarios.

[CreatorHead]

Breaking Change?

Seems like the the function is mutated as a result the function signature has changed. Which Technically breaks backward compatibility with any code that was using the old version.

To me it seems like a breaking change. Can we add a new hook instead of changing the existing one? Or keep the old version working behind the scenes — for example, check if the old hook is set and call it if the new one isn’t used.

Either way, it is good idea to document this change clearly, in the changelog, release notes, or even just in a comments, so others aren’t caught off guard.

[avmnu-sng]

@CreatorHead
Yes, it is a breaking change, so I am open to either of the following approaches:

• Document it as a breaking change so that when people upgrade, they can update it. Also, the build step will fail, which would be caught in the CI stage / local, preventing unexpected issues.
• Introduce a new hook PrepareForRetry(req, retryAttempt) and mark the current hook PrepareRetry(req) deprecate to remove it from future releases.

I prefer the first option. Let me know the final decision so I can make the changes accordingly.

[CreatorHead]

@avmnu-sng, thanks for laying out the options!

Let’s avoid the breaking change for now:

• Keep PrepareRetry(req) as a thin shim that delegates to the new
  PrepareForRetry(req, attempt) (with attempt defaulting to 1).
• Add a // Deprecated: tag plus a lint/staticcheck rule so users see a warning, not an error.
• Document the deprecation in the release notes and set a removal target.

That gives everyone a smooth upgrade path while still moving us to the richer API. Thanks!

[avmnu-sng]

@CreatorHead Sure, I can add PrepareForRetry. We won't be able to use PrepareRetry(req) as a delegator. The implementation would now be something like (which I believe is what you meant):

if c.PrepareForRetry != nil {
  c.PrepareForRetry(req, attempt)
} else if c.PrepareRetry != nil {
  c.PrepareRetry(req)
}

Also, I don't think, we have a RELEASE_NOTES.md. For now, I will update the CHANGELOG.md to include a TO RELEASE section and add the deprecation note and target it for removal in v8.0.0?

The Staticcheck SA1019 will pick up the deprecation warning when run. I don't see a config to disable it, so no additional work is needed.

[hashicorp-cla-app]

All committers have signed the CLA.

[CreatorHead]

Thanks for the update — the changes to PrepareRetry look solid overall, but a few tweaks would help polish things up:

Suggested Changes

• Docs & Comments
  The new retryAttempt param is a great addition, but it needs a brief comment explaining its purpose. Some of the existing comments (e.g., around PrepareRetry) could also be trimmed for clarity.

• Error Context
  When PrepareRetry fails, consider wrapping the error with context (e.g., which attempt failed) to make debugging easier.

• Naming
  prepareChecks could be renamed to something clearer like prepareRetryCount to better reflect its purpose.

• Header Logic
  A short comment on why the retry count is set in the header would help future readers.

• Test Coverage
  Please double-check that the retry logic is well-covered by tests, especially the new param behavior.

Let me know if you’d like help with any of these. Almost there!

[avmnu-sng]

@CreatorHead, I am sorry, but I am confused now. Do you want me to go ahead with the breaking change now? No adding another hook #258 (comment)?

I follow a minimal change philosophy. The PrepareRetry hook implementation, error handling, and testing already existed; therefore, I just introduced the param retryAttempt and made minimal changes in the existing tests to ensure it works as expected.

Renaming, updating how errors are handled, etc., are outside the scope of adding a retry counter parameter, which must be done separately.