fix(hosting): preserve request body when a web frameworks rewrite falls through to a Cloud Function#10760
Conversation
With Web Frameworks enabled, the Hosting emulator forwards every request to the framework dev server first, which consumes the request body. When the dev server 404s and the request falls through to a function rewrite, the proxy forwarded a drained stream while still advertising the original content-length, so the function call hung until the 60s timeout (504). Buffer the body in the dev-server proxy and replay it when the request cascades to a rewrite. Requests outside Web Frameworks are unaffected. Fixes #5986
leoortizz
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where a request body is consumed by the framework dev server, causing subsequent cascaded rewrites (like Cloud Functions) to hang due to a drained stream. It introduces request body buffering in simpleProxy and exposes the buffered body on req.rawBody so it can be replayed in proxyRequestHandler. Tests are added to verify this behavior for both standard and chunked requests. Feedback on the PR points out that http.request does not automatically set the Content-Length header when a Buffer is passed to req.end(). Therefore, when deleting transfer-encoding, content-length should be explicitly set to rawBody.length.toString() to avoid falling back to chunked transfer encoding.
| * Read a request stream into a Buffer so a consumed request body can be replayed | ||
| * to a downstream handler. | ||
| */ | ||
| async function bufferRequestBody(req: IncomingMessage): Promise<Buffer> { |
There was a problem hiding this comment.
do we limit the payload size at all? could a large enough payload cause an OOM crash? I believe cloud functions limits to 10MB anyway, should we just limit if here as well for a more realistic dev experience?
There was a problem hiding this comment.
We didn’t limit it before but it’s a simple change, just implemented it. FWICT Web Frameworks’s CF is gen 2, so that is 32MB: https://docs.cloud.google.com/functions/quotas#resource_limits
| // Client aborted / stream errored mid-upload. End gracefully rather | ||
| // than hanging the response. | ||
| logger.debug("Error buffering request body:", method, path, err); | ||
| originalRes.end(); |
There was a problem hiding this comment.
What would be the originalRes.statusCode be in this case?
should it explicitly be set to something?
There was a problem hiding this comment.
Good call thanks, changed this to return 400 when the response is still writable. I previously tried setting 400 but was having an error since the response wasn't writable anymore, but a simple check fixed it.
| let rawBody: Buffer | undefined; | ||
| if (!["GET", "HEAD"].includes(method)) { | ||
| try { | ||
| rawBody = await bufferRequestBody(originalReq); |
There was a problem hiding this comment.
IIUC this will slow down every single request....could we "Tee" stream this?
There was a problem hiding this comment.
I tried it, but the results are pretty much the same across all frameworks. It'd be a rewrite of how we handle the proxy, so I wasn't sure it's worth the risk for this PR. Framework API routes are ~15% faster on large uploads, but rewrites (this PR's/issue case) behave the same.
Since it's emulator/local only and payloads are capped at 32MB, the gain is just a few ms. Happy to implement it if you'd prefer.
|
thanks! still lgtm, don't have to invest in "Tee" stream for emulator if not worth it |
4 participants
Description
Fixes #5986.
With Web Frameworks enabled, the Hosting emulator forwards every request to the framework's dev server before evaluating rewrites, and that consumes the request body. When the dev server returned a 404 and the request fell through to a
functionrewrite, the emulator forwarded an empty body while still advertising the originalcontent-length, so the function call hung until it timed out with a 504.GETrequests worked; anything with a body failed.The emulator now keeps a copy of the request body before handing the request to the dev server and replays it if the request falls through to a rewrite. Behavior outside Web Frameworks is unchanged, and this only affects the emulator — production Hosting was never affected.
Scenarios Tested
functionrewrite:POST/PUT/DELETEwith JSON, empty, chunked, and ~200KB bodies now reach the function with the body intact (previously a 60s hang → 504). Verified against the reporter's reproduction repo.+server, Astro endpoints) still receive request bodies normally.GETrewrites behave as before.Sample Commands