Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add new dispatch compose #2826

Merged
merged 25 commits into from
Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
560d35b
feat: add new dispatch compose
metcoder95 Feb 23, 2024
5337ea0
fix: review
metcoder95 Feb 25, 2024
8019e59
revert: linting
metcoder95 Feb 25, 2024
1d54170
docs: add documentation
metcoder95 Feb 25, 2024
051caa1
Merge remote-tracking branch 'origin/main' into feat/compose-dispatch
metcoder95 Feb 25, 2024
b45ecad
fix: smaller tweaks to proxy interceptor
metcoder95 Feb 28, 2024
9048eb5
test: fix tests for proxy
metcoder95 Feb 28, 2024
7656e1b
refactor: expose interceptor as is
metcoder95 Feb 29, 2024
9f0631e
Merge branch 'main' into feat/compose-dispatch
metcoder95 Feb 29, 2024
5588a1d
test: add testing for retry
metcoder95 Mar 1, 2024
04c8acb
refactor: rewrite interceptors
metcoder95 Mar 3, 2024
8ac252d
refactor: proxy interceptor
metcoder95 Mar 3, 2024
8c8c064
feat: redirect interceptor
metcoder95 Mar 3, 2024
9d1a6c0
Merge branch 'main' into feat/compose-dispatch
metcoder95 Mar 3, 2024
0fd9ea7
refactor: change the compose behaviour
metcoder95 Mar 6, 2024
53f4cfa
docs: update docs
metcoder95 Mar 6, 2024
6580d84
test: add testing for compose
metcoder95 Mar 6, 2024
1d987a2
Merge branch 'main' into feat/compose-dispatch
metcoder95 Mar 6, 2024
d66530f
feat: composed dispatcher
metcoder95 Mar 8, 2024
b932ad2
docs: adjust documentation
metcoder95 Mar 8, 2024
20d3a33
refactor: apply review
metcoder95 Mar 8, 2024
2d59acc
Merge branch 'main' into feat/compose-dispatch
metcoder95 Mar 8, 2024
2f5982e
docs: tweaks
metcoder95 Mar 8, 2024
eb065f7
Merge branch 'main' into feat/compose-dispatch
metcoder95 Mar 12, 2024
620f9bc
feat: drop proxy
metcoder95 Mar 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/docs/api/Client.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
154 changes: 154 additions & 0 deletions docs/docs/api/Dispatcher.md
Original file line number Diff line number Diff line change
Expand Up @@ -817,6 +817,160 @@ try {
}
```

### `Dispatcher.compose(interceptors[, interceptor])`
mcollina marked this conversation as resolved.
Show resolved Hide resolved

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
mcollina marked this conversation as resolved.
Show resolved Hide resolved

```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

##### `proxy`

The `proxy` interceptor allows you to connect to a proxy server before connecting to the origin server.

It accepts the same arguments as the [`ProxyAgent` constructor](./ProxyAgent.md).

**Example - Basic Proxy Interceptor**

```js
const { Client, interceptors } = require("undici");
const { proxy } = interceptors;

const client = new Client("http://example.com").compose(
proxy("http://proxy.com")
);

client.request({ path: "/" });
```

##### `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'`
Expand Down
5 changes: 5 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ module.exports.RetryHandler = RetryHandler
module.exports.DecoratorHandler = DecoratorHandler
module.exports.RedirectHandler = RedirectHandler
module.exports.createRedirectInterceptor = createRedirectInterceptor
module.exports.interceptors = {
proxy: require('./lib/interceptor/proxy'),
redirect: require('./lib/interceptor/redirect'),
retry: require('./lib/interceptor/retry')
}

module.exports.buildConnector = buildConnector
module.exports.errors = errors
Expand Down
16 changes: 13 additions & 3 deletions lib/dispatcher/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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
Expand Down
48 changes: 47 additions & 1 deletion lib/dispatcher/dispatcher.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
'use strict'

const EventEmitter = require('node:events')

class Dispatcher extends EventEmitter {
Expand All @@ -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') {
ronag marked this conversation as resolved.
Show resolved Hide resolved
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
11 changes: 11 additions & 0 deletions lib/interceptor/proxy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'use strict'
const ProxyAgent = require('../dispatcher/proxy-agent')

module.exports = opts => {
const agent = new ProxyAgent(opts)
return dispatch => {
dispatch = agent.dispatch.bind(agent)

return dispatch
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't an interceptor?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I was referring to when I suggested that ProxyAgent works differently 😅
It requires to hope over the whole dispatch, as it forwards the request to the proxy instead of the intended dispatch.
It does not respects the previous dispatcher, so if proxy is used, it should be placed at the very beginning of the chain.

I couldn't think of any other way without re-working the ProxyAgent, but totally open to suggestions

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will have to investigate this

Copy link
Member

@ronag ronag Mar 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds to me like this should not be an interceptor.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's skip this for now so we can land.

24 changes: 24 additions & 0 deletions lib/interceptor/redirect.js
Original file line number Diff line number Diff line change
@@ -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) {
ronag marked this conversation as resolved.
Show resolved Hide resolved
const { maxRedirections = globalMaxRedirections, ...baseOpts } = opts

if (!maxRedirections) {
return dispatch(opts, handler)
}

const redirectHandler = new RedirectHandler(
dispatch,
maxRedirections,
opts,
handler
)

return dispatch(baseOpts, redirectHandler)
}
}
}
19 changes: 19 additions & 0 deletions lib/interceptor/retry.js
Original file line number Diff line number Diff line change
@@ -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
}
)
)
}
}
}
20 changes: 20 additions & 0 deletions test/dispatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})