feat(ratelimit): accept Redis cluster clients and restructure adapter docs - #2014
Merged
dinwwwh merged 1 commit intoSep 12, 2026
Merged
Conversation
… docs RedisRateLimiter now accepts a node-redis cluster client alongside the standalone client. The Lua script keys on a single key, so EVALSHA routes to the owning shard, and node-redis 6.2+ loads the script on every node. The rate limit docs page now leads with the adapter table in Basic Usage, keeps Blocking Mode with it, and moves Adapters to the end with one subsection and a short description per adapter instead of tabs.
More templates
@orpc/ai-sdk
@orpc/arktype
@orpc/bun
@orpc/client
@orpc/cloudflare
@orpc/contract
@orpc/experimental-effect
@orpc/evlog
@orpc/hibernation
@orpc/json-schema
@orpc/experimental-msw
@orpc/nest
@orpc/next
@orpc/node
@orpc/openapi
@orpc/opentelemetry
@orpc/pinia-colada
@orpc/pino
@orpc/publisher
@orpc/ratelimit
@orpc/server
@orpc/shared
@orpc/swr
@orpc/tanstack-query
@orpc/trpc
@orpc/valibot
@orpc/zod
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
ℹ️ Minor suggestions only.
Reviewed changes
- Redis cluster typing —
RedisRateLimiter's field and constructor parameter are widened fromRedisClientTypetoRedisClientType | RedisClusterType; purely a type change with no runtime path touched. Both types exposeisOpen/connect/scriptLoad/evalShathrough the same mixin, so the union compiles (type:checkand eslint pass locally). - Adapter docs restructure — Basic Usage now leads with an adapter table linking to per-adapter sections,
### Blocking Modemoves under Basic Usage, middleware/handler-plugin stay in the middle, and## Adaptersmoves to the end with one subsection per adapter instead of tabbed<CodeGroup>fences. All anchors resolve and the 4-backtick Upstash fence is preserved.
The type change itself is sound — on node-redis 6.2 SCRIPT LOAD is fanned out to all masters, so the cached scriptSha and the NOSCRIPT reload-retry work on a cluster. The one concern is that the docs advertise cluster support without the version caveat noted in the PR description; details inline.
openrouter/deepseek/deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
| ```ts redis | ||
| ### Redis | ||
|
|
||
| Stores counters in Redis, so every instance using the same server enforces the same limits. Works with both standalone and cluster clients. |
Contributor
There was a problem hiding this comment.
The cluster support claim here has two sharp edges the docs don't mention. On node-redis 6.0.x/6.1.x — still inside the >=6.0.0 peer range — SCRIPT LOAD isn't fanned out to every master, so the NOSCRIPT reload-retry can reload onto the wrong shard and still fail; and on a cluster, concurrent first limit() calls before connect() can fail with "The client is offline" because isOpen flips before topology discovery finishes (the example below calls connect() for this reason, but describes it as optional). Consider scoping the support claim or noting the minimum node-redis version.
Technical details
# Cluster support has undocumented caveats
## Affected sites
- `apps/content/docs/helpers/ratelimit.mdx:190` — claims "Works with both standalone and cluster clients" unconditionally
- `packages/ratelimit/src/adapters/redis.ts:123` — `executeScript` reload-retry depends on `SCRIPT LOAD` reaching the shard that owns the key
- `packages/ratelimit/package.json:69` — `"redis": ">=6.0.0"` admits versions with single-node `SCRIPT LOAD` routing
## Required outcome
- The documented cluster support is accurate across the allowed `redis` peer range, or the adapter is made shard-safe on 6.0.x/6.1.x.
## Suggested approach (optional)
- Add a short note that cluster use requires node-redis >= 6.2 (where `SCRIPT LOAD` fans out to all masters), or
- On `NOSCRIPT`, fall back to `EVAL` with the script body instead of re-running `scriptLoad` + `evalSha`: `EVAL` routes by the single key to the owning shard and loads the script there, sidestepping the per-node cache problem on every supported version.
## Open questions for the human
- Should clusters be supported across the whole `>=6.0.0` peer range, or is raising the peer floor to 6.2 acceptable?
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
1 participant
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.
RedisRateLimiternow accepts a node-redis cluster client as well as a standalone one, so it can be used against Redis Cluster without a cast. The script already keys on a single key, so requests route to the owning shard unchanged. The rate limit docs page now leads with the adapter overview and gives each adapter its own section with a short description.Docs
Testing
Caveats
SCRIPT LOADto one random node, so first requests on other shards can hit NOSCRIPT until the retry has loaded the script there. Both are still inside the>=6.0.0peer range.limit()calls beforeconnect()can fail with "The client is offline" becauseisOpenflips before topology discovery completes. Connecting first avoids it, as the docs example does.