fix(clients): replay rate-limited requests instead of surfacing the 429 - #207
Merged
Conversation
TypeScript counterpart of RoboFinSystems/robosystems-python-client#201. A bulk load runs at the endpoint category budget for minutes at a time, and every rejection came back to the caller as an ordinary failure. The 429 is raised by a request dependency before the endpoint handler runs, so the rejected call had no effect and replaying it is safe even for a POST carrying no idempotency key. Nothing else is retried, since nothing else carries that guarantee. createRetryingFetch composes over an existing fetch rather than replacing it, so the GraphQL client's per-request timeout wrapper stays in place and each replay gets its own full timeout. Facade REST calls go through the generated ops, which share the module-level client and take no per-call transport, so its `fetch` is the only interception point — installed from the RoboSystemsClients constructor, and never over one the caller already supplied. Retry-After is applied as a ceiling on the backoff rather than as the delay itself. The limiter is a sliding window, so the header reports the whole window — the worst case for a caller that filled its budget instantaneously, and minutes of idling for one that merely ran at the sustained rate. maxRetries / retryDelay now reach ordinary HTTP calls; previously they only fed SSE reconnects and operation polling. Set maxRetries: 0 to surface the rejection immediately, which an interactive surface may prefer over waiting.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TypeScript counterpart of RoboFinSystems/robosystems-python-client#201, for consistency across the two published SDKs.
A bulk load runs at the endpoint category budget for minutes at a time, and every rejection came back to the caller as an ordinary failure — the demo tenant load that prompted the Python change lost 25 of 383 events that way, at 92% of budget with zero headroom.
The API answers an exhausted budget with
429plusRetry-After/X-RateLimit-*, and that rejection is raised by a request dependency before the endpoint handler runs — the rejected call had no effect, so replaying it is safe even for aPOSTcarrying no idempotency key. Nothing else is retried, precisely because nothing else carries that guarantee.maxRetries/retryDelayhave been onRoboSystemsClientConfigsince the extensions clients landed but only ever fed SSE reconnects and operation polling; ordinary HTTP had no retry path. They now reach it.Changes
Hand-written
clients/only; generatedsdk/andclients/graphql/generated/untouched.clients/retry.ts(new) —createRetryingFetch, plusbackoffMs/retryAfterMsand the shared constants.fetchis resolved per call, not captured, so harnesses that swapglobalThis.fetchkeep working.Retry-Afteris applied as a ceiling on the backoff, never as the delay. The limiter is a sliding window, so the header reports the whole window — the worst case for a caller that filled its budget instantaneously, and minutes of idling for one that merely ran at the sustained rate. Exponential with full jitter, capped at 30s per attempt.ReadableStreambody is not replayed (the first attempt consumes it), and an aborted signal stops the loop rather than sleeping through it.clients/index.ts— theRoboSystemsClientsconstructor installs the retrying fetch on the shared generated client viaclient.setConfig. That is the only interception point available: facade REST calls go through the generated ops, which share the module-level client and take no per-call transport. A caller who already supplied afetchkeeps it — we never override an explicit choice.createRetryingFetchand its constants are re-exported from the package root so consumers reaching for the raw generated SDK can opt in withclient.setConfig({ fetch: createRetryingFetch() }).clients/graphql/client.ts—createGraphQLClientwraps itstimeoutFetchin the retrying fetch, so typed reads get the same treatment as writes. New optionalmaxRetries/retryDelayonGraphQLClientConfig.clients/{Ledger,Investor,Library}Client.ts— optionalmaxRetries/retryDelayon the three facade configs, threaded fromRoboSystemsClients.clients/retry.test.ts(new) — 15 tests: replay-until-accepted, the 429 surfacing once retries are exhausted,maxRetries: 0sending exactly once, other statuses untouched, a streamed body not replayed, an aborted signal short-circuiting, per-call global resolution, and the inner fetch re-running on every replay (which is what proves retry wraps timeout and not the reverse). Plus the backoff units, including thatRetry-After: 60does not become a 60s sleep.clients/index.test.ts— the SDK client mock gainssetConfig(part of the real surface), and three tests cover the wiring: the retrying fetch installed, a caller-supplied fetch left alone, and the budget reaching the GraphQL facades.Compatibility
ADDITIVE surface; one deliberate runtime-behavior change, confined to 429.
createRetryingFetch,RetryOptions,DEFAULT_MAX_RETRIES,DEFAULT_RETRY_DELAY_MS,MAX_BACKOFF_MS; optionalmaxRetries/retryDelayonGraphQLClientConfigand the three facade configs.maxRetries(default 5) before surfacing the same rejection. Nothing else is affected, and a caller who never hits a rate limit sees no change.maxRetries: 0restores the old behavior and a lower value (2 → ~3s worst case) may suit a UI better than the batch-oriented default. Happy to change the default here if you'd rather it be lower for browsers.Testing
npm run test:all(validate → test → build) in-session: prettier, eslint andtsc --noEmitclean, 331 passed (18 added), build clean.npm run prepare:publishconfirmsartifacts/retry.{js,d.ts,ts}lands in the published layout.sdk/was deliberately untouched.