Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
49 changes: 49 additions & 0 deletions docs/docs/api/Dispatcher.md
Original file line number Diff line number Diff line change
Expand Up @@ -1043,6 +1043,7 @@ The `dns` interceptor enables you to cache DNS lookups for a given duration, per
- The function should return a single record from the records array.
- By default a simplified version of Round Robin is used.
- The `records` property can be mutated to store the state of the balancing algorithm.
- `storage: DNSStorage` - Custom storage for resolved DNS records

> The `Dispatcher#options` also gets extended with the options `dns.affinity`, `dns.dualStack`, `dns.lookup` and `dns.pick` which can be used to configure the interceptor at a request-per-request basis.

Expand All @@ -1057,6 +1058,15 @@ It represents a map of DNS IP addresses records for a single origin.
- `4.ips` - (`DNSInterceptorRecord[] | null`) The IPv4 addresses.
- `6.ips` - (`DNSInterceptorRecord[] | null`) The IPv6 addresses.

**DNSStorage**
It represents a storage object for resolved DNS records.
- `size` - (`number`) current size of the storage.
- `get` - (`(origin: string) => DNSInterceptorOriginRecords | null`) method to get the records for a given origin.
- `set` - (`(origin: string, records: DNSInterceptorOriginRecords | null, options: { ttl: number }) => void`) method to set the records for a given origin.
- `delete` - (`(origin: string) => void`) method to delete records for a given origin.
- `full` - (`() => boolean`) method to check if the storage is full, if returns `true`, DNS lookup will be skipped in this interceptor and new records will not be stored.
```ts

**Example - Basic DNS Interceptor**

```js
Expand All @@ -1073,6 +1083,45 @@ const response = await client.request({
})
```

**Example - DNS Interceptor and LRU cache as a storage**

```js
const { Client, interceptors } = require("undici");
const QuickLRU = require("quick-lru");
const { dns } = interceptors;

const lru = new QuickLRU({ maxSize: 100 });

