diff --git a/packages/rate-limit/README.md b/packages/rate-limit/README.md index 3d2b5ab5..33201aba 100644 --- a/packages/rate-limit/README.md +++ b/packages/rate-limit/README.md @@ -253,6 +253,10 @@ const handler = createHandler({ }) ``` +A request whose handler throws is judged by the response of the error handler +that answers it. When no error handler is registered, the middleware sees no +response at all and counts the request as not successful, whatever its status. + ### `store` The `Store` to use to store the hit count for each client. diff --git a/packages/rate-limit/src/handler.ts b/packages/rate-limit/src/handler.ts index ca83cab3..a776c63d 100644 --- a/packages/rate-limit/src/handler.ts +++ b/packages/rate-limit/src/handler.ts @@ -64,6 +64,8 @@ export function createHandler(input?: OptionsInput) { options.skipFailedRequest || options.skipSuccessfulRequest ) { + // a downstream throw no error handler answered reaches this + // handler as no response at all, so it cannot be judged const wasSuccessful = response ? options.requestWasSuccessful(event, response) : false; @@ -73,7 +75,32 @@ export function createHandler(input?: OptionsInput) { (options.skipSuccessfulRequest && wasSuccessful) ) { await options.store.decrement(key); - setRequestRateLimitInfo(event, 'remaining', Math.max(maxHits - totalHits + 1, 0)); + + const remaining = Math.max(maxHits - totalHits + 1, 0); + setRequestRateLimitInfo(event, 'current', totalHits - 1); + setRequestRateLimitInfo(event, 'remaining', remaining); + + const value = String(remaining); + event.response.headers.set(HeaderName.RATE_LIMIT_REMAINING, value); + + if (!response) { + return response; + } + + try { + response.headers.set(HeaderName.RATE_LIMIT_REMAINING, value); + return response; + } catch { + // the headers of a forwarded fetch() response are immutable + const headers = new Headers(response.headers); + headers.set(HeaderName.RATE_LIMIT_REMAINING, value); + + return new Response(response.body, { + status: response.status, + statusText: response.statusText, + headers, + }); + } } } diff --git a/packages/rate-limit/test/unit/module.spec.ts b/packages/rate-limit/test/unit/module.spec.ts index a6e3fbbc..4426828d 100644 --- a/packages/rate-limit/test/unit/module.spec.ts +++ b/packages/rate-limit/test/unit/module.spec.ts @@ -2,7 +2,9 @@ import { describe, expect, it } from 'vitest'; import { App, HeaderName, + createError, defineCoreHandler, + defineErrorHandler, } from 'routup'; import { RETRY_AGAIN_MESSAGE, rateLimit } from '../../src'; @@ -57,11 +59,11 @@ describe('src/module', () => { let response = await router.fetch(createTestRequest('/')); - expect(response.headers.get(HeaderName.RATE_LIMIT_REMAINING)).toEqual('4'); + expect(response.headers.get(HeaderName.RATE_LIMIT_REMAINING)).toEqual('5'); response = await router.fetch(createTestRequest('/')); - expect(response.headers.get(HeaderName.RATE_LIMIT_REMAINING)).toEqual('4'); + expect(response.headers.get(HeaderName.RATE_LIMIT_REMAINING)).toEqual('5'); }); it('should be possible to skip failed responses', async () => { @@ -70,11 +72,11 @@ describe('src/module', () => { let response = await router.fetch(createTestRequest('/')); - expect(response.headers.get(HeaderName.RATE_LIMIT_REMAINING)).toEqual('4'); + expect(response.headers.get(HeaderName.RATE_LIMIT_REMAINING)).toEqual('5'); response = await router.fetch(createTestRequest('/')); - expect(response.headers.get(HeaderName.RATE_LIMIT_REMAINING)).toEqual('4'); + expect(response.headers.get(HeaderName.RATE_LIMIT_REMAINING)).toEqual('5'); }); it('should skip request', async () => { @@ -89,4 +91,98 @@ describe('src/module', () => { expect(response.headers.get(HeaderName.RATE_LIMIT_RESET)).toBeNull(); expect(response.headers.get(HeaderName.RETRY_AFTER)).toBeNull(); }); + + it('should judge a thrown error by the response of the error handler', async () => { + const router = new App(); + router.use(rateLimit({ + max: 1, + skipSuccessfulRequest: true, + requestWasSuccessful: (_event, response) => response.status !== 401, + })); + router.use(defineCoreHandler(() => { + throw createError({ status: 500 }); + })); + router.use(defineErrorHandler((error, event) => { + event.response.status = error.status; + return error.message; + })); + + let response = await router.fetch(createTestRequest('/')); + expect(response.status).toEqual(500); + expect(response.headers.get(HeaderName.RATE_LIMIT_REMAINING)).toEqual('1'); + + response = await router.fetch(createTestRequest('/')); + expect(response.status).toEqual(500); + }); + + it('should count a thrown error no error handler answered', async () => { + const router = new App(); + router.use(rateLimit({ + max: 1, + skipSuccessfulRequest: true, + requestWasSuccessful: (_event, response) => response.status !== 401, + })); + router.use(defineCoreHandler(() => { + throw createError({ status: 500 }); + })); + + let response = await router.fetch(createTestRequest('/')); + expect(response.status).toEqual(500); + + response = await router.fetch(createTestRequest('/')); + expect(response.status).toEqual(429); + }); + + it('should count a thrown error the predicate rejects', async () => { + const router = new App(); + router.use(rateLimit({ + max: 1, + skipSuccessfulRequest: true, + requestWasSuccessful: (_event, response) => response.status !== 401, + })); + router.use(defineCoreHandler(() => { + throw createError({ status: 401 }); + })); + router.use(defineErrorHandler((error, event) => { + event.response.status = error.status; + return error.message; + })); + + let response = await router.fetch(createTestRequest('/')); + expect(response.status).toEqual(401); + expect(response.headers.get(HeaderName.RATE_LIMIT_REMAINING)).toEqual('0'); + + response = await router.fetch(createTestRequest('/')); + expect(response.status).toEqual(429); + }); + + it('should refresh the remaining header on a response with immutable headers', async () => { + const router = new App(); + router.use(rateLimit({ skipSuccessfulRequest: true })); + router.use(defineCoreHandler(() => Response.redirect('http://localhost/target', 302))); + + const response = await router.fetch(createTestRequest('/')); + + expect(response.status).toEqual(302); + expect(response.headers.get('location')).toEqual('http://localhost/target'); + expect(response.headers.get(HeaderName.RATE_LIMIT_REMAINING)).toEqual('5'); + }); + + it('should bound concurrent failed requests by max', async () => { + const router = new App(); + let reached = 0; + router.use(rateLimit({ max: 2, skipSuccessfulRequest: true })); + router.use(defineCoreHandler(async () => { + reached++; + await new Promise((resolve) => { setTimeout(resolve, 10); }); + throw createError({ status: 401 }); + })); + + const responses = await Promise.all( + Array.from({ length: 10 }, () => router.fetch(createTestRequest('/'))), + ); + + expect(reached).toEqual(2); + expect(responses.filter((r) => r.status === 429)).toHaveLength(8); + }); });