Skip to content

fix(clients): replay rate-limited requests instead of surfacing the 429 - #207

Merged
jfrench9 merged 1 commit into
mainfrom
bugfix/retry-rate-limited-requests
Aug 31, 2026
Merged

fix(clients): replay rate-limited requests instead of surfacing the 429#207
jfrench9 merged 1 commit into
mainfrom
bugfix/retry-rate-limited-requests

Conversation

@jfrench9

Copy link
Copy Markdown
Member

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 429 plus Retry-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 a POST carrying no idempotency key. Nothing else is retried, precisely because nothing else carries that guarantee.

maxRetries / retryDelay have been on RoboSystemsClientConfig since 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; generated sdk/ and clients/graphql/generated/ untouched.

  • clients/retry.ts (new) — createRetryingFetch, plus backoffMs / retryAfterMs and the shared constants.
    • 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 rather than sharing one across attempts. The global fetch is resolved per call, not captured, so harnesses that swap globalThis.fetch keep working.
    • Retry-After is 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.
    • A ReadableStream body is not replayed (the first attempt consumes it), and an aborted signal stops the loop rather than sleeping through it.
  • clients/index.ts — the RoboSystemsClients constructor installs the retrying fetch on the shared generated client via client.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 a fetch keeps it — we never override an explicit choice. createRetryingFetch and its constants are re-exported from the package root so consumers reaching for the raw generated SDK can opt in with client.setConfig({ fetch: createRetryingFetch() }).
  • clients/graphql/client.tscreateGraphQLClient wraps its timeoutFetch in the retrying fetch, so typed reads get the same treatment as writes. New optional maxRetries / retryDelay on GraphQLClientConfig.
  • clients/{Ledger,Investor,Library}Client.ts — optional maxRetries / retryDelay on the three facade configs, threaded from RoboSystemsClients.
  • clients/retry.test.ts (new) — 15 tests: replay-until-accepted, the 429 surfacing once retries are exhausted, maxRetries: 0 sending 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 that Retry-After: 60 does not become a 60s sleep.
  • clients/index.test.ts — the SDK client mock gains setConfig (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.

  • New exports: createRetryingFetch, RetryOptions, DEFAULT_MAX_RETRIES, DEFAULT_RETRY_DELAY_MS, MAX_BACKOFF_MS; optional maxRetries / retryDelay on GraphQLClientConfig and the three facade configs.
  • Unchanged: every facade signature and return type. No removals, no renames, no narrowed inputs, no generated-type changes.
  • Behavior: a request that would have failed on 429 now waits and retries up to maxRetries (default 5) before surfacing the same rejection. Nothing else is affected, and a caller who never hits a rate limit sees no change.
  • Worth a look from the app owners. The three frontends are interactive surfaces, and the default budget means a sustained rate limit can now sit behind a spinner for up to ~30s instead of erroring straight away. maxRetries: 0 restores 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.
  • Stable-tier signatures untouched, so this ships as a minor. Version bump is the release dispatch's job, not this PR's.

Testing

  • npm run test:all (validate → test → build) in-session: prettier, eslint and tsc --noEmit clean, 331 passed (18 added), build clean.
  • npm run prepare:publish confirms artifacts/retry.{js,d.ts,ts} lands in the published layout.
  • Not regenerated — no API surface change is involved, so sdk/ was deliberately untouched.

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.
@jfrench9
jfrench9 merged commit c731fb2 into main Aug 31, 2026
5 checks passed
@jfrench9
jfrench9 deleted the bugfix/retry-rate-limited-requests branch August 31, 2026 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant