diff --git a/docs/docs/api/Agent.md b/docs/docs/api/Agent.md index dd5d99bc1e1..86ce0bb62ee 100644 --- a/docs/docs/api/Agent.md +++ b/docs/docs/api/Agent.md @@ -19,8 +19,6 @@ Returns: `Agent` Extends: [`PoolOptions`](Pool.md#parameter-pooloptions) * **factory** `(origin: URL, opts: Object) => Dispatcher` - Default: `(origin, opts) => new Pool(origin, opts)` -* **maxRedirections** `Integer` - Default: `0`. The number of HTTP redirection to follow unless otherwise specified in `DispatchOptions`. -* **interceptors** `{ Agent: 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. ## Instance Properties diff --git a/docs/docs/api/Client.md b/docs/docs/api/Client.md index 03342f59959..4b193bda508 100644 --- a/docs/docs/api/Client.md +++ b/docs/docs/api/Client.md @@ -29,8 +29,6 @@ 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. **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/DispatchInterceptor.md b/docs/docs/api/DispatchInterceptor.md deleted file mode 100644 index 7dfc260e32a..00000000000 --- a/docs/docs/api/DispatchInterceptor.md +++ /dev/null @@ -1,60 +0,0 @@ -# Interface: DispatchInterceptor - -Extends: `Function` - -A function that can be applied to the `Dispatcher.Dispatch` function before it is invoked with a dispatch request. - -This allows one to write logic to intercept both the outgoing request, and the incoming response. - -### Parameter: `Dispatcher.Dispatch` - -The base dispatch function you are decorating. - -### ReturnType: `Dispatcher.Dispatch` - -A dispatch function that has been altered to provide additional logic - -### Basic Example - -Here is an example of an interceptor being used to provide a JWT bearer token - -```js -'use strict' - -const insertHeaderInterceptor = dispatch => { - return function InterceptedDispatch(opts, handler){ - opts.headers.push('Authorization', 'Bearer [Some token]') - return dispatch(opts, handler) - } -} - -const client = new Client('https://localhost:3000', { - interceptors: { Client: [insertHeaderInterceptor] } -}) - -``` - -### Basic Example 2 - -Here is a contrived example of an interceptor stripping the headers from a response. - -```js -'use strict' - -const clearHeadersInterceptor = dispatch => { - const { DecoratorHandler } = require('undici') - class ResultInterceptor extends DecoratorHandler { - onHeaders (statusCode, headers, resume) { - return super.onHeaders(statusCode, [], resume) - } - } - return function InterceptedDispatch(opts, handler){ - return dispatch(opts, new ResultInterceptor(handler)) - } -} - -const client = new Client('https://localhost:3000', { - interceptors: { Client: [clearHeadersInterceptor] } -}) - -``` diff --git a/docs/docs/api/Pool.md b/docs/docs/api/Pool.md index 8fcabac3154..6b08294b61c 100644 --- a/docs/docs/api/Pool.md +++ b/docs/docs/api/Pool.md @@ -19,7 +19,6 @@ Extends: [`ClientOptions`](Client.md#parameter-clientoptions) * **factory** `(origin: URL, opts: Object) => Dispatcher` - Default: `(origin, opts) => new Client(origin, opts)` * **connections** `number | null` (optional) - Default: `null` - The number of `Client` instances to create. When set to `null`, the `Pool` instance will create an unlimited amount of `Client` instances. -* **interceptors** `{ Pool: DispatchInterceptor[] } }` - Default: `{ Pool: [] }` - 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). ## Instance Properties diff --git a/lib/api/api-connect.js b/lib/api/api-connect.js index f9dbbf64fe8..1f1f5e6aaa4 100644 --- a/lib/api/api-connect.js +++ b/lib/api/api-connect.js @@ -4,6 +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 RedirectHandler = require('../handler/RedirectHandler') const { addSignal, removeSignal } = require('./abort-signal') class ConnectHandler extends AsyncResource { @@ -95,7 +96,18 @@ function connect (opts, callback) { try { const connectHandler = new ConnectHandler(opts, callback) - this.dispatch({ ...opts, method: 'CONNECT' }, connectHandler) + 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 diff --git a/lib/api/api-pipeline.js b/lib/api/api-pipeline.js index 322334e7848..7ebda7e54fd 100644 --- a/lib/api/api-pipeline.js +++ b/lib/api/api-pipeline.js @@ -5,15 +5,16 @@ const { Duplex, PassThrough } = require('node:stream') +const assert = require('node:assert') +const { AsyncResource } = require('node:async_hooks') const { InvalidArgumentError, InvalidReturnValueError, RequestAbortedError } = require('../core/errors') const util = require('../core/util') -const { AsyncResource } = require('node:async_hooks') +const RedirectHandler = require('../handler/RedirectHandler') const { addSignal, removeSignal } = require('./abort-signal') -const assert = require('node:assert') const kResume = Symbol('resume') @@ -240,7 +241,17 @@ class PipelineHandler extends AsyncResource { function pipeline (opts, handler) { try { const pipelineHandler = new PipelineHandler(opts, handler) - this.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) { return new PassThrough().destroy(err) diff --git a/lib/api/api-request.js b/lib/api/api-request.js index 31cc3e5cd00..937cc58e528 100644 --- a/lib/api/api-request.js +++ b/lib/api/api-request.js @@ -1,11 +1,14 @@ 'use strict' +const { AsyncResource } = require('node:async_hooks') const assert = require('node:assert') -const { Readable } = require('./readable') + const { InvalidArgumentError, RequestAbortedError } = require('../core/errors') const util = require('../core/util') +const RedirectHandler = require('../handler/RedirectHandler') + +const { Readable } = require('./readable') const { getResolveErrorBodyCallback } = require('./util') -const { AsyncResource } = require('node:async_hooks') class RequestHandler extends AsyncResource { constructor (opts, callback) { @@ -189,7 +192,18 @@ function request (opts, callback) { } try { - this.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 diff --git a/lib/api/api-stream.js b/lib/api/api-stream.js index fba2266dd7d..cb1a8de6320 100644 --- a/lib/api/api-stream.js +++ b/lib/api/api-stream.js @@ -4,6 +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 RedirectHandler = require('../handler/RedirectHandler') const { getResolveErrorBodyCallback } = require('./util') const { AsyncResource } = require('node:async_hooks') const { addSignal, removeSignal } = require('./abort-signal') @@ -207,7 +208,18 @@ function stream (opts, factory, callback) { } try { - this.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 diff --git a/lib/api/api-upgrade.js b/lib/api/api-upgrade.js index 019fe1efa2d..10d32b7dc83 100644 --- a/lib/api/api-upgrade.js +++ b/lib/api/api-upgrade.js @@ -1,10 +1,11 @@ 'use strict' -const { InvalidArgumentError, SocketError } = require('../core/errors') +const assert = require('node:assert') const { AsyncResource } = require('node:async_hooks') +const { InvalidArgumentError, SocketError } = require('../core/errors') const util = require('../core/util') +const RedirectHandler = require('../handler/RedirectHandler') const { addSignal, removeSignal } = require('./abort-signal') -const assert = require('node:assert') class UpgradeHandler extends AsyncResource { constructor (opts, callback) { @@ -91,11 +92,22 @@ function upgrade (opts, callback) { try { const upgradeHandler = new UpgradeHandler(opts, callback) - this.dispatch({ + const upgradeOpts = { ...opts, method: opts.method || 'GET', upgrade: opts.protocol || 'Websocket' - }, upgradeHandler) + } + + 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 diff --git a/lib/core/symbols.js b/lib/core/symbols.js index c8ba5dd8ec5..1c3732620b1 100644 --- a/lib/core/symbols.js +++ b/lib/core/symbols.js @@ -52,7 +52,6 @@ module.exports = { kMaxRequests: Symbol('maxRequestsPerClient'), kProxy: Symbol('proxy agent options'), kCounter: Symbol('socket request counter'), - kInterceptors: Symbol('dispatch interceptors'), kMaxResponseSize: Symbol('max response size'), kHTTP2Session: Symbol('http2Session'), kHTTP2SessionState: Symbol('http2Session state'), diff --git a/lib/dispatcher/agent.js b/lib/dispatcher/agent.js index 98f1486cac0..bc9e3d80abf 100644 --- a/lib/dispatcher/agent.js +++ b/lib/dispatcher/agent.js @@ -1,17 +1,15 @@ 'use strict' -const { InvalidArgumentError } = require('../core/errors') -const { kClients, kRunning, kClose, kDestroy, kDispatch, kInterceptors } = require('../core/symbols') +const { InvalidArgumentError } = require('./core/errors') +const { kClients, kRunning, kClose, kDestroy, kDispatch } = require('./core/symbols') const DispatcherBase = require('./dispatcher-base') const Pool = require('./pool') const Client = require('./client') -const util = require('../core/util') -const createRedirectInterceptor = require('../interceptor/redirect-interceptor') +const util = require('./core/util') const kOnConnect = Symbol('onConnect') const kOnDisconnect = Symbol('onDisconnect') const kOnConnectionError = Symbol('onConnectionError') -const kMaxRedirections = Symbol('maxRedirections') const kOnDrain = Symbol('onDrain') const kFactory = Symbol('factory') const kOptions = Symbol('options') @@ -23,7 +21,7 @@ function defaultFactory (origin, opts) { } class Agent extends DispatcherBase { - constructor ({ factory = defaultFactory, maxRedirections = 0, connect, ...options } = {}) { + constructor ({ factory = defaultFactory, connect, ...options } = {}) { super() if (typeof factory !== 'function') { @@ -34,23 +32,11 @@ class Agent extends DispatcherBase { throw new InvalidArgumentError('connect must be a function or an object') } - if (!Number.isInteger(maxRedirections) || maxRedirections < 0) { - throw new InvalidArgumentError('maxRedirections must be a positive number') - } - if (connect && typeof connect !== 'function') { connect = { ...connect } } - this[kInterceptors] = options.interceptors?.Agent && Array.isArray(options.interceptors.Agent) - ? options.interceptors.Agent - : [createRedirectInterceptor({ maxRedirections })] - this[kOptions] = { ...util.deepClone(options), connect } - this[kOptions].interceptors = options.interceptors - ? { ...options.interceptors } - : undefined - this[kMaxRedirections] = maxRedirections this[kFactory] = factory this[kClients] = new Map() diff --git a/lib/dispatcher/balanced-pool.js b/lib/dispatcher/balanced-pool.js index 15a7e7b5879..40bf75d1622 100644 --- a/lib/dispatcher/balanced-pool.js +++ b/lib/dispatcher/balanced-pool.js @@ -13,8 +13,8 @@ const { kGetDispatcher } = require('./pool-base') const Pool = require('./pool') -const { kUrl, kInterceptors } = require('../core/symbols') -const { parseOrigin } = require('../core/util') +const { kUrl } = require('./core/symbols') +const { parseOrigin } = require('./core/util') const kFactory = Symbol('factory') const kOptions = Symbol('options') @@ -53,9 +53,6 @@ class BalancedPool extends PoolBase { throw new InvalidArgumentError('factory must be a function.') } - this[kInterceptors] = opts.interceptors?.BalancedPool && Array.isArray(opts.interceptors.BalancedPool) - ? opts.interceptors.BalancedPool - : [] this[kFactory] = factory for (const upstream of upstreams) { diff --git a/lib/dispatcher/dispatcher-base.js b/lib/dispatcher/dispatcher-base.js index bd860acdcf4..afe4e9086db 100644 --- a/lib/dispatcher/dispatcher-base.js +++ b/lib/dispatcher/dispatcher-base.js @@ -6,11 +6,10 @@ const { ClientClosedError, InvalidArgumentError } = require('../core/errors') -const { kDestroy, kClose, kClosed, kDestroyed, kDispatch, kInterceptors } = require('../core/symbols') +const { kDestroy, kClose, kClosed, kDestroyed, kDispatch } = require('../core/symbols') const kOnDestroyed = Symbol('onDestroyed') const kOnClosed = Symbol('onClosed') -const kInterceptedDispatch = Symbol('Intercepted Dispatch') class DispatcherBase extends Dispatcher { constructor () { @@ -30,23 +29,6 @@ class DispatcherBase extends Dispatcher { return this[kClosed] } - get interceptors () { - return this[kInterceptors] - } - - set interceptors (newInterceptors) { - if (newInterceptors) { - for (let i = newInterceptors.length - 1; i >= 0; i--) { - const interceptor = this[kInterceptors][i] - if (typeof interceptor !== 'function') { - throw new InvalidArgumentError('interceptor must be an function') - } - } - } - - this[kInterceptors] = newInterceptors - } - close (callback) { if (callback === undefined) { return new Promise((resolve, reject) => { @@ -142,20 +124,6 @@ class DispatcherBase extends Dispatcher { }) } - [kInterceptedDispatch] (opts, handler) { - if (!this[kInterceptors] || this[kInterceptors].length === 0) { - this[kInterceptedDispatch] = this[kDispatch] - return this[kDispatch](opts, handler) - } - - let dispatch = this[kDispatch].bind(this) - for (let i = this[kInterceptors].length - 1; i >= 0; i--) { - dispatch = this[kInterceptors][i](dispatch) - } - this[kInterceptedDispatch] = dispatch - return dispatch(opts, handler) - } - dispatch (opts, handler) { if (!handler || typeof handler !== 'object') { throw new InvalidArgumentError('handler must be an object') @@ -174,7 +142,7 @@ class DispatcherBase extends Dispatcher { throw new ClientClosedError() } - return this[kInterceptedDispatch](opts, handler) + return this[kDispatch](opts, handler) } catch (err) { if (typeof handler.onError !== 'function') { throw new InvalidArgumentError('invalid onError method') diff --git a/lib/dispatcher/pool.js b/lib/dispatcher/pool.js index 2d84cd96488..87f14aef3c0 100644 --- a/lib/dispatcher/pool.js +++ b/lib/dispatcher/pool.js @@ -12,7 +12,7 @@ const { InvalidArgumentError } = require('../core/errors') const util = require('../core/util') -const { kUrl, kInterceptors } = require('../core/symbols') +const { kUrl } = require('../core/symbols') const buildConnector = require('../core/connect') const kOptions = Symbol('options') @@ -63,15 +63,9 @@ class Pool extends PoolBase { }) } - this[kInterceptors] = options.interceptors?.Pool && Array.isArray(options.interceptors.Pool) - ? options.interceptors.Pool - : [] this[kConnections] = connections || null this[kUrl] = util.parseOrigin(origin) this[kOptions] = { ...util.deepClone(options), connect, allowH2 } - this[kOptions].interceptors = options.interceptors - ? { ...options.interceptors } - : undefined this[kFactory] = factory } diff --git a/lib/dispatcher/proxy-agent.js b/lib/dispatcher/proxy-agent.js index 226b67846da..1924bcc896d 100644 --- a/lib/dispatcher/proxy-agent.js +++ b/lib/dispatcher/proxy-agent.js @@ -1,6 +1,6 @@ 'use strict' -const { kProxy, kClose, kDestroy, kInterceptors } = require('../core/symbols') +const { kProxy, kClose, kDestroy } = require('../core/symbols') const { URL } = require('node:url') const Agent = require('./agent') const Pool = require('./pool') @@ -40,9 +40,6 @@ class ProxyAgent extends DispatcherBase { const { href, origin, port, protocol, username, password, hostname: proxyHostname } = url this[kProxy] = { uri: href, protocol } - this[kInterceptors] = opts.interceptors?.ProxyAgent && Array.isArray(opts.interceptors.ProxyAgent) - ? opts.interceptors.ProxyAgent - : [] this[kRequestTls] = opts.requestTls this[kProxyTls] = opts.proxyTls this[kProxyHeaders] = opts.headers || {} diff --git a/lib/handler/redirect-handler.js b/lib/handler/redirect-handler.js index 16a7b2150a9..3db7a476d45 100644 --- a/lib/handler/redirect-handler.js +++ b/lib/handler/redirect-handler.js @@ -24,6 +24,15 @@ class BodyAsyncIterable { } class RedirectHandler { + 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') diff --git a/test/jest/interceptor.test.js b/test/jest/interceptor.test.js deleted file mode 100644 index 84e6210fb8d..00000000000 --- a/test/jest/interceptor.test.js +++ /dev/null @@ -1,197 +0,0 @@ -'use strict' - -const { createServer } = require('node:http') -const { Agent, request } = require('../../index') -const DecoratorHandler = require('../../lib/handler/decorator-handler') -/* global expect */ - -const defaultOpts = { keepAliveTimeout: 10, keepAliveMaxTimeout: 10 } - -describe('interceptors', () => { - let server - beforeEach(async () => { - server = createServer((req, res) => { - res.setHeader('Content-Type', 'text/plain') - res.end('hello') - }) - await new Promise((resolve) => { server.listen(0, resolve) }) - }) - afterEach(async () => { - await new Promise((resolve) => server.close(resolve)) - }) - - test('interceptors are applied on client from an agent', async () => { - const interceptors = [] - const buildInterceptor = dispatch => { - const interceptorContext = { requestCount: 0 } - interceptors.push(interceptorContext) - return (opts, handler) => { - interceptorContext.requestCount++ - return dispatch(opts, handler) - } - } - - const opts = { interceptors: { Client: [buildInterceptor] }, ...defaultOpts } - const agent = new Agent(opts) - const origin = new URL(`http://localhost:${server.address().port}`) - await Promise.all([ - request(origin, { dispatcher: agent }), - request(origin, { dispatcher: agent }) - ]) - - // Assert that the requests are run on different interceptors (different Clients) - const requestCounts = interceptors.map(x => x.requestCount) - expect(requestCounts).toEqual([1, 1]) - }) - - test('interceptors are applied in the correct order', async () => { - const setHeaderInterceptor = (dispatch) => { - return (opts, handler) => { - opts.headers.push('foo', 'bar') - return dispatch(opts, handler) - } - } - - const assertHeaderInterceptor = (dispatch) => { - return (opts, handler) => { - expect(opts.headers).toEqual(['foo', 'bar']) - return dispatch(opts, handler) - } - } - - const opts = { interceptors: { Pool: [setHeaderInterceptor, assertHeaderInterceptor] }, ...defaultOpts } - const agent = new Agent(opts) - const origin = new URL(`http://localhost:${server.address().port}`) - await request(origin, { dispatcher: agent, headers: [] }) - }) - - test('interceptors handlers are called in reverse order', async () => { - const clearResponseHeadersInterceptor = (dispatch) => { - return (opts, handler) => { - class ResultInterceptor extends DecoratorHandler { - onHeaders (statusCode, headers, resume) { - return super.onHeaders(statusCode, [], resume) - } - } - - return dispatch(opts, new ResultInterceptor(handler)) - } - } - - const assertHeaderInterceptor = (dispatch) => { - return (opts, handler) => { - class ResultInterceptor extends DecoratorHandler { - onHeaders (statusCode, headers, resume) { - expect(headers).toEqual([]) - return super.onHeaders(statusCode, headers, resume) - } - } - - return dispatch(opts, new ResultInterceptor(handler)) - } - } - - const opts = { interceptors: { Agent: [assertHeaderInterceptor, clearResponseHeadersInterceptor] }, ...defaultOpts } - const agent = new Agent(opts) - const origin = new URL(`http://localhost:${server.address().port}`) - await request(origin, { dispatcher: agent, headers: [] }) - }) -}) - -describe('interceptors with NtlmRequestHandler', () => { - class FakeNtlmRequestHandler { - constructor (dispatch, opts, handler) { - this.dispatch = dispatch - this.opts = opts - this.handler = handler - this.requestCount = 0 - } - - onConnect (...args) { - return this.handler.onConnect(...args) - } - - onError (...args) { - return this.handler.onError(...args) - } - - onUpgrade (...args) { - return this.handler.onUpgrade(...args) - } - - onHeaders (statusCode, headers, resume, statusText) { - this.requestCount++ - if (this.requestCount < 2) { - // Do nothing - } else { - return this.handler.onHeaders(statusCode, headers, resume, statusText) - } - } - - onData (...args) { - if (this.requestCount < 2) { - // Do nothing - } else { - return this.handler.onData(...args) - } - } - - onComplete (...args) { - if (this.requestCount < 2) { - this.dispatch(this.opts, this) - } else { - return this.handler.onComplete(...args) - } - } - - onBodySent (...args) { - if (this.requestCount < 2) { - // Do nothing - } else { - return this.handler.onBodySent(...args) - } - } - } - let server - - beforeEach(async () => { - // This Test is important because NTLM and Negotiate require several - // http requests in sequence to run on the same keepAlive socket - - const socketRequestCountSymbol = Symbol('Socket Request Count') - server = createServer((req, res) => { - if (req.socket[socketRequestCountSymbol] === undefined) { - req.socket[socketRequestCountSymbol] = 0 - } - req.socket[socketRequestCountSymbol]++ - res.setHeader('Content-Type', 'text/plain') - - // Simulate NTLM/Negotiate logic, by returning 200 - // on the second request of each socket - if (req.socket[socketRequestCountSymbol] >= 2) { - res.statusCode = 200 - res.end() - } else { - res.statusCode = 401 - res.end() - } - }) - await new Promise((resolve) => { server.listen(0, resolve) }) - }) - afterEach(async () => { - await new Promise((resolve) => server.close(resolve)) - }) - - test('Retry interceptor on Client will use the same socket', async () => { - const interceptor = dispatch => { - return (opts, handler) => { - return dispatch(opts, new FakeNtlmRequestHandler(dispatch, opts, handler)) - } - } - const opts = { interceptors: { Client: [interceptor] }, ...defaultOpts } - const agent = new Agent(opts) - const origin = new URL(`http://localhost:${server.address().port}`) - const { statusCode } = await request(origin, { dispatcher: agent, headers: [] }) - expect(statusCode).toEqual(200) - }) -}) diff --git a/test/node-test/agent.js b/test/node-test/agent.js index 74a3ae26e77..c5238d7b570 100644 --- a/test/node-test/agent.js +++ b/test/node-test/agent.js @@ -662,11 +662,8 @@ test('stream: fails with invalid onInfo', async (t) => { }) test('constructor validations', t => { - const p = tspl(t, { plan: 4 }) + const p = tspl(t, { plan: 1 }) p.throws(() => new Agent({ factory: 'ASD' }), errors.InvalidArgumentError, 'throws on invalid opts argument') - p.throws(() => new Agent({ maxRedirections: 'ASD' }), errors.InvalidArgumentError, 'throws on invalid opts argument') - p.throws(() => new Agent({ maxRedirections: -1 }), errors.InvalidArgumentError, 'throws on invalid opts argument') - p.throws(() => new Agent({ maxRedirections: null }), errors.InvalidArgumentError, 'throws on invalid opts argument') }) test('dispatch validations', async t => { diff --git a/test/redirect-request.js b/test/redirect-request.js index e7554b929f6..fdcea5dd065 100644 --- a/test/redirect-request.js +++ b/test/redirect-request.js @@ -90,7 +90,7 @@ for (const factory of [ const server = await startRedirectingServer() - const { statusCode, headers, body: bodyStream, context: { history } } = await request(t, server, { maxRedirections: 10 }, `http://${server}/300?key=value`) + const { statusCode, headers, body: bodyStream, context: { history } } = await request(t, server, undefined, `http://${server}/300?key=value`, { maxRedirections: 10 }) const body = await bodyStream.text() t.strictEqual(statusCode, 200) @@ -380,10 +380,9 @@ for (const factory of [ t = tspl(t, { plan: 1 }) try { - await request(t, 'localhost', { + await request(t, 'localhost', undefined, 'http://localhost', { + method: 'GET', maxRedirections: 'INVALID' - }, 'http://localhost', { - method: 'GET' }) t.fail('Did not throw') diff --git a/test/redirect-stream.js b/test/redirect-stream.js index c50e5033135..3d03cc946d2 100644 --- a/test/redirect-stream.js +++ b/test/redirect-stream.js @@ -36,7 +36,7 @@ test('should always have a history with the final URL even if no redirections we t.strictEqual(body.join(''), `GET /5 key=value :: host@${server} connection@keep-alive`) }) -test('should not follow redirection by default if not using RedirectAgent', async t => { +test('should not follow redirection by default if max redirect = 0', async t => { t = tspl(t, { plan: 3 }) const body = [] diff --git a/test/types/client.test-d.ts b/test/types/client.test-d.ts index c416d779bec..d9c1e447813 100644 --- a/test/types/client.test-d.ts +++ b/test/types/client.test-d.ts @@ -68,18 +68,6 @@ expectAssignable(new Client(new URL('http://localhost'), {})) expectAssignable(new Client('', { autoSelectFamilyAttemptTimeout: 300e3 })) - expectAssignable(new Client('', { - interceptors: { - Client: [(dispatcher) => { - expectAssignable(dispatcher); - return (opts, handlers) => { - expectAssignable(opts); - expectAssignable(handlers); - return dispatcher(opts, handlers) - } - }] - } - })) } { diff --git a/test/types/interceptor.test-d.ts b/test/types/interceptor.test-d.ts deleted file mode 100644 index ba242bfcadb..00000000000 --- a/test/types/interceptor.test-d.ts +++ /dev/null @@ -1,5 +0,0 @@ -import {expectAssignable} from "tsd"; -import Undici from "../.."; -import Dispatcher from "../../types/dispatcher"; - -expectAssignable(Undici.createRedirectInterceptor({ maxRedirections: 3 })) diff --git a/types/agent.d.ts b/types/agent.d.ts index 58081ce9372..4ff6baccd86 100644 --- a/types/agent.d.ts +++ b/types/agent.d.ts @@ -20,8 +20,6 @@ declare namespace Agent { factory?(origin: string | URL, opts: Object): Dispatcher; /** Integer. Default: `0` */ maxRedirections?: number; - - interceptors?: { Agent?: readonly Dispatcher.DispatchInterceptor[] } & Pool.Options["interceptors"] } export interface DispatchOptions extends Dispatcher.DispatchOptions { diff --git a/types/client.d.ts b/types/client.d.ts index d0a5379f33c..be07a47d351 100644 --- a/types/client.d.ts +++ b/types/client.d.ts @@ -28,12 +28,7 @@ export class Client extends Dispatcher { } export declare namespace Client { - export interface OptionsInterceptors { - Client: readonly Dispatcher.DispatchInterceptor[]; - } export interface Options { - /** TODO */ - interceptors?: OptionsInterceptors; /** The maximum length of request headers in bytes. Default: Node.js' `--max-http-header-size` or `16384` (16KiB). */ maxHeaderSize?: number; /** The amount of time, in milliseconds, the parser will wait to receive the complete HTTP headers (Node 14 and above only). Default: `300e3` milliseconds (300s). */ diff --git a/types/index.d.ts b/types/index.d.ts index f6be35cd9bf..25d6526f633 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -42,7 +42,6 @@ declare namespace Undici { var RedirectHandler: typeof import ('./handlers').RedirectHandler var DecoratorHandler: typeof import ('./handlers').DecoratorHandler var RetryHandler: typeof import ('./retry-handler').default - var createRedirectInterceptor: typeof import ('./interceptors').default.createRedirectInterceptor var BalancedPool: typeof import('./balanced-pool').default; var Client: typeof import('./client').default; var buildConnector: typeof import('./connector').default; diff --git a/types/pool.d.ts b/types/pool.d.ts index bad5ba0308e..d24cfd06870 100644 --- a/types/pool.d.ts +++ b/types/pool.d.ts @@ -33,7 +33,5 @@ declare namespace Pool { factory?(origin: URL, opts: object): Dispatcher; /** The max number of clients to create. `null` if no limit. Default `null`. */ connections?: number | null; - - interceptors?: { Pool?: readonly Dispatcher.DispatchInterceptor[] } & Client.Options["interceptors"] } }