Repository navigation
fix(runtime): drop a bridge connection whose server left its request unread - #440
Conversation
…unread A Node.js handler can stop reading its request before the end, as Next.js does with a Server Action over its size limit: its pipeline's failure destroys the request, and Node.js reads that connection no further. Where the whole upload fitted in the loopback's buffers, the bridge's client had sent all of it, so the Agent pooled the connection. The next call sent on it waited for the headers deadline (30 s), then answered 504. Since #432 the server keeps idle connections, so the server's end of each such connection was never closed either. Once the answer has been read whole, or the call has ended without it, a connection whose server has not read the request to its end is destroyed at both ends, before the Agent can pool it.
|
Arkor — Preview deployments for arkorlab-upwind Use the checkbox below to choose whether this pull request gets preview deployments.
Checking this builds the latest commit and future pushes. Unchecking stops queued and future builds; a running build can finish and an existing preview stays available. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 52 minutes. View limit details
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. |
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. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The account paying for this security review has reached its Codex usage limits. The payer can check the Codex usage dashboard. For personal accounts, using credits requires enabling “Use credits for security reviews” in Code review settings. If you do not manage the paying account, contact this repository's admins. |
[Medium impact] The PR appears safe to merge; no actionable issues were found. SummaryThe bridge now closes both ends of a connection when the server has not finished reading its request.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Response ends or client call closes] --> B{Server request complete?}
B -->|Yes| C[Keep existing connection behavior]
B -->|No| D[Destroy client and server sockets]
D --> E[Connection cannot return to the pool]
Reviews (1) · Last reviewed commit: "fix(runtime): drop a bridge connection w..." · Reviewed by Greptile |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Summary
CLOSE-WAITonce the client side had gone.How it showed up
While I triaged the 16.4.3 nightly (run 41491299214), a local
upwind startbuild ofapp-dir/actionsshowed the problem:The outer server's side of the same failure is #439.
Verification
2 new bridge tests use a handler that reads its body as Next.js's action handler does (a pipeline through a size limit). Both fail on main:
test/native/(187 tests) passes on Node.js 24.22.1 and 26.13.0.With this PR and fix(adapter): read the rest of a body its answer left unread, or close the connection #439, after 20 oversize actions in the same local build:
Summary by cubic
Fixes a bridge connection leak that made calls time out with a 504 after a handler stopped reading its request body, as Next.js does with a Server Action over its size limit.
Previously, the Agent pooled connections whose server had left part of the request unread, so the next call sent on one waited for the headers deadline and answered 504. The server's end of such a connection also stayed open, since idle connections are kept.
Now a connection whose server has not read its request to the end is destroyed at both ends, once the answer has been read whole or the call has ended without it, and before the Agent can pool it.
Bug Fixes
main.test/native/passes on Node.js 24.22.1 and 26.13.0.Written for commit b548876. Summary will update on new commits.