refactor(retry): Caller-supplied retry gating#1413
Conversation
There's a downstream need to handle retries intelligently - both for regular HTTPS queries, but also for arbitrary promises that hide the underlying requests. Upgrade retry() to allow the caller to supply a handler that determines whether a failed request should be retried or not. This can be used intelligently to evaluate (for example) HTTP(S) failures - i.e. to discriminate on the resulting status.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 538175da22
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export function retry<T>(call: () => Promise<T>, options: RetryOptions = {}): Promise<T> { | ||
| const resolved: Required<RetryOptions> = { ...DEFAULT_RETRY_OPTIONS, ...options }; |
There was a problem hiding this comment.
Accept legacy positional retry parameters
retry is part of the public utils surface, but this change replaces (call, times, delayS) with an options object without any runtime compatibility path. In JavaScript consumers (or previously compiled JS artifacts) that still call retry(fn, 5, 0.2), the numeric second argument is silently ignored by object spread, so the function now uses defaults (retries=2, delaySeconds=1) and drops the caller’s retry budget and delay; that can cause premature failures or much longer retry latency after upgrading.
Useful? React with 👍 / 👎.
nicholaspai
left a comment
There was a problem hiding this comment.
I like this, having a universal retry util will go a long way
There's a downstream need to handle retries intelligently - both for regular HTTPS queries, but also for arbitrary promises that hide the underlying requests. Upgrade retry() to allow the caller to supply a handler that determines whether a failed request should be retried or not.
This can be used intelligently to evaluate (for example) HTTP(S) failures - i.e. to discriminate on the resulting status.