const lruAdapter = {
get size() {
return lru.size;
},
get(origin) {
return lru.get(origin);
},
set(origin, records, { ttl }) {
Copy link

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

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

The destructuring assignment assumes the options parameter will always be passed and contain a ttl property. This should be updated to handle cases where options might be undefined: set(origin, records, options = {}) { const { ttl } = options; ... }

Suggested change
set(origin, records, { ttl }) {
set(origin, records, options = {}) {
const { ttl } = options;

Copilot uses AI. Check for mistakes.
lru.set(origin, records, { maxAge: ttl });
},
delete(origin) {
lru.delete(origin);
},
full() {
// For LRU cache, we can always store new records,
// old records will be evicted automatically
return false;
}
}

const client = new Agent().compose([
dns({ storage: lruAdapter })
])

const response = await client.request({
origin: `http://localhost:3030`,
...requestOpts
})
```

##### `responseError`

The `responseError` interceptor throws an error for responses with status code errors (>= 400).
Expand Down
68 changes: 55 additions & 13 deletions lib/interceptor/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,44 @@ const DecoratorHandler = require('../handler/decorator-handler')
const { InvalidArgumentError, InformationalError } = require('../core/errors')
const maxInt = Math.pow(2, 31) - 1

class DNSStorage {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lru-cache adapter example:

const cache = new LRU({ max: maxItems })

const adapter = {
  set: (hostname: string, records: any, opts: { ttl: number }): void => {
    cache.set(hostname, records, opts);
  },
  get: (hostname: string): any => {
    return cache.get(hostname);
  },
  delete: (hostname: string) => {
    cache.delete(hostname);
  },
  full: (): boolean => {
    return false;
  },
};

const interceptor = interceptors.dns({ maxItems: Infinity, maxTTL: 60000, cache: adapter })

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Grafana visualization:

Image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cache metrics was a veeeery useful feature for us. But mostly for HTTP requests ofc)

#maxItems = 0
#records = new Map()

constructor (opts) {
this.#maxItems = opts.maxItems
}

get size () {
return this.#records.size
}

get (hostname) {
return this.#records.get(hostname) ?? null
}

set (hostname, records) {
this.#records.set(hostname, records)
}

delete (hostname) {
this.#records.delete(hostname)
}

// Delegate to storage decide can we do more lookups or not
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For example, lru-cache adapter can always return false here

full () {
return this.size >= this.#maxItems
}
}

class DNSInstance {
#maxTTL = 0
#maxItems = 0
#records = new Map()
dualStack = true
affinity = null
lookup = null
pick = null
storage = null

constructor (opts) {
this.#maxTTL = opts.maxTTL
Expand All @@ -21,17 +51,14 @@ class DNSInstance {
this.affinity = opts.affinity
this.lookup = opts.lookup ?? this.#defaultLookup
this.pick = opts.pick ?? this.#defaultPick
}

get full () {
return this.#records.size === this.#maxItems
this.storage = opts.storage ?? new DNSStorage(opts)
}

runLookup (origin, opts, cb) {
const ips = this.#records.get(origin.hostname)
const ips = this.storage.get(origin.hostname)

// If full, we just return the origin
if (ips == null && this.full) {
if (ips == null && this.storage.full()) {
cb(null, origin)
return
}
Expand All @@ -55,7 +82,7 @@ class DNSInstance {
}

this.setRecords(origin, addresses)
const records = this.#records.get(origin.hostname)
const records = this.storage.get(origin.hostname)

const ip = this.pick(
origin,
Expand Down Expand Up @@ -89,7 +116,7 @@ class DNSInstance {

// If no IPs we lookup - deleting old records
if (ip == null) {
this.#records.delete(origin.hostname)
this.storage.delete(origin.hostname)
this.runLookup(origin, opts, cb)
return
}
Expand Down Expand Up @@ -193,7 +220,7 @@ class DNSInstance {
}

pickFamily (origin, ipFamily) {
const records = this.#records.get(origin.hostname)?.records
const records = this.storage.get(origin.hostname)?.records
if (!records) {
return null
}
Expand Down Expand Up @@ -227,11 +254,13 @@ class DNSInstance {
setRecords (origin, addresses) {
const timestamp = Date.now()
const records = { records: { 4: null, 6: null } }
let minTTL = this.#maxTTL
Copy link

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

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

The minTTL initialization should use Infinity instead of this.#maxTTL to correctly calculate the minimum TTL across all records. With the current implementation, if all records have TTL values greater than maxTTL, minTTL will incorrectly remain at maxTTL instead of the actual minimum.

Suggested change
let minTTL = this.#maxTTL
let minTTL = Infinity

Copilot uses AI. Check for mistakes.
for (const record of addresses) {
record.timestamp = timestamp
if (typeof record.ttl === 'number') {
// The record TTL is expected to be in ms
record.ttl = Math.min(record.ttl, this.#maxTTL)
minTTL = Math.min(minTTL, record.ttl)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For LRU or any other caches, which will not be able to respect ttl per record, just pass the minimum value from records.

Can't find better solution because records is a mutable object and we need to store the same for every origin,

Copy link
Member

Choose a reason for hiding this comment

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

It should be fine to keep it as is, this just duplicates it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea is to find minimum TTL for all resolved records for origin, and pass it to external storage, I don't quite understand about duplication

Copy link
Member

Choose a reason for hiding this comment

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

ah ok see it now, missed that bit

} else {
record.ttl = this.#maxTTL
}
Expand All @@ -242,11 +271,12 @@ class DNSInstance {
records.records[record.family] = familyRecords
}

this.#records.set(origin.hostname, records)
// We provide a default TTL if external storage will be used without TTL per record-level support
this.storage.set(origin.hostname, records, { ttl: minTTL })
Copy link

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

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

The storage interface documentation shows that the ttl parameter is optional, but this code always passes it. Consider checking if the storage implementation supports TTL before passing the options parameter to avoid potential issues with storage implementations that don't expect additional parameters.

Suggested change
this.storage.set(origin.hostname, records, { ttl: minTTL })
if (typeof this.storage.set === 'function' && this.storage.set.length >= 3) {
this.storage.set(origin.hostname, records, { ttl: minTTL })
} else {
this.storage.set(origin.hostname, records)
}

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

The third argument is not used isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can be used in external storages, if lru-cache or quick-lru will be used under the hood

}

deleteRecords (origin) {
this.#records.delete(origin.hostname)
this.storage.delete(origin.hostname)
}

getHandler (meta, opts) {
Expand Down Expand Up @@ -372,6 +402,17 @@ module.exports = interceptorOpts => {
throw new InvalidArgumentError('Invalid pick. Must be a function')
}

if (
interceptorOpts?.storage != null &&
(typeof interceptorOpts?.storage?.get !== 'function' ||
typeof interceptorOpts?.storage?.set !== 'function' ||
typeof interceptorOpts?.storage?.full !== 'function' ||
typeof interceptorOpts?.storage?.delete !== 'function'
)
) {
throw new InvalidArgumentError('Invalid storage. Must be a object with methods: { get, set, full, delete }')
}

const dualStack = interceptorOpts?.dualStack ?? true
let affinity
if (dualStack) {
Expand All @@ -386,7 +427,8 @@ module.exports = interceptorOpts => {
pick: interceptorOpts?.pick ?? null,
dualStack,
affinity,
maxItems: interceptorOpts?.maxItems ?? Infinity
maxItems: interceptorOpts?.maxItems ?? Infinity,
storage: interceptorOpts?.storage
}

const instance = new DNSInstance(opts)
Expand Down
127 changes: 126 additions & 1 deletion test/interceptors/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const { interceptors, Agent } = require('../..')
const { dns } = interceptors

test('Should validate options', t => {
t = tspl(t, { plan: 10 })
t = tspl(t, { plan: 11 })

t.throws(() => dns({ dualStack: 'true' }), { code: 'UND_ERR_INVALID_ARG' })
t.throws(() => dns({ dualStack: 0 }), { code: 'UND_ERR_INVALID_ARG' })
Expand All @@ -27,6 +27,7 @@ test('Should validate options', t => {
t.throws(() => dns({ maxItems: -1 }), { code: 'UND_ERR_INVALID_ARG' })
t.throws(() => dns({ lookup: {} }), { code: 'UND_ERR_INVALID_ARG' })
t.throws(() => dns({ pick: [] }), { code: 'UND_ERR_INVALID_ARG' })
t.throws(() => dns({ storage: new Map() }), { code: 'UND_ERR_INVALID_ARG' })
})

test('Should automatically resolve IPs (dual stack)', async t => {
Expand Down Expand Up @@ -1727,6 +1728,130 @@ test('Should handle max cached items', async t => {
t.equal(await response3.body.text(), 'hello world! (x2)')
})

test('Should support external storage', async t => {
t = tspl(t, { plan: 9 })

let counter = 0
const server1 = createServer({ joinDuplicateHeaders: true })
const server2 = createServer({ joinDuplicateHeaders: true })
const requestOptions = {
method: 'GET',
path: '/',
headers: {
'content-type': 'application/json'
}
}

server1.on('request', (req, res) => {
res.writeHead(200, { 'content-type': 'text/plain' })
res.end('hello world!')
})

server1.listen(0)

server2.on('request', (req, res) => {
res.writeHead(200, { 'content-type': 'text/plain' })
res.end('hello world! (x2)')
})
server2.listen(0)

await Promise.all([once(server1, 'listening'), once(server2, 'listening')])

const cache = new Map()
const storage = {
get (origin) {
return cache.get(origin)
},
set (origin, records) {
cache.set(origin, records)
},
delete (origin) {
cache.delete(origin)
},
// simulate internal DNSStorage behaviour with `maxItems: 1` parameter
full () {
return cache.size === 1
}
}

const client = new Agent().compose([
dispatch => {
return (opts, handler) => {
++counter
const url = new URL(opts.origin)

switch (counter) {
case 1:
t.equal(isIP(url.hostname), 4)
break

case 2:
// [::1] -> ::1
t.equal(isIP(url.hostname.slice(1, 4)), 6)
break

case 3:
t.equal(url.hostname, 'developer.mozilla.org')
// Rewrite origin to avoid reaching internet
opts.origin = `http://127.0.0.1:${server2.address().port}`
break
default:
t.fails('should not reach this point')
}

return dispatch(opts, handler)
}
},
dns({
storage,
lookup: (_origin, _opts, cb) => {
cb(null, [
{
address: '::1',
family: 6
},
{
address: '127.0.0.1',
family: 4
}
])
}
})
])

