fix: handle client disconnection without canceling stream

[yusukebe]

Currently, if the client disconnects, the response stream will be canceled immediately:

node-server/src/utils.ts

Line 8 in b5663eb

stream.cancel()

node-server/src/utils.ts

Line 21 in b5663eb

reader.cancel(error).catch(() => {})

I'm wondering what the correct behavior is in this situation, but it causes #239. To fix it, it should handle client disconnection gracefully.

Referring to https://github.com/hi-ogawa/reproductions/tree/main/web-to-node-handler-body-cancel, the behavior of @mjackson/node-fetch-server is different. With this PR, @hono/node-server behaves the same as it.

Fixes #239

[yusukebe]

Hi @usualoma

Sorry for pinging again. Can you review this?

[usualoma]

Hi @yusukebe

When writable is closed or an error occurs, writable.write(value) will return a falsy value. In other words, if we do not call the stream.cancel(), I think we can simplify it as follows.

diff --git i/src/utils.ts w/src/utils.ts
index c6839bc..a54a707 100644
--- i/src/utils.ts
+++ w/src/utils.ts
@@ -9,46 +9,30 @@ export function writeFromReadableStream(stream: ReadableStream<Uint8Array>, writ
   }
 
   const reader = stream.getReader()
-  let clientDisconnected = false
-
-  const handleClientDisconnect = () => {
-    clientDisconnected = true
-  }
-
   const handleError = () => {
-    clientDisconnected = true
+    // add an error handler to prevent abnormal termination, but simply ignore the error
   }
 
-  writable.on('close', handleClientDisconnect)
   writable.on('error', handleError)
 
   reader.read().then(flow, handleStreamError)
 
   return reader.closed.finally(() => {
-    writable.off('close', handleClientDisconnect)
     writable.off('error', handleError)
   })
 
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   function handleStreamError(error: any) {
-    if (!clientDisconnected) {
-      if (error) {
-        writable.destroy(error)
-      }
+    if (error) {
+      writable.destroy(error)
     }
   }
 
   function onDrain() {
-    if (!clientDisconnected) {
-      reader.read().then(flow, handleStreamError)
-    }
+    reader.read().then(flow, handleStreamError)
   }
 
   function flow({ done, value }: ReadableStreamReadResult<Uint8Array>): void | Promise<void> {
-    if (clientDisconnected) {
-      return
-    }
-
     try {
       if (done) {
         writable.end()

[usualoma]

Alternatively, if you want to ensure that reader.read() is not called again when a problem occurs with writable, you could do the following.

diff --git i/src/utils.ts w/src/utils.ts
index c6839bc..c22d09d 100644
--- i/src/utils.ts
+++ w/src/utils.ts
@@ -9,43 +9,33 @@ export function writeFromReadableStream(stream: ReadableStream<Uint8Array>, writ
   }
 
   const reader = stream.getReader()
-  let clientDisconnected = false
-
-  const handleClientDisconnect = () => {
-    clientDisconnected = true
-  }
-
   const handleError = () => {
-    clientDisconnected = true
+    // add an error handler to prevent abnormal termination, but simply ignore the error
   }
 
-  writable.on('close', handleClientDisconnect)
   writable.on('error', handleError)
 
   reader.read().then(flow, handleStreamError)
 
   return reader.closed.finally(() => {
-    writable.off('close', handleClientDisconnect)
     writable.off('error', handleError)
   })
 
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   function handleStreamError(error: any) {
-    if (!clientDisconnected) {
-      if (error) {
-        writable.destroy(error)
-      }
+    if (error) {
+      writable.destroy(error)
     }
   }
 
   function onDrain() {
-    if (!clientDisconnected) {
+    if (!writable.writable) {
       reader.read().then(flow, handleStreamError)
     }
   }
 
   function flow({ done, value }: ReadableStreamReadResult<Uint8Array>): void | Promise<void> {
-    if (clientDisconnected) {
+    if (!writable.writable) {
       return
     }

[yusukebe]

@usualoma

Thank you for the comment! I think the former is okay without preventing calling reader.read() again. Can you review this PR again?

[usualoma]

Thank you. LGTM!