fixup

nodejs · May 7, 2024 · 1f28af4 · 1f28af4
1 parent 939cf3f
commit 1f28af4
Show file tree

Hide file tree

Showing 4 changed files with 348 additions and 343 deletions.
diff --git a/lib/api/api-request.js b/lib/api/api-request.js
@@ -2,7 +2,7 @@
 
 const assert = require('node:assert')
 const { Readable } = require('./readable')
-const { InvalidArgumentError, AbortError } = require('../core/errors')
+const { InvalidArgumentError, RequestAbortedError } = require('../core/errors')
 const util = require('../core/util')
 const { getResolveErrorBodyCallback } = require('./util')
 const { AsyncResource } = require('node:async_hooks')
@@ -57,6 +57,7 @@ class RequestHandler extends AsyncResource {
     this.highWaterMark = highWaterMark
     this.signal = signal
     this.reason = null
+    this.removeAbortListener = null
 
     if (util.isStream(body)) {
       body.on('error', (err) => {
@@ -66,10 +67,13 @@ class RequestHandler extends AsyncResource {
 
     if (this.signal) {
       if (this.signal.aborted) {
-        this.reason = this.signal.reason ?? new AbortError()
+        this.reason = this.signal.reason ?? new RequestAbortedError()
       } else {
         this.removeAbortListener = util.addAbortListener(this.signal, () => {
-          this.reason = this.signal.reason ?? new AbortError()
+          this.removeAbortListener?.()
+          this.removeAbortListener = null
+
+          this.reason = this.signal.reason ?? new RequestAbortedError()
           if (this.res) {
             util.destroy(this.res, this.reason)
           } else if (this.abort) {
@@ -152,6 +156,9 @@ class RequestHandler extends AsyncResource {
   onError (err) {
     const { res, callback, body, opaque } = this
 
+    this.removeAbortListener?.()
+    this.removeAbortListener = null
+
     if (callback) {
       // TODO: Does this need queueMicrotask?
       this.callback = null
@@ -166,8 +173,6 @@ class RequestHandler extends AsyncResource {
       queueMicrotask(() => {
         util.destroy(res, err)
       })
-    } else if (this.removeAbortListener) {
-      this.removeAbortListener()
     }
 
     if (body) {

diff --git a/test/client-request.js b/test/client-request.js
@@ -135,45 +135,45 @@ test('request hwm', async (t) => {
   await t.completed
 })
 
-// test('request abort before headers', async (t) => {
-//   t = tspl(t, { plan: 6 })
-
-//   const signal = new EE()
-//   const server = createServer((req, res) => {
-//     res.end('hello')
-//     signal.emit('abort')
-//   })
-//   after(() => server.close())
-
-//   server.listen(0, () => {
-//     const client = new Client(`http://localhost:${server.address().port}`)
-//     after(() => client.destroy())
-
-//     client[kConnect](() => {
-//       client.request({
-//         path: '/',
-//         method: 'GET',
-//         signal
-//       }, (err) => {
-//         t.ok(err instanceof errors.RequestAbortedError)
-//         t.strictEqual(signal.listenerCount('abort'), 0)
-//       })
-//       t.strictEqual(signal.listenerCount('abort'), 1)
-
-//       client.request({
-//         path: '/',
-//         method: 'GET',
-//         signal
-//       }, (err) => {
-//         t.ok(err instanceof errors.RequestAbortedError)
-//         t.strictEqual(signal.listenerCount('abort'), 0)
-//       })
-//       t.strictEqual(signal.listenerCount('abort'), 2)
-//     })
-//   })
-
-//   await t.completed
-// })
+test('request abort before headers', async (t) => {
+  t = tspl(t, { plan: 6 })
+
+  const signal = new EE()
+  const server = createServer((req, res) => {
+    res.end('hello')
+    signal.emit('abort')
+  })
+  after(() => server.close())
+
+  server.listen(0, () => {
+    const client = new Client(`http://localhost:${server.address().port}`)
+    after(() => client.destroy())
+
+    client[kConnect](() => {
+      client.request({
+        path: '/',
+        method: 'GET',
+        signal
+      }, (err) => {
+        t.ok(err instanceof errors.RequestAbortedError)
+        t.strictEqual(signal.listenerCount('abort'), 0)
+      })
+      t.strictEqual(signal.listenerCount('abort'), 1)
+
+      client.request({
+        path: '/',
+        method: 'GET',
+        signal
+      }, (err) => {
+        t.ok(err instanceof errors.RequestAbortedError)
+        t.strictEqual(signal.listenerCount('abort'), 0)
+      })
+      t.strictEqual(signal.listenerCount('abort'), 2)
+    })
+  })
+
+  await t.completed
+})
 
 test('request body destroyed on invalid callback', async (t) => {
   t = tspl(t, { plan: 1 })