🐛 Bug Report — Runtime APIs: successive fetch()es fail if upstream doesn't consume body

[mrbbot]

Hey! 👋 When investigating cloudflare/workers-sdk#5095, we observed Network connection lost errors from successive calls to fetch() if the upstream didn't consume the request body. Specifically, every other request would fail. See https://github.com/mrbbot/workerd-network-connection-lost-repro for a minimal reproduction. Uncommenting this line https://github.com/mrbbot/workerd-network-connection-lost-repro/blob/04b2a464c8c269fb467ca5247efbb6da2303c9f5/user.capnp#L19 "fixes" the issue. It also seems like this line https://github.com/mrbbot/workerd-network-connection-lost-repro/blob/04b2a464c8c269fb467ca5247efbb6da2303c9f5/index.mjs#L32 needs to create a body with at least 2**16 + 1 bytes to trigger a reproduction.

This issue feels related to #1376 and #960. In #960 (comment), it was suggested that workerd should be canceling the upload in cases where a response is returned without reading the body. It feels like the same thing should be happening here.

/cc @petebacondarwin @RamIdeas

[petebacondarwin]

I also note that calling await request.body.cancel() does not "fix" the issue either. We actually have to drain the body stream.

[kentonv]

I suspect the HTTP client side thinks the connection is done and available for reuse: It has sent its entire request and received a response. However, what the client doesn't know is that the request body is still sitting in the socket buffer. On the server side, if the response completes without fully reading the request, the HTTP implementation will read and discard up to 64k, and then give up and close the connection.

I'm not entirely sure how to solve this without disabling connection reuse entirely.

Perhaps we should always send Connection: close when the response headers are sent without having read the entire request body?

(This should be fixed in KJ HTTP, probably. Not workerd.)