feat: New Interceptor API

[PandaWorker]

How about making a more convenient implementation of the interceptors?

import type { Dispatcher, Request, Response } from 'undici';
import { Agent } from 'undici';

// new interceptors api
export type InterceptorChain = (
	request: Dispatcher.RequestOptions
) => Promise<Dispatcher.ResponseData>;
export type Interceptor = (next: InterceptorChain) => InterceptorChain;

// OR

export type InterceptorChainFetch = (
	request: Request
) => Promise<Response>;
export type InterceptorFetch = (next: InterceptorChain) => InterceptorChain;

export function composeInterceptors(interceptors: Interceptor[]) {
	return (original: InterceptorChain): InterceptorChain => {
		let chain: InterceptorChain = request => original(request);

		// create chain call:
		// fn3(req) -> fn2(req) -> fn1(req) ->
		//                                                          original
		// fn3(res) <- fn2(res) <- fn1(res) <-
		//
		for (let i = 0; i < interceptors.length; i++) {
			chain = interceptors[i](chain);
		}

		return chain;
	};
}

export function toDispatchComposeInterceptor(
	interceptor: Interceptor
): Dispatcher.DispatcherComposeInterceptor {
	return dispatch => {
		// TODO: implement
		const request = interceptor((request) => {});

		return (opts, handler) => {
			return dispatch(opts, handler);
		};
	};
}

const interceptor = composeInterceptors([loggerInterceptor()]);

const agent = new Agent().compose(toDispatchComposeInterceptor(interceptor));
const agent2 = new Agent().interceptors.add(interceptor);
const agent3 = new Agent().interceptors.clear();

function loggerInterceptor(): Interceptor {
	const log = console.log.bind(console);

	return next => async request => {
		const timeStart = Date.now();
		log('request:', request.method, request.path);

		const resp = await next(request);
		const timeEnd = Date.now();
		log(
			'resp:',
			request.method,
			request.path,
			resp.statusCode,
			(timeEnd - timeStart).toFixed(2)
		);

		return resp;
	};
}

function decompress(body, contentEncoding) {
	// TODO: decompress
	return body;
}

function decompressInterceptor(): Interceptor {
	return next => async request => {
		const resp = await next(request);

		if (request?.decompress === true && resp.body && resp.headers['content-encoding']) {
			resp.body = decompress(resp.body, resp.headers['content-encoding']);
		}

		return resp;
	};
}

In almost 99% of cases, we need the request/response object at once, and not in separate hooks.
I don't think we'll get a performance drawdown when adding such an api.

[metcoder95]

I can see what are you suggesting, but what's the problem they are aiming to solve besides being less noisy by reducing the amount of hooks?

I can see some complications here due to the design of the dispatcher.

The dispatcher does not constructs a request (although it kind of share similarities with dispatchOptions) nor returns a specific response object. Attempting to do that will require several re-designs that might not be worth the effort with the value they can add.

The biggest reason to use the dispatch-like compose, was the level of granular control and compatibility with other undici APIs, so all APIs can work directly with the compose dispatcher with less to no changes.

[PandaWorker]

I suggest not completely abandoning the current implementation of interceptors on cookies, since the internal functions of the library can be strictly tied to them, but adding a new type of interceptors that will be much more convenient to use to create interceptors, without creating a bunch of unnecessary code to solve a simple task.

For example, I need to embed a cookie Jar in requests without an additional wrapper over the request, on new interceptors it would look something like this:

import { CookieJar, GetCookiesOptions, SetCookieOptions } from 'tough-cookie';

const kCookieJar = Symbol('cookie jar');

function createCookieJarInterceptor(
	options: { set?: SetCookieOptions; get?: GetCookiesOptions } = {}
): Interceptor {
	return next => async request => {
		const cookieJar = request.jar as CookieJar;

		if (request[kCookieJar] === true || !(cookieJar)) return next(request);

		request[kCookieJar] = true;

		const requestURL = new URL(request.path, request.origin).toString();

		request.headers.set(
			'cookie',
			cookieJar.getCookieStringSync(requestURL, options.get)
		);

		const resp = await next(request);

		const setCookie = resp.headers.getSetCookie();

		if (setCookie.length > 0) {
			setCookie.forEach(cookieString =>
				cookieJar.setCookieSync(cookieString, requestURL, options.set)
			);
		}

		return resp;
	};
}

And on the version with interceptors on hooks, I would have to create handlers into which I would need to throw data and so on, and the code on hooks for interceptors turns out to be very complex, most likely even worse for performance than just transferring data along the chain between layers.

For a new type of interceptors, I also suggest immediately normalizing requests and responses, namely request headers, which can be like an object/array/generator in Headers. Checking and making all the data look right at each step of the interceptor/core undici is not what we need, we would like all the data to be normalized and reduced to the promised type when creating a query

[PandaWorker]

We could also do asynchronous hooks like it's done in got.
But the current version is completely inconvenient to use

[metcoder95]

Yeah, I said earlier; see your point of how cluttered can it be when aiming to have a small interceptor.

Regarding performance, it depends on how you manage the handlers so cannot take that for granted.

Nevertheless, I've some concerns about the proposal:

1. having two interceptor kinds, can add confusion and lead to unexpected results (not to mention maintenance of both systems).
2. I'm having a bit of troubles to understand where it will be seating; from your examples it seems it depends on a request/response objects which are not a thing in undici itself; not to mention that a chain of responsibility like that might come with some extra overhead due to the several safety-checks that will need to be done to avoid the user enter into an inconsistency (which are already handled by default with the current interceptor design based in dispatcher signature).
3. The async support stills a thing; you can do it right now but you're branching the chain of dispatcher. Although that's not bad per se, it messes with backpressure.

[mcollina]

The current dispatcher API has been designed from the ground up to be performant and minimize overhead, it's a low level API and it makes sense to put performance as its focus. I'm not really convinced we need a high level interceptor API to wrap things in this way.