after(async () => {
await client.close()
server1.close()
server2.close()

await Promise.all([once(server1, 'close'), once(server2, 'close')])
})

const response = await client.request({
...requestOptions,
origin: `http://localhost:${server1.address().port}`
})

t.equal(response.statusCode, 200)
t.equal(await response.body.text(), 'hello world!')

const response2 = await client.request({
...requestOptions,
origin: `http://localhost:${server1.address().port}`
})

t.equal(response2.statusCode, 200)
t.equal(await response2.body.text(), 'hello world!')

const response3 = await client.request({
...requestOptions,
origin: 'https://developer.mozilla.org'
})

t.equal(response3.statusCode, 200)
t.equal(await response3.body.text(), 'hello world! (x2)')
})

test('retry once with dual-stack', async t => {
t = tspl(t, { plan: 2 })

Expand Down
12 changes: 10 additions & 2 deletions types/interceptors.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,22 @@ declare namespace Interceptors {

// DNS interceptor
export type DNSInterceptorRecord = { address: string, ttl: number, family: 4 | 6 }
export type DNSInterceptorOriginRecords = { 4: { ips: DNSInterceptorRecord[] } | null, 6: { ips: DNSInterceptorRecord[] } | null }
export type DNSInterceptorOriginRecords = { records: { 4: { ips: DNSInterceptorRecord[] } | null, 6: { ips: DNSInterceptorRecord[] } | null } }
Copy link
Contributor Author

@SuperOleg39 SuperOleg39 Sep 26, 2025

Choose a reason for hiding this comment

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

Here was incorrect typings - cached records shape is { records: ... }

export type DNSStorage = {
size: number
get(origin: string): DNSInterceptorOriginRecords | null
set(origin: string, records: DNSInterceptorOriginRecords | null, options: { ttl: number }): void
delete(origin: string): void
full(): boolean
}
export type DNSInterceptorOpts = {
maxTTL?: number
maxItems?: number
lookup?: (hostname: string, options: LookupOptions, callback: (err: NodeJS.ErrnoException | null, addresses: DNSInterceptorRecord[]) => void) => void
lookup?: (origin: URL, options: LookupOptions, callback: (err: NodeJS.ErrnoException | null, addresses: DNSInterceptorRecord[]) => void) => void
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here also was incorrect typings - before pick method calls, string origin is converted to URL

pick?: (origin: URL, records: DNSInterceptorOriginRecords, affinity: 4 | 6) => DNSInterceptorRecord
dualStack?: boolean
affinity?: 4 | 6
storage?: DNSStorage
}

export function dump (opts?: DumpInterceptorOpts): Dispatcher.DispatcherComposeInterceptor
Expand Down
Loading