Revert "Dispatch compose (#2795)"

This reverts commit 649185a.

Author: ronag

.gitignore

@@ -79,5 +79,3 @@ fuzz-results-*.json
 # Bundle output
 undici-fetch.js
 /test/imports/undici-import.js
-
-.tap

index.js

@@ -14,7 +14,12 @@ const MockClient = require('./lib/mock/mock-client')
 const MockAgent = require('./lib/mock/mock-agent')
 const MockPool = require('./lib/mock/mock-pool')
 const mockErrors = require('./lib/mock/mock-errors')
+const ProxyAgent = require('./lib/proxy-agent')
+const RetryAgent = require('./lib/retry-agent')
+const RetryHandler = require('./lib/handler/RetryHandler')
 const { getGlobalDispatcher, setGlobalDispatcher } = require('./lib/global')
+const DecoratorHandler = require('./lib/handler/DecoratorHandler')
+const RedirectHandler = require('./lib/handler/RedirectHandler')
 
 Object.assign(Dispatcher.prototype, api)
 
@@ -23,12 +28,12 @@ module.exports.Client = Client
 module.exports.Pool = Pool
 module.exports.BalancedPool = BalancedPool
 module.exports.Agent = Agent
+module.exports.ProxyAgent = ProxyAgent
+module.exports.RetryAgent = RetryAgent
+module.exports.RetryHandler = RetryHandler
 
-module.exports.interceptor = {
-  redirect: require('./lib/interceptor/redirect'),
-  retry: require('./lib/interceptor/retry'),
-  proxy: require('./lib/interceptor/proxy')
-}
+module.exports.DecoratorHandler = DecoratorHandler
+module.exports.RedirectHandler = RedirectHandler
 
 module.exports.buildConnector = buildConnector
 module.exports.errors = errors

lib/api/api-connect.js

@@ -3,7 +3,7 @@
 const { AsyncResource } = require('node:async_hooks')
 const { InvalidArgumentError, RequestAbortedError, 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 {
@@ -91,9 +91,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')
@@ -241,9 +241,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

@@ -7,7 +7,7 @@ const {
   RequestAbortedError
 } = require('../core/errors')
 const util = require('../core/util')
-const redirect = require('../interceptor/redirect')
+const RedirectHandler = require('../handler/RedirectHandler')
 const { getResolveErrorBodyCallback } = require('./util')
 const { addSignal, removeSignal } = require('./abort-signal')
 
@@ -167,9 +167,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

@@ -7,7 +7,7 @@ const {
   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')
 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 assert = require('node:assert')
 const { AsyncResource } = require('node:async_hooks')
 const { InvalidArgumentError, RequestAbortedError, 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 {
@@ -88,9 +88,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

@@ -2,11 +2,7 @@
 
 const EventEmitter = require('node:events')
 
-const kDispatcherVersion = Symbol.for('undici.dispatcher.version')
-
 class Dispatcher extends EventEmitter {
-  [kDispatcherVersion] = 1
-
   dispatch () {
     throw new Error('not implemented')
   }
@@ -18,26 +14,6 @@ class Dispatcher extends EventEmitter {
   destroy () {
     throw new Error('not implemented')
   }
-
-  compose (...interceptors) {
-    let dispatcher = this
-    for (const interceptor of interceptors) {
-      if (interceptor == null) {
-        continue
-      }
-
-      if (typeof interceptor !== 'function') {
-        throw new Error('invalid interceptor')
-      }
-
-      dispatcher = interceptor(dispatcher) ?? dispatcher
-
-      if (dispatcher[kDispatcherVersion] !== 1) {
-        throw new Error('invalid dispatcher')
-      }
-    }
-    return dispatcher
-  }
 }
 
 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 renamed to 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