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

feat: add dns cache #2440 #2552

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 48 additions & 2 deletions README.md
Expand Up @@ -365,6 +365,52 @@ Returns: `URL`
* **protocol** `string` (optional)
* **search** `string` (optional)

## DNS caching

Undici provides DNS caching via the `DNSResolver` class.

This functionality is coming from [cacheable-lookup](https://github.com/szmarczak/cacheable-lookup/tree/9e60c9f6e74a003692aec68f3ddad93afe613b8f) package.

By default a global DNSResolver will be created and passed down by the agent to the pool.
Here is an example of how to configure a custom DNSResolver:

```js
new Agent({
dnsResolver: {
lookupOptions: { family: 6, hints: ALL }
}
})
```

To disable DNS caching for an agent:

```js
new Agent({
dnsResolver: {
disable: true
}
})
```

Provide your custom DNS resolver, it must implement the method `lookup`:

```js
new Agent({
DNSResolver: new MyCustomDNSResolver()
})
```

All arguments match [`cacheable-lookup`](https://github.com/szmarczak/cacheable-lookup/tree/9e60c9f6e74a003692aec68f3ddad93afe613b8f) package

Extra arguments available in Undici:

* **lookupOptions**
* **family** `4 | 6 | 0` - Default: `0`
* **hints** [`getaddrinfo flags`](https://nodejs.org/api/dns.html#supported-getaddrinfo-flags)
* **all** `Boolean` - Default: `false`

* **roundRobinStrategy** `'first' | 'random'` - Default: `'first'`
Copy link
Member

Choose a reason for hiding this comment

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

this should be called scheduling. What does first vs random do?

Copy link
Author

Choose a reason for hiding this comment

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

done


## Specification Compliance

This section documents parts of the HTTP/1.1 specification that Undici does
Expand Down Expand Up @@ -413,9 +459,9 @@ Refs: https://fetch.spec.whatwg.org/#atomic-http-redirect-handling

If you experience problem when connecting to a remote server that is resolved by your DNS servers to a IPv6 (AAAA record)
first, there are chances that your local router or ISP might have problem connecting to IPv6 networks. In that case
undici will throw an error with code `UND_ERR_CONNECT_TIMEOUT`.
undici will throw an error with code `UND_ERR_CONNECT_TIMEOUT`.

If the target server resolves to both a IPv6 and IPv4 (A records) address and you are using a compatible Node version
If the target server resolves to both a IPv6 and IPv4 (A records) address and you are using a compatible Node version
(18.3.0 and above), you can fix the problem by providing the `autoSelectFamily` option (support by both `undici.request`
and `undici.Agent`) which will enable the family autoselection algorithm when establishing the connection.

Expand Down
3 changes: 3 additions & 0 deletions index.js
Expand Up @@ -165,3 +165,6 @@ module.exports.MockClient = MockClient
module.exports.MockPool = MockPool
module.exports.MockAgent = MockAgent
module.exports.mockErrors = mockErrors

const DNSResolver = require('./lib/dns-resolver')
Copy link
Member

Choose a reason for hiding this comment

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

Move this to the top

module.exports.DNSResolver = DNSResolver
14 changes: 14 additions & 0 deletions lib/agent.js
Expand Up @@ -7,6 +7,7 @@ const Pool = require('./pool')
const Client = require('./client')
const util = require('./core/util')
const createRedirectInterceptor = require('./interceptor/redirectInterceptor')
const DNSResolver = require('./dns-resolver')

const kOnConnect = Symbol('onConnect')
const kOnDisconnect = Symbol('onDisconnect')
Expand All @@ -22,6 +23,18 @@ function defaultFactory (origin, opts) {
: new Pool(origin, opts)
}

function getDNSResolver (options) {
if (options?.dnsResolverOptions?.disable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it would be a better DX to have { dnsResolverOptions: false } instead to completely disable it.
Not a blocker, just a thought.

return undefined
antoinegomez marked this conversation as resolved.
Show resolved Hide resolved
}

if (options.DNSResolver && typeof options.DNSResolver.lookup === 'function') {
return options.DNSResolver
}

return new DNSResolver(options.dnsResolverOptions)
}

class Agent extends DispatcherBase {
constructor ({ factory = defaultFactory, maxRedirections = 0, connect, ...options } = {}) {
super()
Expand Down Expand Up @@ -50,6 +63,7 @@ class Agent extends DispatcherBase {
this[kOptions].interceptors = options.interceptors
? { ...options.interceptors }
: undefined
this[kOptions].dnsResolver = getDNSResolver(options)
this[kMaxRedirections] = maxRedirections
this[kFactory] = factory
this[kClients] = new Map()
Expand Down