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

Lifecycle hook support #491

Open
kibertoad opened this issue Dec 5, 2020 · 7 comments
Open

Lifecycle hook support #491

kibertoad opened this issue Dec 5, 2020 · 7 comments

Comments

@kibertoad
Copy link
Contributor

kibertoad commented Dec 5, 2020

It would be helpful to support pre/post-processing of requests/responses. Some of the use-cases that I see are:

  • authentication
  • logging
  • custom container wrapping (e. g. by using Either)

Ideally this feature should come with a certain level of granularity, so that it is either possible to set lifecycle hooks both per Client or globally.

If this is accepted, I can work on implementation.

@mcollina
Copy link
Member

mcollina commented Dec 5, 2020

I'm positive. This technique can be implemented with very little overhead (see Fastify).

However I would not do global, only per-client and per-pool.

@kibertoad
Copy link
Contributor Author

@mcollina Can you point out places in fastify code you would recommend to use as reference? Also should I go for full-blown plugin style as fastify does, or implement minimal useful subset of features?

@mcollina
Copy link
Member

mcollina commented Dec 6, 2020

I would go with minimal features. Here is the hooks implementation: https://github.com/fastify/fastify/blob/master/lib/hooks.js

@kibertoad
Copy link
Contributor Author

Thanks a lot!

@kibertoad
Copy link
Contributor Author

@mcollina BTW, there is no engines constraint for Node.js version in package.json, but CI is running on 10+. Do any older Node versions have to be supported?

@ronag
Copy link
Member

ronag commented Jan 12, 2021

Do any older Node versions have to be supported?

No only LTS versions.

@arontsang
Copy link
Contributor

arontsang commented Apr 3, 2022

I would like to suggest the following signature for extending Dispatchers

function DispatchExtender(dispatcher: Dispatcher, opts: RequestOptions, handler: DispatchHandlers): [RequestOptions, DispatchHandlers]

This way, we can reduce a list of DispatchExtenders when calling dispatch on each dispatcher.

for (const decorator of opts.dispatchExtenders ?? []) {
    [opts, handler] = decorator(this, opts, handler)
}

For example, we can wrap the existing RedirectHandler like this:

function RedirectDecorator (dispatcher, opts, innerHandler) {
  if (dispatcher instanceof Agent) {
    const { maxRedirections = dispatcher[kMaxRedirections] } = opts
    if (maxRedirections != null && maxRedirections !== 0) {
      opts = { ...opts, maxRedirections: 0 } // Stop sub dispatcher from also redirecting.
      return [opts, new RedirectHandler(this, maxRedirections, opts, innerHandler)]
    }
  }
  if (dispatcher instanceof Client) {
    const { maxRedirections = dispatcher[kMaxRedirections] } = opts
    if (maxRedirections) {
      return [opts, new RedirectHandler(this, maxRedirections, opts, innerHandler)]
    }
  }
  return [opts, innerHandler]
}

We can also handle caching as per #1146 using a decorator. The decorator would simply call then innerHandler with the result, then pass a RequestHandler with an already aborted abort signal, so that the Dispatcher will ignore that request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants