[WRONG BRANCH] fix(server): bound raised inbound body concurrency - #482
[WRONG BRANCH] fix(server): bound raised inbound body concurrency#482luvs01 wants to merge 1 commit into
Conversation
|
✅ Deterministic PR hygiene checks passed. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 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 |
github-actions
Bot
changed the title
⏳ DRAFT
What to do
Its title has been prefixed with |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80905d11af
ℹ️ 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".
| releaseText?.(); | ||
| releaseDecoded?.(); | ||
| releaseRaw?.(); | ||
| releaseInboundCapacity(); |
There was a problem hiding this comment.
When maxInboundBodyBytes is raised and a large request finishes parsing but then waits in routing or upstream work, this finally immediately frees its entire process-wide reservation even though the returned object remains live; for example, responses/core.ts stores it in parsed._rawBody and later awaits quota and entitlement work. Subsequent requests can therefore parse and retain additional 300–512 MiB object graphs, so fast uploads followed by slow upstream calls can still accumulate multi-gigabyte bodies despite the advertised 512 MiB budget. Transfer the lease to the request lifecycle, or otherwise keep accounting for the retained parsed graph until request processing releases it.
Useful? React with 👍 / 👎.
| if (maxBytes > MAX_CONCURRENT_INBOUND_BODY_BYTES - reservedInboundBodyBytes) { | ||
| throw new InboundBodyCapacityError(); |
There was a problem hiding this comment.
When another raised-limit reader holds capacity, /v1/chat/completions and /v1/messages do not return the intended retryable 503 server_busy: readChatBody and readAnthropicBody catch this new error and wrap it as ChatCompletionsRequestError or AnthropicRequestError, whose handlers return 400. Only the Responses error decoder recognizes the class, so these clients treat temporary overload as an invalid request and may not retry. Let both wrappers preserve InboundBodyCapacityError and map it to each protocol's 503 representation.
AGENTS.md reference: src/AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
1 participant
Motivation
Description
reserveInboundBodyCapacityhelper that readers call before consuming large bodies; callers above the default cap receive a retryable503 server_busy(InboundBodyCapacityError) when the aggregate budget is exhausted.readBoundedJsonRequestBody) so each raised-limit reader reserves and releases its configured allowance; default-limit readers (<= 256 MiB) keep previous concurrency semantics.JSON.stringifymeasurement copies and using the decoded byte-length as a conservative accounting proxy for translator-budget observation.maxInboundBodyBytesto image and search handlers; restore those routes to the prior route-specific limit by using the shared reader default instead of resolving the opt-in value.server_busyresponse in the request error decoder, add a focused regression test proving concurrent raised-limit readers are serialized, and update docs to document scope and aggregate admission behavior.Testing
bun test tests/usage/request-decompress.test.ts tests/server/server-request-body-size.test.ts tests/server/server-images.test.ts tests/server/server-search.test.ts --timeout 30000and observed the affected tests pass (focused run: 149 passed for the changed areas).bun x tsc --noEmitcompleted successfully.cd docs-site && bun install --frozen-lockfile && bun run build) and the site build completed successfully.bun run privacy:scanran and passed;git diff --checkshowed no extra whitespace/errors.Note: a repository-wide parallel
bun run testwas attempted but stopped after unrelated environment/fixture issues appeared; the focused tests covering these changes are green on the repository-pinned Bun runtime used for validation.Codex Task