Enhance error handling and rate limiting documentation #26
Conversation
…examples and improved clarity
|
Caution Review failedThe pull request is closed. WalkthroughDocumentation updates standardize error handling and rate limiting: adds an ApiError class and notFound helper, an exported AsyncHandler utility, expanded global error handler with Prisma/database error mapping and structured logging, and rate-limiting guidance including dual-keying (IP + wallet), per-route limits, Redis storage, and troubleshooting. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Router
participant AsyncHandler as AsyncHandler
participant Service as Service/DB
participant Mapper as ErrorMapper
participant Err as GlobalErrorHandler
Client->>Router: HTTP request
Router->>AsyncHandler: call handler
AsyncHandler->>Service: business logic / DB
Service-->>AsyncHandler: result or throw
alt success
AsyncHandler-->>Client: 2xx response
else error
AsyncHandler->>Mapper: normalize error (Prisma → ApiError)
Mapper-->>Err: ApiError(status, code, details)
Err-->>Client: JSON {message, code, details} with status
end
sequenceDiagram
autonumber
actor Client
participant RateLimit as RateLimitMiddleware
participant Redis as RedisStore
participant Router
participant Handler
Client->>RateLimit: incoming request
RateLimit->>RateLimit: build key (IP + wallet)
RateLimit->>Redis: increment/check window
alt within limit
RateLimit-->>Router: pass through
Router->>Handler: handle request
Handler-->>Client: response
else exceeded
RateLimit-->>Client: 429 JSON error
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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. Comment |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/rate-limiting.md (1)
88-92: Fix inaccurate claim about health endpoints not counting toward limits.The plugin counts at onRequest; it doesn’t distinguish “successful vs error” outcomes. Recommend excluding health routes from rate limiting (config.rateLimit: false) or assigning high limits.
-- Allows successful requests and errors to not count against limit +- Health routes are typically excluded from rate limiting (set `config.rateLimit: false`) or given very high limits.
🧹 Nitpick comments (11)
docs/rate-limiting.md (7)
14-18: Clarify dual-keying and proxy awareness (trustProxy).If you’re behind a reverse proxy/CDN, document enabling Fastify’s trustProxy so request.ip is correct; otherwise keys collapse to the proxy IP. Also note privacy implications of combining IP + wallet. (fastify.dev)
21-26: Type-safe key generator + proxy note.Prefer a typed example and call out trustProxy in a comment.
+import { FastifyRequest } from 'fastify' -function getRateLimitKey(req) { - const ip = req.ip; - const wallet = req.user?.walletAddress; - return wallet ? `${ip}:${wallet}` : ip; -} +export function getRateLimitKey(req: FastifyRequest): string { + const ip = req.ip // behind proxies: enable fastify({ trustProxy: true }) + const wallet = (req as any).user?.walletAddress + return wallet ? `${ip}:${wallet}` : ip +}This aligns with the plugin’s keyGenerator behavior and Fastify’s IP resolution. (github.com)
31-37: Standardize ESM/TS usage in plugin registration.Use the import(...) form (as in upstream docs) for consistency with TS examples.
-fastify.register(require('@fastify/rate-limit'), { +await fastify.register(import('@fastify/rate-limit'), { max: 100, timeWindow: '1 minute', keyGenerator: getRateLimitKey, // ...other options -}); +})
50-61: Don’t force “seconds” — use context.after as-is and add a code field.errorResponseBuilder’s context.after may already be humanized (e.g., “1 minute”). Align body with your global error format by adding code.
errorResponseBuilder: (req, context) => ({ success: false, error: 'Rate limit exceeded', - message: `Too many requests. Try again in ${context.after} seconds.`, - retryAfter: context.after + message: `Too many requests. Try again in ${context.after}.`, + code: 'RATE_LIMIT_EXCEEDED', + retryAfter: context.after })
101-108: Include the standard retry-after header.The plugin also sets retry-after; add it to the list for completeness.
- `x-ratelimit-reset`: Time when the rate limit window resets + - `retry-after`: Seconds to wait before new requests are allowed
111-120: Align body with error-handling spec.Add a code and avoid hardcoding “seconds” (see above).
{ "success": false, "error": "Rate limit exceeded", - "message": "Too many requests. Try again in X seconds.", + "code": "RATE_LIMIT_EXCEEDED", + "message": "Too many requests. Try again in X.", "retryAfter": 30 }
147-155: Redis example: import client and reuse connection.Show the Redis client import/init and reuse the instance.
- fastify.register(require('@fastify/rate-limit'), { - redis: new Redis(process.env.REDIS_URL), + import Redis from 'ioredis' + const redis = new Redis(process.env.REDIS_URL!) + await fastify.register(import('@fastify/rate-limit'), { + redis, // ...other options - }); + })Operationally consider skipOnError: true to avoid 429s if Redis is temporarily down. (github.com)
docs/error-handling.md (4)
87-92: Name consistency: AsyncHandler vs asyncHandler.Use the same casing everywhere.
-1. Route handlers use `asyncHandler` to automatically catch exceptions +1. Route handlers use `AsyncHandler` to automatically catch exceptions
121-132: Guard Prisma error handling by error type.Check PrismaClientKnownRequestError before reading .code and map P2002 explicitly.
-} catch (e) { - if (e.code === 'P2002') { - throw ApiError.conflict("Duplicate resource", "UNIQUE_CONSTRAINT_VIOLATION"); - } - throw ApiError.internal("Database error", "DATABASE_ERROR"); -} +} catch (e) { + if (e instanceof Prisma.PrismaClientKnownRequestError) { + if (e.code === 'P2002') { + throw ApiError.conflict("Duplicate resource", "UNIQUE_CONSTRAINT_VIOLATION") + } + } + throw ApiError.internal("Database error", "DATABASE_ERROR") +}
135-136: Log a request ID/correlation ID.Add req.id (and any trace ID) to logs for cross-service debugging.
140-144: Optional: include 403 Forbidden in the table.Add a 403 row to cover authorization failures.
| Scenario | Likely Cause | Resolution | |----------|-------------|------------| | 401 Unauthorized | Missing/invalid auth headers | Check `Authorization` header and signature | +| 403 Forbidden | Insufficient permissions | Verify user roles/ACLs | | 409 Conflict | Duplicate resource | Ensure unique fields (e.g., wallet) are not reused | | 429 Too Many Requests | Rate limit exceeded | Wait and retry after `retryAfter` seconds | | 500 Internal Error | Unexpected server error | Check logs for stack trace and context |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/error-handling.md(2 hunks)docs/rate-limiting.md(3 hunks)
🔇 Additional comments (2)
docs/rate-limiting.md (2)
43-46: Per-route config looks good.Route-scoped rateLimit config matches the plugin’s documented pattern.
159-167: Troubleshooting table is solid.Clear, actionable items.
| ```typescript | ||
| app.setErrorHandler((err, req, res) => { | ||
| if (err instanceof ApiError) { | ||
| // Log error with request context | ||
| logger.error({ err, path: req.url, user: req.user }, "API error"); | ||
| res.status(err.status).send({ | ||
| success: false, | ||
| error: err.message, | ||
| code: err.code, | ||
| details: err.details || undefined, | ||
| }); | ||
| } else { | ||
| // Handle unexpected errors | ||
| logger.error({ err, path: req.url }, "Unhandled error"); | ||
| res.status(500).send({ | ||
| success: false, | ||
| error: "Internal server error", | ||
| code: "INTERNAL_ERROR", | ||
| }); | ||
| } | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
Fastify error handler uses reply, not res; fix API usage.
The example is Express-like. Use Fastify’s reply and preferred naming.
-app.setErrorHandler((err, req, res) => {
+fastify.setErrorHandler((err, req, reply) => {
if (err instanceof ApiError) {
- logger.error({ err, path: req.url, user: req.user }, "API error");
- res.status(err.status).send({
+ logger.error({ err, path: req.url, user: req.user }, "API error");
+ reply.status(err.status).send({
success: false,
error: err.message,
code: err.code,
details: err.details || undefined,
});
} else {
- logger.error({ err, path: req.url }, "Unhandled error");
- res.status(500).send({
+ logger.error({ err, path: req.url }, "Unhandled error");
+ reply.status(500).send({
success: false,
error: "Internal server error",
code: "INTERNAL_ERROR",
});
}
});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In docs/error-handling.md around lines 96 to 117, the example uses an
Express-style error handler signature (err, req, res) and res methods; change it
to Fastify's signature (err, request, reply), use request instead of req for
naming, and replace res.status(...).send(...) with reply.code(...).send(...).
For ApiError use reply.code(err.status).send({...}) and for unexpected errors
use reply.code(500).send({...}); keep logging but reference request.url and
request.user consistently.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Both
error-handling.mdandrate-limiting.mdin the docs folder have been improved with more detailed information, practical code examples, production tips, and troubleshooting sections. The documentation is now more comprehensive and actionable for developers and API consumers.Summary by CodeRabbit
New Features
Documentation