feat(gateway): make request body size limit configurable via MAX_REQUEST_BODY_BYTES#253
feat(gateway): make request body size limit configurable via MAX_REQUEST_BODY_BYTES#253tlnarayana005 wants to merge 1 commit into
Conversation
…EST_BODY_BYTES Replace hardcoded 10MB body size limit in handleSummarize and CacheMiddleware with a configurable MAX_REQUEST_BODY_BYTES env var (default 10485760). Add formatBytes() for dynamic 413 error messages, unit tests, .env.example entry, and README config table row.
|
@tlnarayana005 is attempting to deploy a commit to the ankanmisra's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Hi @tlnarayana005, thanks for opening this PR. Every contribution helps MicroAI-Paygate grow. If you find the project useful, consider starring the repository — it helps others discover it. Star MicroAI-Paygate on GitHub Looking forward to reviewing this PR. |
📝 WalkthroughWalkthroughThe hardcoded 10MB request body size limit in the gateway's summarize handler and cache middleware is replaced with a configurable ChangesConfigurable Max Request Body Size
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant CacheMiddleware
participant Config
participant handleSummarize
Client->>CacheMiddleware: POST /api/ai/summarize
CacheMiddleware->>Config: getMaxRequestBodySize()
Config-->>CacheMiddleware: maxBody
CacheMiddleware->>CacheMiddleware: MaxBytesReader(body, maxBody)
alt body exceeds maxBody
CacheMiddleware-->>Client: 413 Payload too large (formatBytes(maxBody))
else within limit
CacheMiddleware->>handleSummarize: forward request
handleSummarize->>Config: getMaxRequestBodySize()
handleSummarize-->>Client: process request
end
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gateway/main.go (1)
467-481: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSet
Connection: closeon the oversized-body 413 path.
http.MaxBytesReaderwith Gin’s wrapped writer won’t reliably trigger the server-side close signal here, so this branch should mirrorCacheMiddlewareand add the header before returning 413.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@gateway/main.go` around lines 467 - 481, The oversized-body handling in the request body read branch does not set Connection: close before returning 413. Update the `requestBody == nil` / `io.ReadAll(c.Request.Body)` error path in `gateway/main.go` so that when `errors.As(err, &maxBytesErr)` matches, the response mirrors `CacheMiddleware` by explicitly adding the close header before `c.JSON(413, ...)`. Use the existing `respondError`, `getMaxRequestBodySize`, and oversized-body branch as the anchor for the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@gateway/main.go`:
- Around line 467-481: The oversized-body handling in the request body read
branch does not set Connection: close before returning 413. Update the
`requestBody == nil` / `io.ReadAll(c.Request.Body)` error path in
`gateway/main.go` so that when `errors.As(err, &maxBytesErr)` matches, the
response mirrors `CacheMiddleware` by explicitly adding the close header before
`c.JSON(413, ...)`. Use the existing `respondError`, `getMaxRequestBodySize`,
and oversized-body branch as the anchor for the fix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8715b4b8-9d91-476a-b8bd-71ffccb2a801
📒 Files selected for processing (6)
.env.exampleREADME.mdgateway/cache.gogateway/config.gogateway/config_test.gogateway/main.go
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f9374e503
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| HEALTH_CHECK_TIMEOUT_SECONDS=2 | ||
| # Maximum request body size in bytes (default: 10485760 = 10MB) | ||
| # Applies to POST /api/ai/summarize in both the handler and cache middleware. | ||
| # MAX_REQUEST_BODY_BYTES=10485760 |
There was a problem hiding this comment.
Pass MAX_REQUEST_BODY_BYTES through Compose
When users follow the documented Docker Compose flow and uncomment/set this value in the root .env, the gateway container still will not see it because the docker-compose.yml gateway environment list only passes selected variables and omits MAX_REQUEST_BODY_BYTES (checked docker-compose.yml lines 8-26). As a result Compose deployments keep using the 10MB default despite the new .env.example/README setting; add this variable to the gateway service environment so the advertised configuration works there too.
Useful? React with 👍 / 👎.
| }) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Remove the non-gofmt EOF blank line
The repository instructions require Go changes to be gofmt-clean, but this added trailing blank line makes gofmt -l gateway/config_test.go report the file and git diff --check flags new blank line at EOF. Removing the extra EOF blank line keeps the committed test file formatter-clean and avoids formatting-check churn.
Useful? React with 👍 / 👎.
| | `METRICS_PATH` | Gateway | Gateway metrics path. Default `/metrics`; values without a leading slash are normalized. | | ||
| | `ALLOWED_ORIGINS` | Gateway | Comma-separated CORS origins, no paths or query strings. | | ||
| | `TRUSTED_PROXIES` | Gateway | Comma-separated trusted proxy CIDRs for production IP handling. | | ||
| | `MAX_REQUEST_BODY_BYTES` | Gateway | Maximum request body size in bytes for `POST /api/ai/summarize`. Default `10485760` (10 MB). | |
There was a problem hiding this comment.
Keep gateway-specific docs in sync
This new root README entry documents the knob, but the gateway service README still has its own environment table and does not mention MAX_REQUEST_BODY_BYTES (checked gateway/README.md around the config table). Contributors running or deploying only gateway/ from that service README will miss the new setting, so add the same variable there to keep the service docs aligned with the changed gateway behavior.
Useful? React with 👍 / 👎.
| | `METRICS_PATH` | Gateway | Gateway metrics path. Default `/metrics`; values without a leading slash are normalized. | | ||
| | `ALLOWED_ORIGINS` | Gateway | Comma-separated CORS origins, no paths or query strings. | | ||
| | `TRUSTED_PROXIES` | Gateway | Comma-separated trusted proxy CIDRs for production IP handling. | | ||
| | `MAX_REQUEST_BODY_BYTES` | Gateway | Maximum request body size in bytes for `POST /api/ai/summarize`. Default `10485760` (10 MB). | |
There was a problem hiding this comment.
Add the new gateway limit to production env docs
The production path points operators at .env.production.example and the Render env block in DEPLOY.md, but this change only documents MAX_REQUEST_BODY_BYTES in the local example/root README. In production deployments that use those templates, raising or lowering the summarize payload limit remains undiscoverable and the gateway silently keeps the 10MB default; add the variable to the production env template/deploy block as well.
Useful? React with 👍 / 👎.
Summary
handleSummarizeandCacheMiddlewarewith a configurableMAX_REQUEST_BODY_BYTESenvironment variable (default: 10485760 bytes / 10MB).formatBytes()helper function to return clean, human-readable strings (e.g. "10MB", "512KB") for dynamic and accurate 413 Payload Too Large error responses.config_test.gocovering default values, custom limits, non-positive bounds, formatting strings, and invalid values..env.exampleand the config table inREADME.mdto document the new variable.Type Of Change
Affected Areas
gateway/)verifier/)web/)tests/,run_e2e.sh)bench/)deploy/, Docker, env, workflows)Contributor Checklist
Verification
List the exact commands you ran and their result.