fix: pass down context in onConnect (#3858)

* fix: pass down context in onConnect * tests: check if context is present --------- Co-authored-by: Carlos Fuentes <[email protected]>
nodejs · Nov 22, 2024 · a781473 · a781473
1 parent 0ea7897
commit a781473
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 18 deletions.
diff --git a/lib/handler/cache-handler.js b/lib/handler/cache-handler.js
@@ -49,14 +49,14 @@ class CacheHandler extends DecoratorHandler {
     this.#handler = handler
   }
 
-  onConnect (abort) {
+  onConnect (...args) {
     if (this.#writeStream) {
       this.#writeStream.destroy()
       this.#writeStream = undefined
     }
 
     if (typeof this.#handler.onConnect === 'function') {
-      this.#handler.onConnect(abort)
+      this.#handler.onConnect(...args)
     }
   }
 

diff --git a/lib/handler/retry-handler.js b/lib/handler/retry-handler.js
@@ -37,6 +37,7 @@ class RetryHandler {
     this.opts = { ...dispatchOpts, body: wrapRequestBody(opts.body) }
     this.abort = null
     this.aborted = false
+    this.connectCalled = false
     this.retryOpts = {
       retry: retryFn ?? RetryHandler[kRetryHandlerDefaultRetry],
       retryAfter: retryAfter ?? true,
@@ -68,16 +69,6 @@ class RetryHandler {
     this.end = null
     this.etag = null
     this.resume = null
-
-    // Handle possible onConnect duplication
-    this.handler.onConnect(reason => {
-      this.aborted = true
-      if (this.abort) {
-        this.abort(reason)
-      } else {
-        this.reason = reason
-      }
-    })
   }
 
   onRequestSent () {
@@ -92,11 +83,14 @@ class RetryHandler {
     }
   }
 
-  onConnect (abort) {
-    if (this.aborted) {
-      abort(this.reason)
-    } else {
-      this.abort = abort
+  onConnect (abort, context) {
+    this.abort = abort
+    if (!this.connectCalled) {
+      this.connectCalled = true
+      this.handler.onConnect(reason => {
+        this.aborted = true
+        this.abort(reason)
+      }, context)
     }
   }
 

diff --git a/test/interceptors/retry.js b/test/interceptors/retry.js
@@ -6,7 +6,7 @@ const { createServer } = require('node:http')
 const { once } = require('node:events')
 
 const { Client, interceptors } = require('../..')
-const { retry } = interceptors
+const { retry, redirect } = interceptors
 
 test('Should retry status code', async t => {
   t = tspl(t, { plan: 4 })
@@ -243,6 +243,41 @@ test('Should retry with defaults', async t => {
   t.equal(await response.body.text(), 'hello world!')
 })
 
+test('Should pass context from other interceptors', async t => {
+  t = tspl(t, { plan: 2 })
+
+  const server = createServer()
+  const requestOptions = {
+    method: 'GET',
+    path: '/'
+  }
+
+  server.on('request', (req, res) => {
+    res.writeHead(200)
+    res.end('hello world!')
+  })
+
+  server.listen(0)
+
+  await once(server, 'listening')
+
+  const client = new Client(
+    `http://localhost:${server.address().port}`
+  ).compose(redirect({ maxRedirections: 1 }), retry())
+
+  after(async () => {
+    await client.close()
+    server.close()
+
+    await once(server, 'close')
+  })
+
+  const response = await client.request(requestOptions)
+
+  t.equal(response.statusCode, 200)
+  t.deepStrictEqual(response.context, { history: [] })
+})
+
 test('Should handle 206 partial content', async t => {
   t = tspl(t, { plan: 5 })