feat(client): add option to retry only idempotent requests
What
retryHTTPCheck retries a request on any 5xx (and on status 0) without
checking the HTTP method. A non-idempotent request such as a POST is
therefore retried, which can duplicate a side effect the server already
applied. GitLab can, for example, create a note and still return a 5xx; the
retry then posts a second note.
Following review, the fix is opt-in: the default behavior is unchanged, and a
new WithOnlyIdempotentRetries client option enables the new behavior. The
maintainers plan to make it the default in the 3.0 release.
Change
A new client option, WithOnlyIdempotentRetries, restricts automatic retries
to idempotent methods. With the option set:
- The 5xx / status-
0branch only retries an idempotent method, soPOSTandPATCHare not retried. AnilRequestkeeps the previous behavior for callers that synthesize a response without one (for example the status-0AWS ALB case). - The network-error branch uses the same idempotent method set in place of its legacy inline list.
The method set follows RFC 9110 §9.2.2 (idempotent methods: GET, HEAD,
OPTIONS, TRACE, PUT, DELETE) plus QUERY, which RFC 10008 defines as
safe and idempotent. net/http has no QUERY constant yet, so it is a string
literal.
429 Too Many Requests is still retried for every method, with or without the
option. A rate-limited request is rejected before the server processes it, so
retrying it cannot duplicate a side effect.
Without the option, nothing changes: the 5xx / status-0 branch retries any
method, and the network-error branch keeps its previous inline method list
(including CONNECT, and excluding PUT and DELETE).
Tests
The by-method retry tests run each case twice, once per mode:
TestClient_DefaultRetryPolicy_RetryOn5xxStatus_ByMethod: a 5xx is retried for every method by default; with the option it is retried for idempotent methods and not forPOST,PATCH,CONNECT.TestClient_DefaultRetryPolicy_RetryOnIdempotentRequests_ByMethod(network errors): the default keeps the legacy method list; the option applies the idempotent set.TestClient_DefaultRetryPolicy_RetryOnTooManyRequests_AnyMethod: a429is retried forGET,POST, andPATCHeven with the option set.TestMarkdownUploads_UploadProjectMarkdown_NoRetryOn5xx: with the option, an upload (aPOST) is sent once and the error surfaces without a retry.