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.

Sign up for GitHub

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: implement local dns cache #132

Merged
merged 2 commits into from
Mar 24, 2022
Merged

feat: implement local dns cache #132

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2fe797
feat: implement local dns cache
nlf Mar 22, 2022
2256a55
fixup! feat: implement local dns cache
nlf Mar 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pooling, proxies, retries, [and more](#features)!
* [`opts.retry`](#opts-retry)
* [`opts.onRetry`](#opts-onretry)
* [`opts.integrity`](#opts-integrity)
* [`opts.dns`](#opts-dns)
* [Message From Our Sponsors](#wow)

### Example
Expand Down Expand Up @@ -67,6 +68,7 @@ fetch('https://registry.npmjs.org/make-fetch-happen').then(res => {
* Transparent gzip and deflate support
* [Subresource Integrity](https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity) support
* Literally punches nazis
* Built in DNS cache
* (PENDING) Range request caching and resuming

### Contributing
Expand Down Expand Up @@ -146,6 +148,7 @@ make-fetch-happen augments the `minipass-fetch` API with additional features ava
* [`opts.retry`](#opts-retry) - Request retry settings
* [`opts.onRetry`](#opts-onretry) - a function called whenever a retry is attempted
* [`opts.integrity`](#opts-integrity) - [Subresource Integrity](https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity) metadata.
* [`opts.dns`](#opts-dns) - DNS cache options

#### <a name="opts-cache-path"></a> `> opts.cachePath`

Expand Down Expand Up @@ -387,3 +390,12 @@ fetch('https://malicious-registry.org/make-fetch-happen/-/make-fetch-happen-1.0.
integrity: 'sha1-o47j7zAYnedYFn1dF/fR9OV3z8Q='
}) // Error: EINTEGRITY
```

#### <a name="opts-dns"></a> `> opts.dns`

An object that provides options for the built-in DNS cache. The following options are available:

Note: Due to limitations in the current proxy agent implementation, users of proxies will not benefit from the DNS cache.

* `ttl`: Milliseconds to keep cached DNS responses for. Defaults to `5 * 60 * 1000` (5 minutes)
* `lookup`: A custom lookup function, see [`dns.lookup()`](https://nodejs.org/api/dns.html#dnslookuphostname-options-callback) for implementation details. Defaults to `require('dns').lookup`.
5 changes: 5 additions & 0 deletions lib/agent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
const LRU = require('lru-cache')
const url = require('url')
const isLambda = require('is-lambda')
const dns = require('./dns.js')

const AGENT_CACHE = new LRU({ max: 50 })
const HttpAgent = require('agentkeepalive')
Expand Down Expand Up @@ -77,11 +78,13 @@ function getAgent (uri, opts) {
rejectUnauthorized: opts.rejectUnauthorized,
timeout: agentTimeout,
freeSocketTimeout: 15000,
lookup: dns.getLookup(opts.dns),
}) : new HttpAgent({
maxSockets: agentMaxSockets,
localAddress: opts.localAddress,
timeout: agentTimeout,
freeSocketTimeout: 15000,
lookup: dns.getLookup(opts.dns),
})
AGENT_CACHE.set(key, agent)
return agent
Expand Down Expand Up @@ -171,6 +174,8 @@ const HttpsProxyAgent = require('https-proxy-agent')
const SocksProxyAgent = require('socks-proxy-agent')
module.exports.getProxy = getProxy
function getProxy (proxyUrl, opts, isHttps) {
// our current proxy agents do not support an overridden dns lookup method, so will not
// benefit from the dns cache
const popts = {
host: proxyUrl.hostname,
port: proxyUrl.port,
Expand Down
51 changes: 51 additions & 0 deletions lib/dns.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
const LRUCache = require('lru-cache')
const dns = require('dns')

const defaultOptions = exports.defaultOptions = {
family: undefined,
hints: dns.ADDRCONFIG,
all: false,
verbatim: true,
}

const lookupCache = exports.lookupCache = new LRUCache({ max: 50 })
Copy link
Contributor

@lukekarrys lukekarrys Mar 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I don't have any input on what this number should be, but I'm curious on what factors would lead us to change this. Do we have a target package-lock.json that would install without evicting any DNS cache entries?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is per-host, so unless you have more than 50 hosts you're making requests to in a single package-lock this should be just fine. also the lru-cache behaviors mean even if you did unless there's an even distribution you'll end up hitting the cache for the majority of requests anyway.


// this is a factory so that each request can have its own lookup function and ttl while still
// sharing the cache across all requests
nlf marked this conversation as resolved.
Show resolved Hide resolved
exports.getLookup = (dnsOptions) => {
return (hostname, options, callback) => {
if (typeof options === 'function') {
callback = options
options = null
}

if (typeof options === 'number') {
nlf marked this conversation as resolved.
Show resolved Hide resolved
options = { family: options }
}

options = { ...defaultOptions, ...options }

const key = JSON.stringify({
hostname,
family: options.family,
hints: options.hints,
all: options.all,
verbatim: options.verbatim,
})

if (lookupCache.has(key)) {
const [address, family] = lookupCache.get(key)
process.nextTick(callback, null, address, family)
return
}

dnsOptions.lookup(hostname, options, (err, address, family) => {
if (err) {
return callback(err)
}

lookupCache.set(key, [address, family], { ttl: dnsOptions.ttl })
return callback(null, address, family)
})
}
}
4 changes: 4 additions & 0 deletions lib/options.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const dns = require('dns')

const conditionalHeaders = [
'if-modified-since',
'if-none-match',
Expand Down Expand Up @@ -26,6 +28,8 @@ const configureOptions = (opts) => {
options.retry = { retries: 0, ...options.retry }
}

options.dns = { ttl: 5 * 60 * 1000, lookup: dns.lookup, ...options.dns }

options.cache = options.cache || 'default'
if (options.cache === 'default') {
const hasConditionalHeader = Object.keys(options.headers || {}).some((name) => {
Expand Down
10 changes: 5 additions & 5 deletions test/agent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ t.test('agent: false returns false', async t => {
})

t.test('all expected options passed down to HttpAgent', async t => {
t.same(agent('http://foo.com/bar', OPTS), {
t.match(agent('http://foo.com/bar', OPTS), {
__type: 'http',
maxSockets: 5,
localAddress: 'localAddress',
Expand All @@ -57,7 +57,7 @@ t.test('all expected options passed down to HttpAgent', async t => {
})

t.test('timeout 0 keeps timeout 0', async t => {
t.same(agent('http://foo.com/bar', { ...OPTS, timeout: 0 }), {
t.match(agent('http://foo.com/bar', { ...OPTS, timeout: 0 }), {
__type: 'http',
maxSockets: 5,
localAddress: 'localAddress',
Expand All @@ -67,7 +67,7 @@ t.test('timeout 0 keeps timeout 0', async t => {
})

t.test('no max sockets gets 15 max sockets', async t => {
t.same(agent('http://foo.com/bar', { ...OPTS, maxSockets: undefined }), {
t.match(agent('http://foo.com/bar', { ...OPTS, maxSockets: undefined }), {
__type: 'http',
maxSockets: 15,
localAddress: 'localAddress',
Expand All @@ -77,7 +77,7 @@ t.test('no max sockets gets 15 max sockets', async t => {
})

t.test('no timeout gets timeout 0', async t => {
t.same(agent('http://foo.com/bar', { ...OPTS, timeout: undefined }), {
t.match(agent('http://foo.com/bar', { ...OPTS, timeout: undefined }), {
__type: 'http',
maxSockets: 5,
localAddress: 'localAddress',
Expand All @@ -87,7 +87,7 @@ t.test('no timeout gets timeout 0', async t => {
})

t.test('all expected options passed down to HttpsAgent', async t => {
t.same(agent('https://foo.com/bar', OPTS), {
t.match(agent('https://foo.com/bar', OPTS), {
__type: 'https',
ca: 'ca',
cert: 'cert',
Expand Down
Loading