fix: detection of client premature close (#218)

* fix: detection of client premature close Ref: #217 Signed-off-by: Tomas Dvorak <[email protected]> * fix: improve detection of client premature close Ref: #217 Signed-off-by: Tomas Dvorak <[email protected]> * fixup! fix: improve detection of client premature close Ref: #217 Signed-off-by: Tomas Dvorak <[email protected]> * fixup! fixup! fix: improve detection of client premature close Ref: #217 Signed-off-by: Tomas Dvorak <[email protected]> --------- Signed-off-by: Tomas Dvorak <[email protected]>
honojs · Nov 12, 2024 · 2fdcdad · 2fdcdad
1 parent b13dead
commit 2fdcdad
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 60 deletions.
diff --git a/src/listener.ts b/src/listener.ts
@@ -189,6 +189,8 @@ export const getRequestListener = (
       outgoing.on('close', () => {
         if (incoming.errored) {
           req[getAbortController]().abort(incoming.errored.toString())
+        } else if (!outgoing.writableFinished) {
+          req[getAbortController]().abort('Client connection prematurely closed.')
         }
       })
 

diff --git a/test/listener.test.ts b/test/listener.test.ts
@@ -180,94 +180,100 @@ describe('Abort request', () => {
     server.close()
   })
 
-  it('should emit an abort event when the nodejs request is aborted', async () => {
-    const requests: Request[] = []
-    const abortedPromise = new Promise<void>((resolve) => {
-      onAbort = (req) => {
-        requests.push(req)
-        resolve()
-      }
-    })
-
-    const req = request(server)
-      .get('/abort')
-      .end(() => {})
-
-    await reqReadyPromise
-
-    req.abort()
-
-    await abortedPromise
-
-    expect(requests).toHaveLength(1)
-    const abortedReq = requests[0]
-    expect(abortedReq).toBeInstanceOf(Request)
-    expect(abortedReq.signal.aborted).toBe(true)
-  })
-
-  it('should emit an abort event when the nodejs request is aborted on multiple requests', async () => {
-    const requests: Request[] = []
-
-    {
+  it.each(['get', 'put', 'patch', 'delete'] as const)(
+    'should emit an abort event when the nodejs %s request is aborted',
+    async (method) => {
+      const requests: Request[] = []
       const abortedPromise = new Promise<void>((resolve) => {
         onAbort = (req) => {
           requests.push(req)
           resolve()
         }
       })
 
-      reqReadyPromise = new Promise<void>((r) => {
-        reqReadyResolve = r
-      })
-
       const req = request(server)
-        .get('/abort')
+        [method]('/abort')
         .end(() => {})
 
       await reqReadyPromise
 
       req.abort()
 
       await abortedPromise
-    }
-
-    expect(requests).toHaveLength(1)
 
-    for (const abortedReq of requests) {
+      expect(requests).toHaveLength(1)
+      const abortedReq = requests[0]
       expect(abortedReq).toBeInstanceOf(Request)
       expect(abortedReq.signal.aborted).toBe(true)
     }
+  )
 
-    {
-      const abortedPromise = new Promise<void>((resolve) => {
-        onAbort = (req) => {
-          requests.push(req)
-          resolve()
-        }
-      })
+  it.each(['get', 'post', 'head', 'patch', 'delete', 'put'] as const)(
+    'should emit an abort event when the nodejs request is aborted on multiple %s requests',
+    async (method) => {
+      const requests: Request[] = []
 
-      reqReadyPromise = new Promise<void>((r) => {
-        reqReadyResolve = r
-      })
+      {
+        const abortedPromise = new Promise<void>((resolve) => {
+          onAbort = (req) => {
+            requests.push(req)
+            resolve()
+          }
+        })
 
-      const req = request(server)
-        .get('/abort')
-        .end(() => {})
+        reqReadyPromise = new Promise<void>((r) => {
+          reqReadyResolve = r
+        })
 
-      await reqReadyPromise
+        const req = request(server)
+          [method]('/abort')
+          .end(() => {})
 
-      req.abort()
+        await reqReadyPromise
 
-      await abortedPromise
-    }
+        req.abort()
 
-    expect(requests).toHaveLength(2)
+        await abortedPromise
+      }
 
-    for (const abortedReq of requests) {
-      expect(abortedReq).toBeInstanceOf(Request)
-      expect(abortedReq.signal.aborted).toBe(true)
+      expect(requests).toHaveLength(1)
+
+      for (const abortedReq of requests) {
+        expect(abortedReq).toBeInstanceOf(Request)
+        expect(abortedReq.signal.aborted).toBe(true)
+      }
+
+      {
+        const abortedPromise = new Promise<void>((resolve) => {
+          onAbort = (req) => {
+            requests.push(req)
+            resolve()
+          }
+        })
+
+        reqReadyPromise = new Promise<void>((r) => {
+          reqReadyResolve = r
+        })
+
+        const req = request(server)
+          [method]('/abort')
+          .end(() => {})
+
+        await reqReadyPromise
+
+        req.abort()
+
+        await abortedPromise
+      }
+
+      expect(requests).toHaveLength(2)
+
+      for (const abortedReq of requests) {
+        expect(abortedReq).toBeInstanceOf(Request)
+        expect(abortedReq.signal.aborted).toBe(true)
+      }
     }
-  })
+  )
 })
 
 describe('overrideGlobalObjects', () => {