From 9507a7b2aad724e236b42a65b5b57eeb5e5c0a2a Mon Sep 17 00:00:00 2001 From: capJavert Date: Sun, 24 May 2026 21:14:50 +0000 Subject: [PATCH 1/5] fix(public-api): include statusCode in rate-limit errorResponseBuilder @fastify/rate-limit's errorResponseBuilder returns a plain object that is then thrown as the error. Without an explicit statusCode field, the global setErrorHandler (which keys off err.statusCode) treats it as an unknown error and returns 500 instead of 429. Adds 'statusCode: 429' to both IP and per-user errorResponseBuilders. Now exceeding either limit returns a proper 429 with retryAfter and the rate_limit_exceeded error code, matching what the docs promise and what consumers expect. Regression test exercises the per-user limit (61 requests > 60/min cap) and asserts the 429 status + body shape. --- __tests__/routes/public/rateLimit.ts | 23 +++++++++++++++++++++++ src/routes/public/index.ts | 7 +++++++ 2 files changed, 30 insertions(+) diff --git a/__tests__/routes/public/rateLimit.ts b/__tests__/routes/public/rateLimit.ts index 03e71d1759..3c9145c229 100644 --- a/__tests__/routes/public/rateLimit.ts +++ b/__tests__/routes/public/rateLimit.ts @@ -53,5 +53,28 @@ describe('Public API Rate Limiting', () => { expect(remaining2).toBeLessThan(remaining1); }); + + // Regression: errorResponseBuilder must include statusCode so the global + // setErrorHandler preserves 429. Without it, exceeding the limit returns + // a misleading 500. + it('should return 429 (not 500) when user rate limit is exceeded', async () => { + const token = await createTokenForUser(state.con, '5'); + + // Fire 61 requests in a row — limit is 60/min/user. + let lastRes; + for (let i = 0; i < 61; i++) { + lastRes = await request(state.app.server) + .get('/public/v1/feeds/foryou') + .set('Authorization', `Bearer ${token}`); + if (lastRes.status === 429) break; + } + + expect(lastRes!.status).toBe(429); + expect(lastRes!.body).toMatchObject({ + statusCode: 429, + error: 'rate_limit_exceeded', + message: expect.stringContaining('rate limit'), + }); + }); }); }); diff --git a/src/routes/public/index.ts b/src/routes/public/index.ts index ed64f4faf8..c31931aa37 100644 --- a/src/routes/public/index.ts +++ b/src/routes/public/index.ts @@ -138,6 +138,10 @@ export default async function ( timeWindow: '1 minute', keyGenerator: (request: FastifyRequest) => request.ip, errorResponseBuilder: () => ({ + // statusCode is required so the global setErrorHandler preserves 429. + // @fastify/rate-limit throws this body as a plain object (no Error + // class), so without an explicit statusCode it falls through to 500. + statusCode: 429, error: 'rate_limit_exceeded', message: 'Too many requests from this IP. Please slow down.', retryAfter: 60, @@ -178,6 +182,9 @@ export default async function ( keyGenerator: (request: FastifyRequest) => request.apiUserId, skip: (request: FastifyRequest) => !request.apiUserId, errorResponseBuilder: () => ({ + // statusCode is required so the global setErrorHandler preserves 429. + // See IP rate limiter above for the same workaround. + statusCode: 429, error: 'rate_limit_exceeded', message: 'User rate limit exceeded. Please slow down.', retryAfter: 60, From b9394c4c7e383db2d9c1a0b43ed3e4a1e7683208 Mon Sep 17 00:00:00 2001 From: capJavert Date: Mon, 25 May 2026 07:09:03 +0000 Subject: [PATCH 2/5] test: drop body.error assertion clobbered by global handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The global setErrorHandler rewrites the response body using 'err.name || "Error"', which drops the original 'rate_limit_exceeded' code from @fastify/rate-limit's errorResponseBuilder. Asserting on it fails. Fixing that round-trip is out of scope here — we just verify the 429 status and message string for now. --- __tests__/routes/public/rateLimit.ts | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/__tests__/routes/public/rateLimit.ts b/__tests__/routes/public/rateLimit.ts index 3c9145c229..c4ad980e7c 100644 --- a/__tests__/routes/public/rateLimit.ts +++ b/__tests__/routes/public/rateLimit.ts @@ -54,13 +54,10 @@ describe('Public API Rate Limiting', () => { expect(remaining2).toBeLessThan(remaining1); }); - // Regression: errorResponseBuilder must include statusCode so the global - // setErrorHandler preserves 429. Without it, exceeding the limit returns - // a misleading 500. - it('should return 429 (not 500) when user rate limit is exceeded', async () => { + it('should return 429 when user rate limit is exceeded', async () => { const token = await createTokenForUser(state.con, '5'); - // Fire 61 requests in a row — limit is 60/min/user. + // Limit is 60/min/user. Fire until we hit the limit. let lastRes; for (let i = 0; i < 61; i++) { lastRes = await request(state.app.server) @@ -70,11 +67,8 @@ describe('Public API Rate Limiting', () => { } expect(lastRes!.status).toBe(429); - expect(lastRes!.body).toMatchObject({ - statusCode: 429, - error: 'rate_limit_exceeded', - message: expect.stringContaining('rate limit'), - }); + expect(lastRes!.body.statusCode).toBe(429); + expect(lastRes!.body.message).toMatch(/rate limit/i); }); }); }); From cc87eafcfb1846bb7af851d9bdf4cb272958f5b7 Mon Sep 17 00:00:00 2001 From: capJavert Date: Mon, 25 May 2026 07:11:53 +0000 Subject: [PATCH 3/5] fix(errors): preserve thrown object's own 'error' field in global handler @fastify/rate-limit's errorResponseBuilder returns a plain object that is thrown verbatim, including an 'error' field naming the failure code (e.g. 'rate_limit_exceeded'). The previous handler discarded it and substituted 'Error' (because the thrown plain object has no .name). Prefer the object's own 'error' field when present, fall back to err.name, then 'Error'. Restores the documented public API response shape for 429s. --- __tests__/routes/public/rateLimit.ts | 7 +++++-- src/index.ts | 8 +++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/__tests__/routes/public/rateLimit.ts b/__tests__/routes/public/rateLimit.ts index c4ad980e7c..bd07b17c1f 100644 --- a/__tests__/routes/public/rateLimit.ts +++ b/__tests__/routes/public/rateLimit.ts @@ -67,8 +67,11 @@ describe('Public API Rate Limiting', () => { } expect(lastRes!.status).toBe(429); - expect(lastRes!.body.statusCode).toBe(429); - expect(lastRes!.body.message).toMatch(/rate limit/i); + expect(lastRes!.body).toMatchObject({ + statusCode: 429, + error: 'rate_limit_exceeded', + message: expect.stringMatching(/rate limit/i), + }); }); }); }); diff --git a/src/index.ts b/src/index.ts index e972f590ac..aa3771a091 100644 --- a/src/index.ts +++ b/src/index.ts @@ -175,9 +175,15 @@ export default async function app( req.log.warn({ err }, err.message); + // Plugins like @fastify/rate-limit throw a plain object (the return value + // of errorResponseBuilder). Prefer that object's own `error` field so the + // public response shape is preserved (e.g. 'rate_limit_exceeded'). + const errorCode = + (err as unknown as { error?: string }).error || err.name || 'Error'; + return res.code(statusCode).send({ statusCode, - error: err.name || 'Error', + error: errorCode, message: err.message, }); }); From 69748ae05545d6a99effb646eba6b87c2b6a4ce6 Mon Sep 17 00:00:00 2001 From: capJavert Date: Mon, 25 May 2026 07:27:37 +0000 Subject: [PATCH 4/5] refactor: return real Error from rate-limit builders, revert handler hack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @fastify/rate-limit's default errorResponseBuilder returns 'new Error()' with statusCode set (see index.js#L31). Our overrides were returning plain objects which lost both the Error prototype (no .name) and any non-statusCode metadata. Switch both IP and per-user builders to return a real Error with .name = 'rate_limit_exceeded' and .statusCode = 429. The global setErrorHandler then works as-is: err.statusCode preserves 429, err.name preserves the public 'rate_limit_exceeded' code in the body. Reverts the err.error workaround added in the previous commit — no longer needed once builders return proper Errors. --- src/index.ts | 8 +------- src/routes/public/index.ts | 34 +++++++++++++++++----------------- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/src/index.ts b/src/index.ts index aa3771a091..e972f590ac 100644 --- a/src/index.ts +++ b/src/index.ts @@ -175,15 +175,9 @@ export default async function app( req.log.warn({ err }, err.message); - // Plugins like @fastify/rate-limit throw a plain object (the return value - // of errorResponseBuilder). Prefer that object's own `error` field so the - // public response shape is preserved (e.g. 'rate_limit_exceeded'). - const errorCode = - (err as unknown as { error?: string }).error || err.name || 'Error'; - return res.code(statusCode).send({ statusCode, - error: errorCode, + error: err.name || 'Error', message: err.message, }); }); diff --git a/src/routes/public/index.ts b/src/routes/public/index.ts index c31931aa37..7574def9ab 100644 --- a/src/routes/public/index.ts +++ b/src/routes/public/index.ts @@ -137,15 +137,15 @@ export default async function ( max: IP_RATE_LIMIT_PER_MINUTE, timeWindow: '1 minute', keyGenerator: (request: FastifyRequest) => request.ip, - errorResponseBuilder: () => ({ - // statusCode is required so the global setErrorHandler preserves 429. - // @fastify/rate-limit throws this body as a plain object (no Error - // class), so without an explicit statusCode it falls through to 500. - statusCode: 429, - error: 'rate_limit_exceeded', - message: 'Too many requests from this IP. Please slow down.', - retryAfter: 60, - }), + errorResponseBuilder: () => { + // @fastify/rate-limit throws the return value of this builder. Return + // a proper Error so the global setErrorHandler can read err.name and + // err.statusCode (plain objects lose both). + const err = new Error('Too many requests from this IP. Please slow down.'); + err.name = 'rate_limit_exceeded'; + (err as Error & { statusCode: number }).statusCode = 429; + return err; + }, skipOnError: false, addHeadersOnExceeding: { 'x-ratelimit-limit': false, @@ -181,14 +181,14 @@ export default async function ( hook: 'preHandler', keyGenerator: (request: FastifyRequest) => request.apiUserId, skip: (request: FastifyRequest) => !request.apiUserId, - errorResponseBuilder: () => ({ - // statusCode is required so the global setErrorHandler preserves 429. - // See IP rate limiter above for the same workaround. - statusCode: 429, - error: 'rate_limit_exceeded', - message: 'User rate limit exceeded. Please slow down.', - retryAfter: 60, - }), + errorResponseBuilder: () => { + // See IP rate limiter above. Return a proper Error so name/statusCode + // survive the throw and reach the global setErrorHandler. + const err = new Error('User rate limit exceeded. Please slow down.'); + err.name = 'rate_limit_exceeded'; + (err as Error & { statusCode: number }).statusCode = 429; + return err; + }, skipOnError: false, addHeadersOnExceeding: { 'x-ratelimit-limit': true, From 9f1f62b7485454b71c53f3ee82b8d00b1d845b4a Mon Sep 17 00:00:00 2001 From: capJavert Date: Mon, 25 May 2026 11:27:04 +0200 Subject: [PATCH 5/5] fix: test --- __tests__/routes/public/rateLimit.ts | 6 ++++-- src/routes/public/index.ts | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/__tests__/routes/public/rateLimit.ts b/__tests__/routes/public/rateLimit.ts index bd07b17c1f..3fde91da4b 100644 --- a/__tests__/routes/public/rateLimit.ts +++ b/__tests__/routes/public/rateLimit.ts @@ -63,12 +63,14 @@ describe('Public API Rate Limiting', () => { lastRes = await request(state.app.server) .get('/public/v1/feeds/foryou') .set('Authorization', `Bearer ${token}`); - if (lastRes.status === 429) break; + + if (lastRes.status === 429 && lastRes.body.statusCode === 429) { + break; + } } expect(lastRes!.status).toBe(429); expect(lastRes!.body).toMatchObject({ - statusCode: 429, error: 'rate_limit_exceeded', message: expect.stringMatching(/rate limit/i), }); diff --git a/src/routes/public/index.ts b/src/routes/public/index.ts index 7574def9ab..216c8767bd 100644 --- a/src/routes/public/index.ts +++ b/src/routes/public/index.ts @@ -141,7 +141,9 @@ export default async function ( // @fastify/rate-limit throws the return value of this builder. Return // a proper Error so the global setErrorHandler can read err.name and // err.statusCode (plain objects lose both). - const err = new Error('Too many requests from this IP. Please slow down.'); + const err = new Error( + 'Too many requests from this IP. Please slow down.', + ); err.name = 'rate_limit_exceeded'; (err as Error & { statusCode: number }).statusCode = 429; return err;