ReadableStream body issue since v1.14.2

[rphlmr]

👋 Hello there, I'm the maintenainer of react-router-hono-server

Since the removal of getInternalBody, responseViaResponseObject does not return a ReadableStream body through writeFromReadableStream but as a Uint8Array.

I wonder if it is expected since it was handled by getInternalBody.

node-server/src/listener.ts

Lines 142 to 160 in b5663eb

	if (
	transferEncoding ||
	contentEncoding ||
	contentLength ||
	// nginx buffering variant
	(accelBuffering && regBuffer.test(accelBuffering as string)) ||
	!regContentType.test(contentType as string)
	) {
	outgoing.writeHead(res.status, resHeaderRecord)
	flushHeaders(outgoing)
	await writeFromReadableStream(res.body, outgoing)
	} else {
	const buffer = await res.arrayBuffer()
	resHeaderRecord['content-length'] = buffer.byteLength
	outgoing.writeHead(res.status, resHeaderRecord)
	outgoing.end(new Uint8Array(buffer))
	}

This breaks React Router loader "defer" (returns an awaitable promise).

It seems to be handled by responseViaCache, so maybe we can do the same thing in responseViaResponseObject?

Maybe something like this? #259

[usualoma]

Hi @rphlmr

Thanks for the feedback.

As you know, we are currently determining whether it is a readableStream from the response header.
https://github.com/usualoma/node-server/blob/1abcca32d85896ede8864f4070768276355a9b0f/src/listener.ts#L134-L149

I don't think this is the best approach, but since res.body is always a readableStream, there is currently no other option.

In your use case, is it possible to use the response header to infer whether it is a readableStream?

[rphlmr]

Hi @usualoma
Thanks for your support!

You are right, let's try something else:
https://github.com/honojs/node-server/pull/259/files#diff-0a9945c399ef4b66209b330f33399ac079477f017039f62d33e7d8296163469dR146

What do you think about that?

(!contentLength && !etag) ||

If the response has a contentLength, should it be a readableStream (by nature, isn't it of an unknown content length?) or a buffer?

I've reverted the changes I made to the /etag/buffer test (I have removed the content-length I added), and to keep the tests passing, I've also excluded etag from being treated as readableStream.

Or, if you prefer, we could also treat text/html like text/event-stream with

const regContentType = /^(application\/json\b|text\/(?!event-stream\b|html\b))/i

[usualoma]

Hi @rphlmr
Thanks for your reply!
I think it will be different from the pull request you created, but I will look into it to see if I can solve the problem.