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

try undici http client #1419

Open
1 task done
rifler opened this issue Aug 25, 2020 · 8 comments
Open
1 task done

try undici http client #1419

rifler opened this issue Aug 25, 2020 · 8 comments
Assignees
Labels
enhancement This change will extend Got features ✭ help wanted ✭

Comments

@rifler
Copy link
Contributor

rifler commented Aug 25, 2020

What problem are you trying to solve?

speed improvement

Describe the feature

use new http/1.1 client https://github.com/nodejs/undici, which is faster than builtin http

original tweet - https://twitter.com/matteocollina/status/1298148085210775553

Checklist

  • I have read the documentation and made sure this feature doesn't already exist.
@szmarczak szmarczak self-assigned this Aug 26, 2020
@szmarczak szmarczak added enhancement This change will extend Got features ✭ help wanted ✭ labels Aug 26, 2020
@ghermeto
Copy link
Contributor

ghermeto commented Sep 4, 2020

I might be able to help with that. We are discussing how Undici fits in the long term HTTP technical plans in the next web-server-frameworks team meeting. If the interface will be kept I can implement it.

cc/ @mmarchini

@szmarczak
Copy link
Collaborator

We can solve this in two ways:

  1. wrap http ClientRequest so it has a similar interface to undici (might have a tiny performance impact, but I'd have to benchmark this)

  2. write an UndiciRequest class which has the same interface as Request (zero performance impact)

@szmarczak
Copy link
Collaborator

To everyone here, how do you use http.Agents? Do you use it to modify requests?

The agent class has a .addRequest(request, options) method. It is undocumented, however used by the native http module and many popular HTTP proxy agents overwrite it to mutate the requests. For example:

https://github.com/TooTallNate/node-http-proxy-agent/blob/ef184d6b7af42e02920b015ecea557a0ac201fdc/src/agent.ts#L110

Currently it's not possible to mutate the request using undici. See nodejs/undici#887 (comment)

I made a PoC wrapper that allows native Agents to be used with undici. The drawback here is that it will error when an agent tries to mutate the request.

A workaround would be to soft-fail but that can lead to unexpected behavior.

@szmarczak
Copy link
Collaborator

I agree with the concept that it should not be possible to mutate the request. So eventually we can just soft-fail (nop, do nothing).

If anyone has any arguments for the opposite, please comment.

@ronag
Copy link

ronag commented Jul 19, 2021

process.emitWarning?

@titanism
Copy link

We used got at first, but switched to undici. Following along this issue, I think the only other performance bottleneck between using http (got) vs net (undici) we had discovered was the setup time with got.extend and iteration through the loop related to mutableDefaults.

Anyways - thanks for making got and undici - we rewrote 🍊 Tangerine (npm install tangerine) to use undici now (instead of got) and released it today at https://github.com/forwardemail/tangerine. Benchmarks including HTTP benchmarks with got vs undici are at the bottom of README.

@titanism
Copy link

Oh, also - as soon as we switched from using got (or any other HTTP library) to using undici, our package tangerine became ⚡ faster ⚡ than the Native Node.js dns module!!! 😲

@JaneJeon
Copy link
Contributor

Unfortunately "just use undici directly" isn't an option for me, given 1. community packages (got-scraping is a godsend), and 2. hooks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This change will extend Got features ✭ help wanted ✭
Projects
None yet
Development

No branches or pull requests

6 participants