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

chore!: drop interceptors #3399

Merged
merged 4 commits into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion docs/docs/api/Agent.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ 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

Expand Down
2 changes: 0 additions & 2 deletions docs/docs/api/Client.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
<!-- 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
60 changes: 0 additions & 60 deletions docs/docs/api/DispatchInterceptor.md

This file was deleted.

1 change: 0 additions & 1 deletion docs/docs/api/Pool.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 0 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const RetryHandler = require('./lib/handler/retry-handler')
const { getGlobalDispatcher, setGlobalDispatcher } = require('./lib/global')
const DecoratorHandler = require('./lib/handler/decorator-handler')
const RedirectHandler = require('./lib/handler/redirect-handler')
const createRedirectInterceptor = require('./lib/interceptor/redirect-interceptor')

Object.assign(Dispatcher.prototype, api)

Expand All @@ -37,7 +36,6 @@ 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'),
Expand Down
14 changes: 13 additions & 1 deletion lib/api/api-connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/redirect-handler')
const { addSignal, removeSignal } = require('./abort-signal')

class ConnectHandler extends AsyncResource {
Expand Down Expand Up @@ -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
Expand Down
15 changes: 12 additions & 3 deletions lib/api/api-pipeline.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/redirect-handler')
const { addSignal, removeSignal } = require('./abort-signal')
const assert = require('node:assert')

const kResume = Symbol('resume')

Expand Down Expand Up @@ -240,7 +241,15 @@ 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)
Expand Down
16 changes: 14 additions & 2 deletions lib/api/api-request.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
'use strict'

const assert = require('node:assert')
const { AsyncResource } = require('node:async_hooks')
const { Readable } = require('./readable')
const { InvalidArgumentError, RequestAbortedError } = require('../core/errors')
const util = require('../core/util')
const RedirectHandler = require('../handler/redirect-handler')

Check failure on line 8 in lib/api/api-request.js

View workflow job for this annotation

GitHub Actions / Lint

'RedirectHandler' is assigned a value but never used
const { getResolveErrorBodyCallback } = require('./util')
const { AsyncResource } = require('node:async_hooks')

class RequestHandler extends AsyncResource {
constructor (opts, callback) {
Expand Down Expand Up @@ -189,7 +190,18 @@
}

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

this.dispatch(opts, handler)
} catch (err) {
if (typeof callback !== 'function') {
throw err
Expand Down
16 changes: 14 additions & 2 deletions lib/api/api-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@

const assert = require('node:assert')
const { finished, PassThrough } = require('node:stream')
const { AsyncResource } = require('node:async_hooks')
const { InvalidArgumentError, InvalidReturnValueError } = require('../core/errors')
const util = require('../core/util')
const RedirectHandler = require('../handler/redirect-handler')
const { getResolveErrorBodyCallback } = require('./util')
const { AsyncResource } = require('node:async_hooks')
const { addSignal, removeSignal } = require('./abort-signal')

class StreamHandler extends AsyncResource {
Expand Down Expand Up @@ -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
Expand Down
18 changes: 15 additions & 3 deletions lib/api/api-upgrade.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

const { InvalidArgumentError, SocketError } = require('../core/errors')
const { AsyncResource } = require('node:async_hooks')
const assert = require('node:assert')
const util = require('../core/util')
const RedirectHandler = require('../handler/redirect-handler')
const { addSignal, removeSignal } = require('./abort-signal')
const assert = require('node:assert')

class UpgradeHandler extends AsyncResource {
constructor (opts, callback) {
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion lib/core/symbols.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
18 changes: 2 additions & 16 deletions lib/dispatcher/agent.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
'use strict'

const { InvalidArgumentError } = require('../core/errors')
const { kClients, kRunning, kClose, kDestroy, kDispatch, kInterceptors } = require('../core/symbols')
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 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')
Expand All @@ -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') {
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

Can you throw a dedicated error if maxRedirections or interceptors are specified?

this[kFactory] = factory
this[kClients] = new Map()

Expand Down
5 changes: 1 addition & 4 deletions lib/dispatcher/balanced-pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const {
kGetDispatcher
} = require('./pool-base')
const Pool = require('./pool')
const { kUrl, kInterceptors } = require('../core/symbols')
const { kUrl } = require('../core/symbols')
const { parseOrigin } = require('../core/util')
const kFactory = Symbol('factory')

Expand Down Expand Up @@ -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) {
Expand Down
Loading
Loading