Honor per-operation retry policies in the generated client - #94
Conversation
|
Checking in on this one since it has been in draft for a while and the branch is now conflicting with The gap you describe still looks real on Could you rebase onto |
148c20c to
1fca6aa
Compare
|
Rebased onto current The generated client now uses each operation's Validation: full The PR is now ready for review. |
There was a problem hiding this comment.
🟢 Approval recommended
The implementation and conformance coverage consistently enforce the declared retry contracts without disrupting refresh behavior.
Pull request overview
Updates the generated Go client to honor operation-specific retry attempt limits and HTTP statuses.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
Changes:
- Generates retry policies from
x-hey-retry. - Preserves explicit retry-count overrides and existing refresh/hook behavior.
- Adds Go-only conformance coverage and discovery support.
File summaries
| File | Description |
|---|---|
go/templates/client.tmpl |
Generates and applies operation retry policies. |
go/pkg/generated/client.gen.go |
Regenerates the Go client with those policies. |
conformance/tests/go/retry-policy.json |
Tests retry status and attempt limits. |
conformance/runner/go/main.go |
Discovers Go-only cases and supports repeated operations. |
Review details
- Files reviewed: 3/4 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…s policy The generated client has honored each operation's retry policy since #94, but hey.Client passed WithRetryConfig, which replaced the operation's attempt count with MaxRetries+1: a route modelled with two sends got four, an operation the contract gives no policy got four on transport failures, and the policy's base delay was never read. The policy is now what an operation may spend, and the client's settings only make it gentler: MaxRetries caps the sends, BaseDelay is the floor under the first wait, MaxDelay the ceiling over every backoff wait. The operation an answer came from rides on the request context, so FollowPagination reads every later page under the same policy; a path the caller wrote still runs on the client's own settings. The shared retry-policy fixture gains hey-layer cases that prove the ceiling from both sides, and both runners read the ceiling from the case's configOverrides.
…s policy (#164) * Hold the Go client's retry settings as a ceiling over each operation's policy The generated client has honored each operation's retry policy since #94, but hey.Client passed WithRetryConfig, which replaced the operation's attempt count with MaxRetries+1: a route modelled with two sends got four, an operation the contract gives no policy got four on transport failures, and the policy's base delay was never read. The policy is now what an operation may spend, and the client's settings only make it gentler: MaxRetries caps the sends, BaseDelay is the floor under the first wait, MaxDelay the ceiling over every backoff wait. The operation an answer came from rides on the request context, so FollowPagination reads every later page under the same policy; a path the caller wrote still runs on the client's own settings. The shared retry-policy fixture gains hey-layer cases that prove the ceiling from both sides, and both runners read the ceiling from the case's configOverrides. * Declare the workflow stage read's retry policy in the spec The behavior model gives every @readonly operation a default policy of three sends on 429 and 503, so the Rust route table carries one for GetWorkflowStage, while the OpenAPI document — and with it the Go client's table — only carries what @heyRetry declares. Declaring it puts the same policy in both tables, so the two clients send the HTML route the same number of times.
What changed
The generated Go client now reads each operation's
x-hey-retrycontract:maxAttemptssets the operation default attempt budgetretryOnsets the operation-specific HTTP status listWithRetryConfigstill overrides the attempt count, while retryable statuses remain contract-definedHooks.OnRetrybehavior added by Generated client honors WithMaxRetries and retries once after a 401 refresh #137 and Generated client retries and refresh resends fire Hooks.OnRetry #139 remain intactThe patch is limited to the Go client template, its regenerated output, and one Go-only conformance case plus the runner support needed to discover it. The case uses
DeleteBoxDesignation: one call stops immediately on undeclared 500, then a second call retries declared 503 once and stops at the operation's two-attempt limit before a 200 sentinel.Rust retry behavior and the higher-level
hey.Clientdefault retry override are intentionally unchanged.Validation
GOWORK=off mise exec -- make check