feat: dogfood our aws api for ratelimiting#3124
Conversation
chronark
requested review from
MichaelUnkey,
mcstepp,
ogzhanolguncu and
perkinsjr
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
📝 WalkthroughWalkthroughThis set of changes updates the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Ratelimit
participant UnkeyAPI
Client->>Ratelimit: Request rate limit check
Ratelimit->>UnkeyAPI: Call ratelimit.limit()
UnkeyAPI-->>Ratelimit: Response or error
alt Success
Ratelimit-->>Client: Return res.data (possibly with overrideId)
else APIError
Ratelimit-->>Client: Throw formatted Error with status, message, body
else Other error
Ratelimit-->>Client: Throw generic error
end
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
packages/ratelimit/src/ratelimit.ts (1)
89-92:⚠️ Potential issueDocumentation example appears incorrect
The example for "Rejecting the request" shows the same implementation as "Letting requests pass" - both return
success: true. One of these should likely returnsuccess: false.* @example Rejecting the request * ```ts - * onError: () => ({ success: true, limit: 0, remaining: 0, reset: 0 }) + * onError: () => ({ success: false, limit: 0, remaining: 0, reset: 0 }) * ```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
packages/ratelimit/package.json(1 hunks)packages/ratelimit/src/overrides.ts(2 hunks)packages/ratelimit/src/ratelimit.ts(4 hunks)packages/ratelimit/src/types.ts(1 hunks)packages/ratelimit/tsconfig.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Test Packages / Test ./internal/encryption
- GitHub Check: Test Packages / Test ./packages/cache
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Packages / Test ./packages/nextjs
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test Go API Local / Test (Shard 1/8)
- GitHub Check: Test Packages / Test ./internal/resend
- GitHub Check: Test Go API Local / Test (Shard 4/8)
- GitHub Check: Test Packages / Test ./packages/hono
- GitHub Check: Test Packages / Test ./packages/rbac
- GitHub Check: Test Go API Local / Test (Shard 3/8)
- GitHub Check: Test Packages / Test ./internal/hash
- GitHub Check: Test Packages / Test ./apps/dashboard
- GitHub Check: Test Packages / Test ./internal/id
- GitHub Check: Test Packages / Test ./internal/billing
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
packages/ratelimit/package.json (1)
29-29:Details
❓ Verification inconclusive
Note the use of a pre-release version for a production dependency.
You've updated to a pre-release alpha version (
2.0.0-alpha.7) of@unkey/apiwithout using a caret range. While this ensures compatibility with the specific changes you've made to accommodate the new API, be aware that:
- This locks you to this exact alpha version
- Pre-release versions may contain breaking changes or unstable features
Consider whether you should:
- Pin to this exact version if you need these specific features and have tested them thoroughly
- Use a caret range once a stable 2.0.0 is released
🌐 Web query:
Is @unkey/api version 2.0.0 stable or still in alpha/beta?💡 Result:
@unkey/api version 2.0.0 is not stable and is still in alpha. The latest published version is 2.0.0-alpha.6, as indicated on the npm package page, and the maintainers explicitly state: "This SDK is not yet ready for production use" as of April 15, 2025[1][6].
Only alpha versions of 2.0.0 have been released so far (e.g., 2.0.0-alpha.1, 2.0.0-alpha.2, 2.0.0-alpha.5, 2.0.0-alpha.6), and there is no indication of a stable or general availability (GA) release[2][6].
Summary Table
Version Status Production-Ready? Release Notes/Warnings 2.0.0-alpha.6 Alpha No "Not yet ready for production use"[1][6] For production systems, you should wait for a stable (non-alpha/beta) release.
Citations:
- 1: https://npmjs.com/package/@unkey/api
- 2: https://www.npmjs.com/package/@unkey/api/v/0.10.3?activeTab=versions
- 3: https://www.unkey.com
- 4: https://github.com/unkeyed/speakeasy/blob/main/gen.yaml
- 5: https://www.unkey.com/changelog/2023-12-15
- 6: https://www.npmjs.com/package/@unkey/api
- 7: https://www.prisma.io/llms-full.txt
- 8: https://www.unkey.com/docs/api-reference/overview
- 9: https://www.unkey.com/changelog
- 10: https://tv.algora.io/algora/8540?t=83
Attention: Verify Pre-Release Dependency Usage
In
packages/ratelimit/package.jsonon line 29, you’ve pinned@unkey/apito version2.0.0-alpha.7. Note that according to the npm package page, all available versions of@unkey/apiare still in alpha—with the latest published version being2.0.0-alpha.6and a clear indication that the SDK is not yet production-ready.
- Using this pre-release dependency in production carries inherent risks, as alpha versions can include unstable or breaking changes.
- The pinned version (
alpha.7) does not match the latest published alpha (alpha.6); please verify that this discrepancy is intentional (e.g., a custom build or early access) and that it has been thoroughly tested.- Consider either sticking with the published alpha version until a stable release is available or employing a caret range once version
2.0.0reaches general availability.packages/ratelimit/src/types.ts (1)
32-36: LGTM: Appropriate type extension with proper documentation.The addition of the optional
overrideIdproperty to theRatelimitResponsetype is well-documented and maintains backward compatibility since it's optional.packages/ratelimit/tsconfig.json (1)
31-31:Details
❓ Verification inconclusive
Verify moduleResolution compatibility with consumers.
Changing from
"node"to"bundler"moduleResolution may affect how imports are resolved. The "bundler" option is relatively new in TypeScript and optimized for modern bundlers.Consider potential compatibility issues:
- Users with older TypeScript versions might encounter errors
- Module resolution behavior differences could affect package consumers
🏁 Script executed:
#!/bin/bash # Check minimum TypeScript version that supports "bundler" moduleResolution npm info typescript@* version | grep -E "4\.[7-9]\.|5\." | head -n 1Length of output: 78
Action Required – Confirm Consumer Compatibility with "bundler" Module Resolution
The updated tsconfig change sets
"moduleResolution": "bundler", which is only supported in TypeScript version 5.8.3 and above. Please verify that all consumers of this package are using TypeScript 5.8.3 or later. Also, double-check that the resolution behavior differences between"node"and"bundler"do not introduce any subtle import resolution issues for consumers.
- File: packages/ratelimit/tsconfig.json (Line 31)
- Key Point: Minimum supported TypeScript version for
"bundler"is 5.8.3 (as verified).- Action: Confirm that users with older TypeScript versions are not affected or update peer dependency requirements accordingly.
packages/ratelimit/src/overrides.ts (2)
10-10: Documentation URL updated correctly.URL has been updated from
unkey.devtoapp.unkey.comwhich aligns with the current product domain.
28-28: API property name consistently updated.The property has been correctly updated from plural
ratelimitsto singularratelimitin all four methods, aligning with the changes in the Unkey API.Also applies to: 31-31, 34-34, 37-37
packages/ratelimit/src/ratelimit.ts (4)
2-2: Added import for better error handlingThe import of
APIErrorallows for type-specific error handling in the refactored promise chain.
172-172: API method name updated from plural to singularUpdated from
ratelimitstoratelimitto align with the latest Unkey API convention.
180-191: Improved error handling with explicit promise chainThe refactored promise handling is more robust and provides better error messages by:
- Using proper promise chaining with
.then()and.catch()- Specifically handling
APIErrorinstances with detailed error information- Providing fallback error handling for other error types
This is a significant improvement over the previous implementation which likely used conditional checks on
res.error.
7-119: Documentation URLs should be updatedLine 14 references "https://unkey.dev/app/settings/root-keys" but according to the PR title "feat: dogfood our aws api for ratelimiting", you might be moving to a new API. Verify if this documentation URL is still accurate.
2 participants
Summary by CodeRabbit
Bug Fixes
New Features
Chores