Skip to content

fix: expose SDK rate limit controls#999

Open
realfishsam wants to merge 1 commit into
mainfrom
fix/sdk-rate-limit-options
Open

fix: expose SDK rate limit controls#999
realfishsam wants to merge 1 commit into
mainfrom
fix/sdk-rate-limit-options

Conversation

@realfishsam

Copy link
Copy Markdown
Contributor

Summary

  • add TypeScript SDK rateLimit option/getter/setter and apply it to SDK HTTP dispatch
  • add Python SDK rate_limit constructor option/property and apply it to SDK HTTP dispatch
  • update the Python exchange generator so concrete generated exchange wrappers expose rate_limit

Fixes #996

Test Plan

  • python3 -m py_compile sdks/python/pmxt/client.py sdks/python/pmxt/models.py sdks/python/pmxt/_exchanges.py
  • node core/scripts/generate-python-exchanges.js (rerun stability checked by checksum)
  • static smoke check for TypeScript/Python rate-limit accessor and dispatch wiring
  • git diff --check

Blocked locally:

  • TypeScript Jest test could not run in this checkout because sdks/typescript/generated/src/index.js is absent (same generated-client environment blocker seen in existing focused SDK PRs).
  • Python pytest could not run because this cron image's /usr/bin/python3 lacks pip/pytest; py_compile passed.

Add TypeScript and Python SDK rate limit accessors that mirror the core BaseExchange rateLimit surface, including constructor options and focused tests.

Fixes #996
@realfishsam

Copy link
Copy Markdown
Contributor Author

CI note: the focused static/Python compile checks pass, but the generated client sync checks are red. I inspected the failed logs; running the client-method generators would remove existing hosted-mode SDK code from client.py/client.ts (submit/cancel/fetch hosted dispatch paths, etc.) unrelated to this rate-limit accessor PR. I did not commit that broad generator drift into this focused PR. The Python exchange generator sync check is green.

@realfishsam

Copy link
Copy Markdown
Contributor Author

PR Review: FAIL

What This Does

This exposes SDK-side rate-limit controls in both TypeScript and Python clients, adds constructor options, and throttles SDK HTTP requests before the underlying sidecar/hosted request is sent. For SDK consumers, it introduces rateLimit / rate_limit inspection and runtime updates.

Blast Radius

Both SDKs are affected: TypeScript ExchangeOptions and fetch transport, Python Exchange and generated exchange constructors, plus the Python exchange generator. The core sidecar API surface and OpenAPI field propagation are not changed.

Consumer Verification

Before (base branch):
origin/main SDK clients do not expose a public SDK-side rateLimit / rate_limit constructor option or property, and the TypeScript fetchWithRetry() path calls fetch(...) directly without SDK-side delay.

After (PR branch):
PR head 88b69b08 adds:

new Polymarket({ rateLimit: 25 }).rateLimit === 25

and Python:

Polymarket(rate_limit=25).rate_limit == 25.0

Sequential SDK requests now pass through throttleHttpRequest() / _throttle_http_request() before the HTTP call. However, I found a concrete concurrency gap in the TypeScript implementation below.

Test Results

  • Build: PASS for npm run build --workspace=pmxt-core
  • Unit tests: PASS for npm test --workspace=pmxt-core -- --runInBand test/utils/throttler.test.ts; Python syntax compile PASS for changed Python files/tests.
  • SDK tests/build: BLOCKED locally: npm run build --workspace=pmxtjs fails because sdks/typescript/generated/src/index.js is absent; regenerating the generated client is blocked in this runner because java is not installed. Python pytest is also unavailable because this runner lacks pip/pytest.
  • Server starts: Not run; SDK transport/property change.
  • E2E smoke: Not fully run due missing generated SDK artifacts in this checkout.

Findings

  1. sdks/typescript/pmxt/client.ts:438-448 — TypeScript rate limiting does not serialize concurrent calls, so it does not actually guarantee the advertised “minimum delay between SDK HTTP requests.” Two SDK methods started in the same tick both enter throttleHttpRequest(), both can observe _lastHttpRequestAt === 0 (or the same stale timestamp), and both proceed to fetchWithRetry() immediately. Python uses _rate_limit_lock, but the TypeScript implementation has no equivalent mutex/promise chain. A concrete consumer failure is Promise.all([api.fetchMarkets(), api.fetchEvents()]) with rateLimit: 1000: both HTTP requests can leave together, defeating the rate-limit control the PR is adding.

PMXT Pipeline Check

  • Field propagation (3-layer): N/A
  • OpenAPI sync: N/A
  • Financial precision: N/A
  • Type safety: ISSUE — full TypeScript SDK build/test could not run in this runner because the generated client is missing; additionally, the new rate-limit contract is not covered for concurrent TS requests.
  • Auth safety: N/A

Semver Impact

minor -- this exposes new public SDK constructor options/properties and changes SDK request pacing behavior.

Risk

Even after SDK generated-client setup is fixed in the test environment, the TypeScript throttler needs an async serialization mechanism (for example a per-client promise queue/mutex, similar in effect to the Python lock) and a concurrent-request test before this can be considered safe for consumers relying on rate limiting.

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.

rateLimit getter/setter on core BaseExchange not exposed in either SDK

1 participant