Revert "Dispatch compose (nodejs#2795)"

This reverts commit 649185a.

lib/api/api-connect.js

@@ -4,7 +4,7 @@ const assert = require('node:assert')
 const { AsyncResource } = require('node:async_hooks')
 const { InvalidArgumentError, SocketError } = require('../core/errors')
 const util = require('../core/util')
-const redirect = require('../interceptor/redirect')
+const RedirectHandler = require('../handler/RedirectHandler')
 const { addSignal, removeSignal } = require('./abort-signal')
 
 class ConnectHandler extends AsyncResource {
@@ -95,9 +95,19 @@ function connect (opts, callback) {
   }
 
   try {
-    this
-      .compose(redirect(opts))
-      .dispatch({ ...opts, method: opts?.method || 'CONNECT' }, new ConnectHandler(opts, callback))
+    const connectHandler = new ConnectHandler(opts, callback)
+    const connectOptions = { ...opts, method: 'CONNECT' }
+
+    if (opts?.maxRedirections != null && (!Number.isInteger(opts?.maxRedirections) || opts?.maxRedirections < 0)) {
+      throw new InvalidArgumentError('maxRedirections must be a positive number')
+    }
+
+    if (opts?.maxRedirections > 0) {
+      RedirectHandler.buildDispatch(this, opts.maxRedirections)(connectOptions, connectHandler)
+      return
+    }
+
+    this.dispatch(connectOptions, connectHandler)
   } catch (err) {
     if (typeof callback !== 'function') {
       throw err

lib/api/api-pipeline.js

@@ -13,7 +13,7 @@ const {
   RequestAbortedError
 } = require('../core/errors')
 const util = require('../core/util')
-const redirect = require('../interceptor/redirect')
+const RedirectHandler = require('../handler/RedirectHandler')
 const { addSignal, removeSignal } = require('./abort-signal')
 
 const kResume = Symbol('resume')
@@ -243,9 +243,15 @@ function pipeline (opts, handler) {
   try {
     const pipelineHandler = new PipelineHandler(opts, handler)
 
-    this
-      .compose(redirect(opts))
-      .dispatch({ ...opts, body: pipelineHandler.req }, pipelineHandler)
+    if (opts?.maxRedirections != null && (!Number.isInteger(opts?.maxRedirections) || opts?.maxRedirections < 0)) {
+      throw new InvalidArgumentError('maxRedirections must be a positive number')
+    }
+
+    if (opts?.maxRedirections > 0) {
+      RedirectHandler.buildDispatch(this, opts.maxRedirections)({ ...opts, body: pipelineHandler.req }, pipelineHandler)
+    } else {
+      this.dispatch({ ...opts, body: pipelineHandler.req }, pipelineHandler)
+    }
 
     return pipelineHandler.ret
   } catch (err) {

lib/api/api-request.js

@@ -4,7 +4,7 @@ const assert = require('node:assert')
 const { Readable } = require('./readable')
 const { InvalidArgumentError, RequestAbortedError } = require('../core/errors')
 const util = require('../core/util')
-const redirect = require('../interceptor/redirect')
+const RedirectHandler = require('../handler/RedirectHandler')
 const { getResolveErrorBodyCallback } = require('./util')
 const { AsyncResource } = require('node:async_hooks')
 
@@ -195,9 +195,18 @@ function request (opts, callback) {
   }
 
   try {
-    this
-      .compose(redirect(opts))
-      .dispatch(opts, new RequestHandler(opts, callback))
+    const handler = new RequestHandler(opts, callback)
+
+    if (opts?.maxRedirections != null && (!Number.isInteger(opts?.maxRedirections) || opts?.maxRedirections < 0)) {
+      throw new InvalidArgumentError('maxRedirections must be a positive number')
+    }
+
+    if (opts?.maxRedirections > 0) {
+      RedirectHandler.buildDispatch(this, opts.maxRedirections)(opts, handler)
+      return
+    }
+
+    this.dispatch(opts, handler)
   } catch (err) {
     if (typeof callback !== 'function') {
       throw err

lib/api/api-stream.js

@@ -4,7 +4,7 @@ const assert = require('node:assert')
 const { finished, PassThrough } = require('node:stream')
 const { InvalidArgumentError, InvalidReturnValueError } = require('../core/errors')
 const util = require('../core/util')
-const redirect = require('../interceptor/redirect')
+const RedirectHandler = require('../handler/RedirectHandler')
 const { getResolveErrorBodyCallback } = require('./util')
 const { AsyncResource } = require('node:async_hooks')
 const { addSignal, removeSignal } = require('./abort-signal')
@@ -208,9 +208,18 @@ function stream (opts, factory, callback) {
   }
 
   try {
-    this
-      .compose(redirect(opts))
-      .dispatch(opts, new StreamHandler(opts, factory, callback))
+    const handler = new StreamHandler(opts, factory, callback)
+
+    if (opts?.maxRedirections != null && (!Number.isInteger(opts?.maxRedirections) || opts?.maxRedirections < 0)) {
+      throw new InvalidArgumentError('maxRedirections must be a positive number')
+    }
+
+    if (opts?.maxRedirections > 0) {
+      RedirectHandler.buildDispatch(this, opts.maxRedirections)(opts, handler)
+      return
+    }
+
+    this.dispatch(opts, handler)
   } catch (err) {
     if (typeof callback !== 'function') {
       throw err

lib/api/api-upgrade.js

@@ -4,7 +4,7 @@ const { InvalidArgumentError, SocketError } = require('../core/errors')
 const { AsyncResource } = require('node:async_hooks')
 const { InvalidArgumentError, SocketError } = require('../core/errors')
 const util = require('../core/util')
-const redirect = require('../interceptor/redirect')
+const RedirectHandler = require('../handler/RedirectHandler')
 const { addSignal, removeSignal } = require('./abort-signal')
 
 class UpgradeHandler extends AsyncResource {
@@ -91,9 +91,23 @@ function upgrade (opts, callback) {
   }
 
   try {
-    this
-      .compose(redirect(opts))
-      .dispatch({ ...opts, method: opts?.method || 'GET', upgrade: opts?.protocol || 'Websocket' }, new UpgradeHandler(opts, callback))
+    const upgradeHandler = new UpgradeHandler(opts, callback)
+    const upgradeOpts = {
+      ...opts,
+      method: opts.method || 'GET',
+      upgrade: opts.protocol || 'Websocket'
+    }
+
+    if (opts?.maxRedirections != null && (!Number.isInteger(opts?.maxRedirections) || opts?.maxRedirections < 0)) {
+      throw new InvalidArgumentError('maxRedirections must be a positive number')
+    }
+
+    if (opts?.maxRedirections > 0) {
+      RedirectHandler.buildDispatch(this, opts.maxRedirections)(upgradeOpts, upgradeHandler)
+      return
+    }
+
+    this.dispatch(upgradeOpts, upgradeHandler)
   } catch (err) {
     if (typeof callback !== 'function') {
       throw err

lib/dispatcher.js

@@ -0,0 +1,19 @@
+'use strict'
+
+const EventEmitter = require('node:events')
+
+class Dispatcher extends EventEmitter {
+  dispatch () {
+    throw new Error('not implemented')
+  }
+
+  close () {
+    throw new Error('not implemented')
+  }
+
+  destroy () {
+    throw new Error('not implemented')
+  }
+}
+
+module.exports = Dispatcher

lib/handler/DecoratorHandler.js

@@ -0,0 +1,31 @@
+'use strict'
+
+module.exports = class DecoratorHandler {
+  constructor (handler) {
+    this.handler = handler
+  }
+
+  onConnect (...args) {
+    return this.handler.onConnect(...args)
+  }
+
+  onError (...args) {
+    return this.handler.onError(...args)
+  }
+
+  onUpgrade (...args) {
+    return this.handler.onUpgrade(...args)
+  }
+
+  onHeaders (...args) {
+    return this.handler.onHeaders(...args)
+  }
+
+  onData (...args) {
+    return this.handler.onData(...args)
+  }
+
+  onComplete (...args) {
+    return this.handler.onComplete(...args)
+  }
+}

lib/interceptor/redirect.js → lib/handler/RedirectHandler.js

@@ -5,7 +5,6 @@ const { kBodyUsed } = require('../core/symbols')
 const assert = require('node:assert')
 const { InvalidArgumentError } = require('../core/errors')
 const EE = require('node:events')
-const Dispatcher = require('../dispatcher')
 
 const redirectableStatusCodes = [300, 301, 302, 303, 307, 308]
 
@@ -25,14 +24,27 @@ class BodyAsyncIterable {
 }
 
 class RedirectHandler {
-  constructor (dispatcher, dispatcherOpts, redirectOpts, handler) {
-    const { maxRedirections } = redirectOpts ?? {}
+  static buildDispatch (dispatcher, maxRedirections) {
+    if (maxRedirections != null && (!Number.isInteger(maxRedirections) || maxRedirections < 0)) {
+      throw new InvalidArgumentError('maxRedirections must be a positive number')
+    }
+
+    const dispatch = dispatcher.dispatch.bind(dispatcher)
+    return (opts, originalHandler) => dispatch(opts, new RedirectHandler(dispatch, maxRedirections, opts, originalHandler))
+  }
+
+  constructor (dispatch, maxRedirections, opts, handler) {
+    if (maxRedirections != null && (!Number.isInteger(maxRedirections) || maxRedirections < 0)) {
+      throw new InvalidArgumentError('maxRedirections must be a positive number')
+    }
+
+    util.validateHandler(handler, opts.method, opts.upgrade)
 
-    this.dispatcher = dispatcher
+    this.dispatch = dispatch
     this.location = null
     this.abort = null
-    this.opts = { ...dispatcherOpts } // opts must be a copy
-    this.maxRedirections = maxRedirections ?? 0
+    this.opts = { ...opts, maxRedirections: 0 } // opts must be a copy
+    this.maxRedirections = maxRedirections
     this.handler = handler
     this.history = []
     this.redirectionLimitReached = false
@@ -165,7 +177,7 @@ class RedirectHandler {
       this.location = null
       this.abort = null
 
-      this.dispatcher.dispatch(this.opts, this)
+      this.dispatch(this.opts, this)
     } else {
       this.handler.onComplete(trailers)
     }
@@ -220,38 +232,4 @@ function cleanRequestHeaders (headers, removeContent, unknownOrigin) {
   return ret
 }
 
-class RedirectDispatcher extends Dispatcher {
-  #opts
-  #dispatcher
-
-  constructor (dispatcher, opts) {
-    super()
-
-    this.#dispatcher = dispatcher
-    this.#opts = opts
-  }
-
-  dispatch (opts, handler) {
-    return this.#dispatcher.dispatch(opts, new RedirectHandler(this.#dispatcher, opts, this.#opts, handler))
-  }
-
-  close (...args) {
-    return this.#dispatcher.close(...args)
-  }
-
-  destroy (...args) {
-    return this.#dispatcher.destroy(...args)
-  }
-}
-
-module.exports = (opts) => {
-  if (opts?.maxRedirections == null || opts?.maxRedirections === 0) {
-    return null
-  }
-
-  if (!Number.isInteger(opts.maxRedirections) || opts.maxRedirections < 0) {
-    throw new InvalidArgumentError('maxRedirections must be a positive number')
-  }
-
-  return (dispatcher) => new RedirectDispatcher(dispatcher, opts)
-}
+module.exports = RedirectHandler

lib/interceptor/retry.js → lib/handler/RetryHandler.js

@@ -3,7 +3,6 @@ const assert = require('node:assert')
 const { kRetryHandlerDefaultRetry } = require('../core/symbols')
 const { RequestRetryError } = require('../core/errors')
 const { isDisturbed, parseHeaders, parseRangeHeader } = require('../core/util')
-const Dispatcher = require('../dispatcher')
 
 function calculateRetryAfterHeader (retryAfter) {
   const current = Date.now()
@@ -13,7 +12,8 @@ function calculateRetryAfterHeader (retryAfter) {
 }
 
 class RetryHandler {
-  constructor (dispatcher, dispatchOpts, retryOpts, handler) {
+  constructor (opts, handlers) {
+    const { retryOptions, ...dispatchOpts } = opts
     const {
       // Retry scoped
       retry: retryFn,
@@ -26,10 +26,10 @@ class RetryHandler {
       errorCodes,
       retryAfter,
       statusCodes
-    } = retryOpts ?? {}
+    } = retryOptions ?? {}
 
-    this.dispatcher = dispatcher
-    this.handler = handler
+    this.dispatch = handlers.dispatch
+    this.handler = handlers.handler
     this.opts = dispatchOpts
     this.abort = null
     this.aborted = false
@@ -324,38 +324,12 @@ class RetryHandler {
       }
 
       try {
-        this.dispatcher.dispatch(this.opts, this)
+        this.dispatch(this.opts, this)
       } catch (err) {
         this.handler.onError(err)
       }
     }
   }
 }
 
-class RetryDispatcher extends Dispatcher {
-  #dispatcher
-  #opts
-
-  constructor (dispatcher, opts) {
-    super()
-
-    this.#dispatcher = dispatcher
-    this.#opts = opts
-  }
-
-  dispatch (opts, handler) {
-    return this.#dispatcher.dispatch(opts, new RetryHandler(this.#dispatcher, opts, this.#opts, handler))
-  }
-
-  close (...args) {
-    return this.#dispatcher.close(...args)
-  }
-
-  destroy (...args) {
-    return this.#dispatcher.destroy(...args)
-  }
-}
-
-module.exports = (opts) => {
-  return (dispatcher) => new RetryDispatcher(dispatcher, opts)
-}
+module.exports = RetryHandler