feat: add new dispatch compose (nodejs#2826)

* feat: add new dispatch compose * fix: review Co-authored-by: Matteo Collina <[email protected]> * revert: linting * docs: add documentation * fix: smaller tweaks to proxy interceptor * test: fix tests for proxy * refactor: expose interceptor as is * test: add testing for retry * refactor: rewrite interceptors * refactor: proxy interceptor * feat: redirect interceptor * refactor: change the compose behaviour * docs: update docs * test: add testing for compose * feat: composed dispatcher * docs: adjust documentation * refactor: apply review * docs: tweaks * feat: drop proxy --------- Co-authored-by: Matteo Collina <[email protected]>
KhafraDev · Mar 14, 2024 · fbaa00d · fbaa00d
1 parent ca561b5
commit fbaa00d
Show file tree

Hide file tree

Showing 10 changed files with 1,418 additions and 5 deletions.
diff --git a/docs/docs/api/Client.md b/docs/docs/api/Client.md
@@ -29,7 +29,8 @@ Returns: `Client`
 * **pipelining** `number | null` (optional) - Default: `1` - The amount of concurrent requests to be sent over the single TCP/TLS connection according to [RFC7230](https://tools.ietf.org/html/rfc7230#section-6.3.2). Carefully consider your workload and environment before enabling concurrent requests as pipelining may reduce performance if used incorrectly. Pipelining is sensitive to network stack settings as well as head of line blocking caused by e.g. long running requests. Set to `0` to disable keep-alive connections.
 * **connect** `ConnectOptions | Function | null` (optional) - Default: `null`.
 * **strictContentLength** `Boolean` (optional) - Default: `true` - Whether to treat request content length mismatches as errors. If true, an error is thrown when the request content-length header doesn't match the length of the request body.
-* **interceptors** `{ Client: DispatchInterceptor[] }` - Default: `[RedirectInterceptor]` - A list of interceptors that are applied to the dispatch method. Additional logic can be applied (such as, but not limited to: 302 status code handling, authentication, cookies, compression and caching). Note that the behavior of interceptors is Experimental and might change at any given time.
+  <!-- TODO: Remove once we drop its support -->
+* **interceptors** `{ Client: DispatchInterceptor[] }` - Default: `[RedirectInterceptor]` - A list of interceptors that are applied to the dispatch method. Additional logic can be applied (such as, but not limited to: 302 status code handling, authentication, cookies, compression and caching). Note that the behavior of interceptors is Experimental and might change at any given time. **Note: this is deprecated in favor of [Dispatcher#compose](./Dispatcher.md#dispatcher). Support will be droped in next major.**
 * **autoSelectFamily**: `boolean` (optional) - Default: depends on local Node version, on Node 18.13.0 and above is `false`. Enables a family autodetection algorithm that loosely implements section 5 of [RFC 8305](https://tools.ietf.org/html/rfc8305#section-5). See [here](https://nodejs.org/api/net.html#socketconnectoptions-connectlistener) for more details. This option is ignored if not supported by the current Node version.
 * **autoSelectFamilyAttemptTimeout**: `number` - Default: depends on local Node version, on Node 18.13.0 and above is `250`. The amount of time in milliseconds to wait for a connection attempt to finish before trying the next address when using the `autoSelectFamily` option. See [here](https://nodejs.org/api/net.html#socketconnectoptions-connectlistener) for more details.
 * **allowH2**: `boolean` - Default: `false`. Enables support for H2 if the server has assigned bigger priority to it through ALPN negotiation.

diff --git a/docs/docs/api/Dispatcher.md b/docs/docs/api/Dispatcher.md
@@ -817,6 +817,141 @@ try {
 }
 ```
 
+### `Dispatcher.compose(interceptors[, interceptor])`
+
+Compose a new dispatcher from the current dispatcher and the given interceptors.
+
+> _Notes_:
+> - The order of the interceptors matters. The first interceptor will be the first to be called.
+> - It is important to note that the `interceptor` function should return a function that follows the `Dispatcher.dispatch` signature.
+> - Any fork of the chain of `interceptors` can lead to unexpected results.
+
+Arguments:
+
+* **interceptors** `Interceptor[interceptor[]]`: It is an array of `Interceptor` functions passed as only argument, or several interceptors passed as separate arguments.
+
+Returns: `Dispatcher`.
+
+#### Parameter: `Interceptor`
+
+A function that takes a `dispatch` method and returns a `dispatch`-like function.
+
+#### Example 1 - Basic Compose
+
+```js
+const { Client, RedirectHandler } = require('undici')
+
+const redirectInterceptor = dispatch => {
+    return (opts, handler) => {
+      const { maxRedirections } = opts
+
+      if (!maxRedirections) {
+        return dispatch(opts, handler)
+      }
+
+      const redirectHandler = new RedirectHandler(
+        dispatch,
+        maxRedirections,
+        opts,
+        handler
+      )
+      opts = { ...opts, maxRedirections: 0 } // Stop sub dispatcher from also redirecting.
+      return dispatch(opts, redirectHandler)
+    }
+}
+
+const client = new Client('http://localhost:3000')
+  .compose(redirectInterceptor)
+
+await client.request({ path: '/', method: 'GET' })
+```
+
+#### Example 2 - Chained Compose
+
+```js
+const { Client, RedirectHandler, RetryHandler } = require('undici')
+
+const redirectInterceptor = dispatch => {
+    return (opts, handler) => {
+      const { maxRedirections } = opts
+
+      if (!maxRedirections) {
+        return dispatch(opts, handler)
+      }
+
+      const redirectHandler = new RedirectHandler(
+        dispatch,
+        maxRedirections,
+        opts,
+        handler
+      )
+      opts = { ...opts, maxRedirections: 0 }
+      return dispatch(opts, redirectHandler)
+    }
+}
+
+const retryInterceptor = dispatch => {
+  return function retryInterceptor (opts, handler) {
+    return dispatch(
+      opts,
+      new RetryHandler(opts, {
+        handler,
+        dispatch
+      })
+    )
+  }
+}
+
+const client = new Client('http://localhost:3000')
+  .compose(redirectInterceptor)
+  .compose(retryInterceptor)
+
+await client.request({ path: '/', method: 'GET' })
+```
+
+#### Pre-built interceptors
+
+##### `redirect`
+
+The `redirect` interceptor allows you to customize the way your dispatcher handles redirects.
+
+It accepts the same arguments as the [`RedirectHandler` constructor](./RedirectHandler.md).
+
+**Example - Basic Redirect Interceptor**
+
+```js
+const { Client, interceptors } = require("undici");
+const { redirect } = interceptors;
+
+const client = new Client("http://example.com").compose(
+  redirect({ maxRedirections: 3, throwOnMaxRedirects: true })
+);
+client.request({ path: "/" })
+```
+
+##### `retry`
+
+The `retry` interceptor allows you to customize the way your dispatcher handles retries.
+
+It accepts the same arguments as the [`RetryHandler` constructor](./RetryHandler.md).
+
+**Example - Basic Redirect Interceptor**
+
+```js
+const { Client, interceptors } = require("undici");
+const { retry } = interceptors;
+
+const client = new Client("http://example.com").compose(
+  retry({
+    maxRetries: 3,
+    minTimeout: 1000,
+    maxTimeout: 10000,
+    timeoutFactor: 2,
+    retryAfter: true,
+  })
+);
+```
+
 ## Instance Events
 
 ### Event: `'connect'`

diff --git a/index.js b/index.js
@@ -36,6 +36,10 @@ module.exports.RetryHandler = RetryHandler
 module.exports.DecoratorHandler = DecoratorHandler
 module.exports.RedirectHandler = RedirectHandler
 module.exports.createRedirectInterceptor = createRedirectInterceptor
+module.exports.interceptors = {
+  redirect: require('./lib/interceptor/redirect'),
+  retry: require('./lib/interceptor/retry')
+}
 
 module.exports.buildConnector = buildConnector
 module.exports.errors = errors

diff --git a/lib/dispatcher/client.js b/lib/dispatcher/client.js
@@ -59,6 +59,7 @@ const {
 } = require('../core/symbols.js')
 const connectH1 = require('./client-h1.js')
 const connectH2 = require('./client-h2.js')
+let deprecatedInterceptorWarned = false
 
 const kClosedResolve = Symbol('kClosedResolve')
 
@@ -207,9 +208,18 @@ class Client extends DispatcherBase {
       })
     }
 
-    this[kInterceptors] = interceptors?.Client && Array.isArray(interceptors.Client)
-      ? interceptors.Client
-      : [createRedirectInterceptor({ maxRedirections })]
+    if (interceptors?.Client && Array.isArray(interceptors.Client)) {
+      this[kInterceptors] = interceptors.Client
+      if (!deprecatedInterceptorWarned) {
+        deprecatedInterceptorWarned = true
+        process.emitWarning('Client.Options#interceptor is deprecated. Use Dispatcher#compose instead.', {
+          code: 'UNDICI-CLIENT-INTERCEPTOR-DEPRECATED'
+        })
+      }
+    } else {
+      this[kInterceptors] = [createRedirectInterceptor({ maxRedirections })]
+    }
+
     this[kUrl] = util.parseOrigin(url)
     this[kConnector] = connect
     this[kPipelining] = pipelining != null ? pipelining : 1

diff --git a/lib/dispatcher/dispatcher.js b/lib/dispatcher/dispatcher.js
@@ -1,5 +1,4 @@
 'use strict'
-
 const EventEmitter = require('node:events')
 
 class Dispatcher extends EventEmitter {
@@ -14,6 +13,53 @@ class Dispatcher extends EventEmitter {
   destroy () {
     throw new Error('not implemented')
   }
+
+  compose (...args) {
+    // So we handle [interceptor1, interceptor2] or interceptor1, interceptor2, ...
+    const interceptors = Array.isArray(args[0]) ? args[0] : args
+    let dispatch = this.dispatch.bind(this)
+
+    for (const interceptor of interceptors) {
+      if (interceptor == null) {
+        continue
+      }
+
+      if (typeof interceptor !== 'function') {
+        throw new TypeError(`invalid interceptor, expected function received ${typeof interceptor}`)
+      }
+
+      dispatch = interceptor(dispatch)
+
+      if (dispatch == null || typeof dispatch !== 'function' || dispatch.length !== 2) {
+        throw new TypeError('invalid interceptor')
+      }
+    }
+
+    return new ComposedDispatcher(this, dispatch)
+  }
+}
+
+class ComposedDispatcher extends Dispatcher {
+  #dispatcher = null
+  #dispatch = null
+
+  constructor (dispatcher, dispatch) {
+    super()
+    this.#dispatcher = dispatcher
+    this.#dispatch = dispatch
+  }
+
+  dispatch (...args) {
+    this.#dispatch(...args)
+  }
+
+  close (...args) {
+    return this.#dispatcher.close(...args)
+  }
+
+  destroy (...args) {
+    return this.#dispatcher.destroy(...args)
+  }
 }
 
 module.exports = Dispatcher
diff --git a/lib/interceptor/redirect.js b/lib/interceptor/redirect.js
@@ -0,0 +1,24 @@
+'use strict'
+const RedirectHandler = require('../handler/redirect-handler')
+
+module.exports = opts => {
+  const globalMaxRedirections = opts?.maxRedirections
+  return dispatch => {
+    return function redirectInterceptor (opts, handler) {
+      const { maxRedirections = globalMaxRedirections, ...baseOpts } = opts
+
+      if (!maxRedirections) {
+        return dispatch(opts, handler)
+      }
+
+      const redirectHandler = new RedirectHandler(
+        dispatch,
+        maxRedirections,
+        opts,
+        handler
+      )
+
+      return dispatch(baseOpts, redirectHandler)
+    }
+  }
+}
diff --git a/lib/interceptor/retry.js b/lib/interceptor/retry.js
@@ -0,0 +1,19 @@
+'use strict'
+const RetryHandler = require('../handler/retry-handler')
+
+module.exports = globalOpts => {
+  return dispatch => {
+    return function retryInterceptor (opts, handler) {
+      return dispatch(
+        opts,
+        new RetryHandler(
+          { ...opts, retryOptions: { ...globalOpts, ...opts.retryOptions } },
+          {
+            handler,
+            dispatch
+          }
+        )
+      )
+    }
+  }
+}
diff --git a/test/dispatcher.js b/test/dispatcher.js
@@ -20,3 +20,23 @@ test('dispatcher implementation', (t) => {
   t.throws(() => poorImplementation.close(), Error, 'throws on unimplemented close')
   t.throws(() => poorImplementation.destroy(), Error, 'throws on unimplemented destroy')
 })
+
+test('dispatcher.compose', (t) => {
+  t = tspl(t, { plan: 10 })
+
+  const dispatcher = new Dispatcher()
+  const interceptor = () => (opts, handler) => {}
+  // Should return a new dispatcher
+  t.ok(Object.getPrototypeOf(dispatcher.compose(interceptor)) instanceof Dispatcher)
+  t.ok(Object.getPrototypeOf(dispatcher.compose(interceptor, interceptor)) instanceof Dispatcher)
+  t.ok(Object.getPrototypeOf(dispatcher.compose([interceptor, interceptor])) instanceof Dispatcher)
+  t.ok(dispatcher.compose(interceptor) !== dispatcher)
+  t.throws(() => dispatcher.dispatch({}), Error, 'invalid interceptor')
+  t.throws(() => dispatcher.dispatch(() => null), Error, 'invalid interceptor')
+  t.throws(() => dispatcher.dispatch(dispatch => dispatch, () => () => {}, Error, 'invalid interceptor'))
+
+  const composed = dispatcher.compose(interceptor)
+  t.equal(typeof composed.dispatch, 'function', 'returns an object with a dispatch method')
+  t.equal(typeof composed.close, 'function', 'returns an object with a close method')
+  t.equal(typeof composed.destroy, 'function', 'returns an object with a destroy method')
+})