fix: Avoid error if connection is aborted before internal request object is created (#221)

* test: add test for request abort without requestCache

Co-authored-by: Hironao OTSUBO <[email protected]>

* fix: Avoid error if connection is aborted before internal request object is created

At this time, since the user is not listening for abort events, so we should return immediately.

---------

Co-authored-by: Hironao OTSUBO <[email protected]>

Author: usualoma

src/listener.ts

@@ -1,7 +1,7 @@
 import type { IncomingMessage, ServerResponse, OutgoingHttpHeaders } from 'node:http'
 import type { Http2ServerRequest, Http2ServerResponse } from 'node:http2'
 import {
-  getAbortController,
+  abortControllerKey,
   newRequest,
   Request as LightweightRequest,
   toRequestError,
@@ -187,10 +187,15 @@ export const getRequestListener = (
 
       // Detect if request was aborted.
       outgoing.on('close', () => {
+        const abortController = req[abortControllerKey] as AbortController | undefined
+        if (!abortController) {
+          return
+        }
+
         if (incoming.errored) {
-          req[getAbortController]().abort(incoming.errored.toString())
+          req[abortControllerKey].abort(incoming.errored.toString())
         } else if (!outgoing.writableFinished) {
-          req[getAbortController]().abort('Client connection prematurely closed.')
+          req[abortControllerKey].abort('Client connection prematurely closed.')
         }
       })
 

src/request.ts

@@ -96,7 +96,7 @@ const getRequestCache = Symbol('getRequestCache')
 const requestCache = Symbol('requestCache')
 const incomingKey = Symbol('incomingKey')
 const urlKey = Symbol('urlKey')
-const abortControllerKey = Symbol('abortControllerKey')
+export const abortControllerKey = Symbol('abortControllerKey')
 export const getAbortController = Symbol('getAbortController')
 
 const requestPrototype: Record<string | symbol, any> = {

test/listener.test.ts

@@ -274,6 +274,17 @@ describe('Abort request', () => {
       }
     }
   )
+
+  it('should handle request abort without requestCache', async () => {
+    const fetchCallback = async () => {
+      // NOTE: we don't req.signal
+      await new Promise(() => {}) // never resolve
+    }
+    const requestListener = getRequestListener(fetchCallback)
+    const server = createServer(requestListener)
+    const req = request(server).post('/abort').timeout({ deadline: 1 })
+    await expect(req).rejects.toHaveProperty('timeout')
+  })
 })
 
 describe('overrideGlobalObjects', () => {

test/request.test.ts

@@ -5,6 +5,7 @@ import {
   Request as LightweightRequest,
   GlobalRequest,
   getAbortController,
+  abortControllerKey,
   RequestError,
 } from '../src/request'
 
@@ -81,6 +82,21 @@ describe('Request', () => {
       expect(z).not.toBe(y)
     })
 
+    it('should be able to safely check if an AbortController has been initialized by referencing the abortControllerKey', async () => {
+      const req = newRequest({
+        headers: {
+          host: 'localhost',
+        },
+        rawHeaders: ['host', 'localhost'],
+        url: '/foo.txt',
+      } as IncomingMessage)
+
+      expect(req[abortControllerKey]).toBeUndefined() // not initialized, do not initialize internal request object automatically
+
+      expect(req[getAbortController]()).toBeDefined()
+      expect(req[abortControllerKey]).toBeDefined() // initialized
+    })
+
     it('Should throw error if host header contains path', async () => {
       expect(() => {
         newRequest({