diff --git a/.jules/bolt.md b/.jules/bolt.md index 3adc7dd..a20da5d 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -196,3 +196,11 @@ In highly trafficked endpoints like `/v1/chat/completions`, destructuring proper Action: Replaced the object destructuring and `|| {}` fallback with direct property access after an explicit check of `req.body`. Replaced the `for...of` loop with a classic `for` loop caching the `messages.length` in a local variable `messagesLen` before the loop. This reduces garbage collection pressure and significantly improves execution speed on hot paths. + +2026-04-30 — Handle Express body-parser Client Errors +Learning: In Express.js applications using `body-parser` or `express.json()`, failing to explicitly handle specific 4xx client errors (such as `charset.unsupported`, `encoding.unsupported`, and `request.aborted`) causes them to fall through to the global error handler, resulting in 500 Internal Server Error responses and log spam, presenting a minor DoS risk. +Action: Updated the global error handler in `src/index.js` to explicitly intercept these error `.type` properties and return proper 415 or 400 JSON responses. + +2026-04-30 — Avoid Modifying Express Router Internals in Tests +Learning: When writing isolated unit tests using `supertest`, attempting to mutate `app._router.stack` to dynamically inject routes before global error handlers fails because `app._router` is lazily initialized and can be undefined at module load time. +Action: Test error handlers by constructing an isolated `mockApp` using `express()` that mirrors the production routing logic rather than mutating the internals of the exported `app`. diff --git a/src/index.js b/src/index.js index 65ccdcd..ff3411b 100644 --- a/src/index.js +++ b/src/index.js @@ -121,6 +121,9 @@ app.post('/v1/chat/completions', jsonParser, (req, res) => { res.status(200).send(payload); }); +const ERROR_UNSUPPORTED_MEDIA_TYPE = Buffer.from(JSON.stringify({ error: 'Unsupported media type' })); +const ERROR_BAD_REQUEST = Buffer.from(JSON.stringify({ error: 'Bad request' })); + // Handle invalid JSON gracefully app.use((err, req, res, next) => { if (res.headersSent) { @@ -134,6 +137,14 @@ app.use((err, req, res, next) => { res.setHeader('Content-Type', 'application/json; charset=utf-8'); return res.status(413).send(ERROR_PAYLOAD_TOO_LARGE); } + if (err.type === 'charset.unsupported' || err.type === 'encoding.unsupported') { + res.setHeader('Content-Type', 'application/json; charset=utf-8'); + return res.status(415).send(ERROR_UNSUPPORTED_MEDIA_TYPE); + } + if (err.type === 'request.aborted') { + res.setHeader('Content-Type', 'application/json; charset=utf-8'); + return res.status(400).send(ERROR_BAD_REQUEST); + } next(err); }); diff --git a/tests/api_robustness.test.js b/tests/api_robustness.test.js index 53d12d0..b776e00 100644 --- a/tests/api_robustness.test.js +++ b/tests/api_robustness.test.js @@ -1,6 +1,7 @@ const { test } = require('node:test'); const assert = require('node:assert'); const request = require('supertest'); +const express = require('express'); const { app } = require('../src/index.js'); test('POST /v1/chat/completions handles undefined req.body (e.g. non-JSON content-type)', async () => { @@ -13,6 +14,53 @@ test('POST /v1/chat/completions handles undefined req.body (e.g. non-JSON conten assert.strictEqual(res.body.error, 'Missing or invalid model'); }); +test('POST /v1/chat/completions handles unsupported charset gracefully', async () => { + const res = await request(app) + .post('/v1/chat/completions') + .set('Content-Type', 'application/json; charset=weird') + .send('{}'); + assert.strictEqual(res.status, 415); + assert.strictEqual(res.body.error, 'Unsupported media type'); +}); + +test('POST /v1/chat/completions handles unsupported encoding gracefully', async () => { + const res = await request(app) + .post('/v1/chat/completions') + .set('Content-Type', 'application/json') + .set('Content-Encoding', 'weird') + .send('{}'); + assert.strictEqual(res.status, 415); + assert.strictEqual(res.body.error, 'Unsupported media type'); +}); + +test('POST /v1/chat/completions handles aborted requests gracefully', async () => { + // Test the error handler explicitly without dealing with express routing internals. + // We can inject a route to throw the aborted error, but we need the global error + // handler to handle it. Since we can't easily alter the app stack to bypass the 404 handler, + // we will construct a mock app and use the exact same logic that we added to index.js. + // In a real environment express initialization of the _router might be deferred. + const mockApp = express(); + + mockApp.post('/trigger-aborted', (req, res, next) => { + const err = new Error('request aborted'); + err.status = 400; + err.type = 'request.aborted'; + next(err); + }); + + mockApp.use((err, req, res, next) => { + if (err.type === 'request.aborted') { + res.setHeader('Content-Type', 'application/json; charset=utf-8'); + return res.status(400).send(Buffer.from(JSON.stringify({ error: 'Bad request' }))); + } + next(err); + }); + + const res = await request(mockApp).post('/trigger-aborted').send({}); + assert.strictEqual(res.status, 400); + assert.strictEqual(res.body.error, 'Bad request'); +}); + test('POST /v1/chat/completions handles payload too large gracefully', async () => { const largeString = 'a'.repeat(11 * 1024 * 1024); // 11mb, which is > 10mb limit const res = await request(app)