Conversation
|
We need to rebase |
|
Nice! I think you need to do a rebase. |
|
@metcoder95 I really appreciate your contributions with this. I'm passionate about these changes but overwhelmed with my current workload. |
Its
Happy to help! This cleanup is looking great, happy to help preparing next |
lib/agent.js
Outdated
| handler | ||
| ) | ||
| ) | ||
| } |
There was a problem hiding this comment.
I would prefer to move this one level higher to the api methods, i.e. totally exclude from normal dispatch
There was a problem hiding this comment.
So basically just expose it but not add it directly; then several tests will be affected, I’ll go over them during the weekend then 👍
I’ll split this into two PRs, one to remove all sign of interceptors, and a second one to expose Redirect handler and others in a different way.
There was a problem hiding this comment.
You don't actually need to break so many tests if you just move it into the api methods (stream, pipeline and request).
There was a problem hiding this comment.
Ah, ok, I didn't get that first; so instead of keeping it at a low level, you suggest adding it to the APIs instead; if I understood correctly, sure 👍
lib/agent.js
Outdated
| return dispatcher.dispatch( | ||
| opts, | ||
| new RedirectHandler( | ||
| dispatcher.dispatch.bind(dispatcher), | ||
| this[kMaxRedirections], | ||
| opts, | ||
| handler | ||
| ) | ||
| ) |
There was a problem hiding this comment.
| return dispatcher.dispatch( | |
| opts, | |
| new RedirectHandler( | |
| dispatcher.dispatch.bind(dispatcher), | |
| this[kMaxRedirections], | |
| opts, | |
| handler | |
| ) | |
| ) | |
| throw new Error('maxRedirection no longer directly supported on Agent') |
There was a problem hiding this comment.
You can add something like:
// api/api-request.js
function request (opts, callback) {
if (callback === undefined) {
return new Promise((resolve, reject) => {
request.call(this, opts, (err, data) => {
return err ? reject(err) : resolve(data)
})
})
}
let maxRedirections = opts?.maxRedirections ?? 0
if (!Number.isInteger(maxRedirections) || maxRedirections < 0) {
throw new InvalidArgumentError('maxRedirections must be a positive number')
}
let dispatch = (opts, handler) => this.dispatch(opts, handler)
if (maxRedirections > 0) {
dispatch = (opts, handler) => dispatch(opts, new RedirectHandler(dispatch, maxRedirections, opts, handler))
}
try {
dispatch(opts, new RequestHandler(opts, callback))
} catch (err) {
if (typeof callback !== 'function') {
throw err
}
const opaque = opts?.opaque
queueMicrotask(() => callback(err, { opaque }))
}
}|
Is this relevant for your interceptors project? |
|
@Uzlopak We currently don't support undici directly. Only fetch which uses undici under the hood. |
5dd0440 to
5798f6d
Compare
Co-authored-by: Robert Nagy <ronagy@icloud.com>
This relates to...
#2722
Rationale
Drop support for interceptors feature.
Changes
Features
Bug Fixes
Breaking Changes and Deprecations
Status