fix: handle calling writable.end() correctly #96
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi there! It looks like a bug was introduced in v1.2.2 with the addition of
writeFromReadableStream
in this PR. I was able to reproduce this on both Node v18 and v20.With fast responses on a fast connection the problem isn't as noticeable, but if you're serving large responses (particularly if the response returned from a Hono handler is a ReadableStream that's slow to enqueue chunks), then
writable.end()
inwriteFromReadableStream
is never called. This leads to responses in the browser showing as 'Pending' indefinitely.I've created a reproduction repo here that demonstrates the issue - try adding a log inside the
if (done) {...}
statement in theflow
function innode_modules/@hono/node-server/dist/index.mjs
and you'll see it never gets logged. The reproduction returns an image response as a stream, with a short artificial delay inserted before each chunk is enqueued to better reproduce the problem.The changes in this PR align more closely with this example from the Node.js documentation, where a single-use
drain
listener is registered withwritable.once
each timewritable.write
returnsfalse
. This should ensure that write calls are properly paused + resumed and the stream read to the end with the writable closed successfully.Let me know if you would prefer a different approach or if there's anything else you'd like me to add to this 🙂