Skip to content

[TEST] Add rate-limit bypass test for health/metrics endpoints - #960

Open
dami080 wants to merge 2 commits into
ritik4ever:mainfrom
dami080:fix/904-rate-limit-bypass-test
Open

dami080 wants to merge 2 commits into
ritik4ever:mainfrom
dami080:fix/904-rate-limit-bypass-test

Conversation

@dami080

@dami080 dami080 commented Jul 28, 2026

Copy link
Copy Markdown

Overview

This PR adds comprehensive tests verifying that health endpoints (/api/health, /api/health/deep, /worker/health) and the metrics endpoint (/api/metrics) remain exempt from rate limiting under high load, while confirming regular endpoints are correctly throttled. It also moves the /api/metrics route before the readLimiter middleware and adds it to the exempt paths to close a gap where metrics were unintentionally rate-limited.

Related Issue

Closes #904

Changes

Rate-limit exemption for /api/metrics

  • [MODIFY] backend/src/utils.ts � Renamed HEALTH_PATHS to EXEMPT_PATHS and added /api/metrics to the set so the readLimiter skip function covers it
  • [MODIFY] backend/src/app.ts � Moved GET /api/metrics route definition before app.use(readLimiter) (matching the pattern used by health endpoints) so it is never subject to the global GET rate limiter

Rate-limit bypass tests

  • [REWRITE] backend/test/rateLimit.test.ts � Replaced the single passthrough test with 6 focused tests:
    • 4 bypass tests: Verifies /api/health, /api/health/deep, /worker/health, and /api/metrics all return 200 under 130 concurrent requests (exceeding the 120 req/min read limit)
    • 2 rate-limit-active tests: Confirms a regular endpoint gets 429 under the same load and passes requests below the limit

Incidental fixes

  • [FIX] backend/src/app.ts � The /api/open-issues route had an empty try {} with no catch block (syntax error). Added the proper implementation using listOpenIssues()

Verification Results

npm test -- test/rateLimit.test.ts
? 6/6 passed

Rate-limit bypass for exempt endpoints
  ? GET /api/health bypasses rate limiting under high load
  ? GET /api/health/deep bypasses rate limiting under high load
  ? GET /worker/health bypasses rate limiting under high load
  ? GET /api/metrics bypasses rate limiting under high load

Rate limiting is active for regular endpoints
  ? a regular GET endpoint is rate-limited after exceeding the limit
  ? passes requests below the rate limit successfully
Acceptance Criteria Status
Health endpoints remain responsive under load past the normal rate-limit threshold ? All 3 health endpoints return 200 at 130 req
Metrics endpoint remains responsive under load past the normal rate-limit threshold ? /api/metrics returns 200 at 130 req
A regular API endpoint is correctly rate-limited under the same load ? Returns 429 with Retry-After header
Regression guard for future exemption removal ? Tests would fail if exemption is accidentally removed

- Moved GET /api/metrics route before readLimiter middleware
- Added /api/metrics to the exempt paths set (EXEMPT_PATHS)
- Rewrote rateLimit.test.ts with 6 tests covering bypass and throttling
- Fixed pre-existing empty try block in /api/open-issues route

Closes ritik4ever#904
@vercel

vercel Bot commented Jul 28, 2026

Copy link
Copy Markdown

@dami080 is attempting to deploy a commit to the ritik4ever's projects Team on Vercel.

A member of the Team first needs to authorize it.

@drips-wave

drips-wave Bot commented Jul 28, 2026

Copy link
Copy Markdown

@dami080 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@ritik4ever

Copy link
Copy Markdown
Owner

Hi @dami080,

This PR could not be merged because it has merge conflicts with the target branch.

Please resolve the merge conflicts, push the updated changes, and the PR can be reviewed and merged.

Thank you!

1 similar comment
@ritik4ever

Copy link
Copy Markdown
Owner

Hi @dami080,

This PR could not be merged because it has merge conflicts with the target branch.

Please resolve the merge conflicts, push the updated changes, and the PR can be reviewed and merged.

Thank you!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TEST] Add rate-limit bypass test for health/metrics endpoints

2 participants