TEST-002: Unit tests for errorHandler and rateLimit middleware#8
TEST-002: Unit tests for errorHandler and rateLimit middleware#8
Conversation
…it to jest config Co-authored-by: edinc <4791281+edinc@users.noreply.github.com>
Co-authored-by: edinc <4791281+edinc@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds Jest unit test coverage for two Express middleware modules (errorHandler and rateLimit) and adjusts the API Jest config to avoid hangs caused by a module-level timer.
Changes:
- Added comprehensive unit tests for
errorHandler(errors, response shaping, middleware behavior, async wrapper, 404 handler). - Added unit tests for
rateLimit(per-key buckets, headers, 429 behavior, skip/keyGenerator/custom handler, factory). - Updated
services/api/jest.config.jsto setforceExit: truedue to an uncleaned module-levelsetIntervalin the rate limiter.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| services/api/src/tests/errorHandler.test.ts | Adds unit tests covering error classes and error-handler middleware utilities. |
| services/api/src/tests/rateLimit.test.ts | Adds unit tests covering rate limiting behavior, headers, and configuration options. |
| services/api/jest.config.js | Forces Jest to exit to work around an open-handle timer from the rate limiter store. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * Covers: | ||
| * - Token bucket algorithm: allows up to maxRequests, blocks when exceeded, resets after window | ||
| * - Per-IP tracking: separate buckets per key | ||
| * - Rate limit exceeded: 429 status + Retry-After header |
There was a problem hiding this comment.
The file header and section labels describe the implementation as a “token bucket algorithm”, but the current middleware logic is a fixed-window counter (it increments a count until resetTime, then resets). This mismatch can mislead future maintainers about what behavior is actually being tested—please adjust the test descriptions (or the middleware docs) to match the real algorithm.
There was a problem hiding this comment.
Fixed in 2ff7b45. Updated the file header, the describe block name ('token bucket algorithm' → 'fixed-window counter algorithm'), and the inline comments inside those tests to accurately reflect the fixed-window counter behaviour of the middleware.
| // Required because RateLimitStore creates a module-level setInterval for cleanup | ||
| // that cannot be cleared from tests (defaultStore is not exported). | ||
| forceExit: true, |
There was a problem hiding this comment.
Setting forceExit: true makes the Jest run always exit even if there are still open handles, which can hide real resource leaks and cause async work to be cut off. Since the hang is caused by the module-level setInterval in RateLimitStore, it would be better to address that directly (e.g., unref() the interval or make the store/timer lifecycle controllable for tests) and remove forceExit.
| // Required because RateLimitStore creates a module-level setInterval for cleanup | |
| // that cannot be cleared from tests (defaultStore is not exported). | |
| forceExit: true, |
… counter, not token bucket) Co-authored-by: edinc <4791281+edinc@users.noreply.github.com>
Adds comprehensive unit tests for the two untested middleware modules:
errorHandler.tsandrateLimit.ts.errorHandler.test.ts (49 tests)
AppError,ValidationError,NotFoundError,UnauthorizedError: constructor args,statusCode,code,details,instanceofchaincreateErrorResponse()— response shape,requestId/path/timestamp/detailsinclusion, stack suppression in production, genericError→INTERNAL_ERRORwrappingerrorHandlerMiddleware()— status code per error type,requestId/correlationIdfallback,req.loggerdelegationasyncHandler()— forwards rejected promises tonext(), no spuriousnext()call on successnotFoundMiddleware()—NotFoundErrorwith 404, method+path in message anddetailsrateLimit.test.ts (21 tests)
maxRequests, blocks the exceeding request, resets after window expirysocket.remoteAddressfallback, customkeyGeneratorgroupingRetry-Afterheader,RATE_LIMITEDcode, customstatusCode/message/handlerX-RateLimit-Limit,X-RateLimit-Remaining(decrements, clamps at 0),X-RateLimit-Resetskippredicate andcreateRateLimiterfactoryjest.config.js
Added
forceExit: trueto prevent Jest from hanging on the module-levelsetIntervalinsideRateLimitStore(cleanup timer is never exported, so tests can't calldestroy()).Original prompt
Work item: AB#1733
Created via Azure DevOps
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.