feat: increase request body size limit to 50MB with verification#109
feat: increase request body size limit to 50MB with verification#109
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
llm-server/src/main.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-prettier". (The package "eslint-plugin-prettier" was not found when loaded as a Node module from the directory "/llm-server".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-prettier" was referenced from the config file in "llm-server/.eslintrc.cjs". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThe update modifies the JSON body parsing middleware in the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Middleware
Note over Server: App constructor initializes middleware
Client->>Middleware: Send request with JSON body
Middleware->>Middleware: Parse JSON (limit: 50MB)
alt Body within limit
Middleware-->>Client: Process request normally
else Body exceeds limit
Middleware-->>Client: Throw error: "Payload too large"
end
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| this.app.use( | ||
| express.json({ | ||
| limit: '50mb', | ||
| verify: (req, res, buf) => { | ||
| if (buf?.length > 50 * 1024 * 1024) { | ||
| throw new Error('Request body exceeds 50MB limit'); | ||
| } | ||
| }, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify the middleware configuration and improve error handling.
The current implementation has redundant size checks and suboptimal error handling. Consider these improvements:
+const MAX_REQUEST_BODY_SIZE = '50mb';
+
export class App {
private readonly logger = new Logger(App.name);
private app: Express;
// ...
constructor(llmProvider: LLMProvider) {
this.app = express();
- this.app.use(
- express.json({
- limit: '50mb',
- verify: (req, res, buf) => {
- if (buf?.length > 50 * 1024 * 1024) {
- throw new Error('Request body exceeds 50MB limit');
- }
- },
- }),
- );
+ this.app.use(express.json({ limit: MAX_REQUEST_BODY_SIZE }));Rationale:
- The
limitoption inexpress.json()already handles size limits and automatically sends a 413 (Payload Too Large) status code. - The verification function adds unnecessary overhead with a redundant check.
- Defining the limit as a constant improves maintainability.
3 participants
Summary by CodeRabbit