diff --git a/lib/interceptor/dns.js b/lib/interceptor/dns.js index c8d56c2cf77..38287607143 100644 --- a/lib/interceptor/dns.js +++ b/lib/interceptor/dns.js @@ -32,7 +32,7 @@ class DNSInstance { // If full, we just return the origin if (ips == null && this.full) { - cb(null, origin.origin) + cb(null, origin) return } @@ -74,9 +74,9 @@ class DNSInstance { cb( null, - `${origin.protocol}//${ + new URL(`${origin.protocol}//${ ip.family === 6 ? `[${ip.address}]` : ip.address - }${port}` + }${port}`) ) }) } else { @@ -105,9 +105,9 @@ class DNSInstance { cb( null, - `${origin.protocol}//${ + new URL(`${origin.protocol}//${ ip.family === 6 ? `[${ip.address}]` : ip.address - }${port}` + }${port}`) ) } } @@ -192,6 +192,38 @@ class DNSInstance { return ip } + pickFamily (origin, ipFamily) { + const records = this.#records.get(origin.hostname)?.records + if (!records) { + return null + } + + const family = records[ipFamily] + if (!family) { + return null + } + + if (family.offset == null || family.offset === maxInt) { + family.offset = 0 + } else { + family.offset++ + } + + const position = family.offset % family.ips.length + const ip = family.ips[position] ?? null + if (ip == null) { + return ip + } + + if (Date.now() - ip.timestamp > ip.ttl) { // record TTL is already in ms + // We delete expired records + // It is possible that they have different TTL, so we manage them individually + family.ips.splice(position, 1) + } + + return ip + } + setRecords (origin, addresses) { const timestamp = Date.now() const records = { records: { 4: null, 6: null } } @@ -228,10 +260,13 @@ class DNSDispatchHandler extends DecoratorHandler { #dispatch = null #origin = null #controller = null + #newOrigin = null + #firstTry = true - constructor (state, { origin, handler, dispatch }, opts) { + constructor (state, { origin, handler, dispatch, newOrigin }, opts) { super(handler) this.#origin = origin + this.#newOrigin = newOrigin this.#opts = { ...opts } this.#state = state this.#dispatch = dispatch @@ -242,21 +277,36 @@ class DNSDispatchHandler extends DecoratorHandler { case 'ETIMEDOUT': case 'ECONNREFUSED': { if (this.#state.dualStack) { - // We delete the record and retry - this.#state.runLookup(this.#origin, this.#opts, (err, newOrigin) => { - if (err) { - super.onResponseError(controller, err) - return - } - - const dispatchOpts = { - ...this.#opts, - origin: newOrigin - } + if (!this.#firstTry) { + super.onResponseError(controller, err) + return + } + this.#firstTry = false + + // Pick an ip address from the other family + const otherFamily = this.#newOrigin.hostname[0] === '[' ? 4 : 6 + const ip = this.#state.pickFamily(this.#origin, otherFamily) + if (ip == null) { + super.onResponseError(controller, err) + return + } - this.#dispatch(dispatchOpts, this) - }) + let port + if (typeof ip.port === 'number') { + port = `:${ip.port}` + } else if (this.#origin.port !== '') { + port = `:${this.#origin.port}` + } else { + port = '' + } + const dispatchOpts = { + ...this.#opts, + origin: `${this.#origin.protocol}//${ + ip.family === 6 ? `[${ip.address}]` : ip.address + }${port}` + } + this.#dispatch(dispatchOpts, this) return } @@ -266,7 +316,8 @@ class DNSDispatchHandler extends DecoratorHandler { } case 'ENOTFOUND': this.#state.deleteRecords(this.#origin) - // eslint-disable-next-line no-fallthrough + super.onResponseError(controller, err) + break default: super.onResponseError(controller, err) break @@ -356,11 +407,10 @@ module.exports = interceptorOpts => { return handler.onResponseError(null, err) } - let dispatchOpts = null - dispatchOpts = { + const dispatchOpts = { ...origDispatchOpts, servername: origin.hostname, // For SNI on TLS - origin: newOrigin, + origin: newOrigin.origin, headers: { host: origin.host, ...origDispatchOpts.headers @@ -369,7 +419,10 @@ module.exports = interceptorOpts => { dispatch( dispatchOpts, - instance.getHandler({ origin, dispatch, handler }, origDispatchOpts) + instance.getHandler( + { origin, dispatch, handler, newOrigin }, + origDispatchOpts + ) ) }) diff --git a/test/interceptors/dns.js b/test/interceptors/dns.js index 6dc0420527c..75fc21faec6 100644 --- a/test/interceptors/dns.js +++ b/test/interceptors/dns.js @@ -198,7 +198,7 @@ test('Should respect DNS origin hostname for SNI on TLS', async t => { }) test('Should recover on network errors (dual stack - 4)', async t => { - t = tspl(t, { plan: 8 }) + t = tspl(t, { plan: 7 }) let counter = 0 const server = createServer() @@ -236,11 +236,6 @@ test('Should recover on network errors (dual stack - 4)', async t => { break case 3: - // [::1] -> ::1 - t.equal(isIP(url.hostname), 4) - break - - case 4: // [::1] -> ::1 t.equal(isIP(url.hostname.slice(1, 4)), 6) break @@ -1732,6 +1727,57 @@ test('Should handle max cached items', async t => { t.equal(await response3.body.text(), 'hello world! (x2)') }) +test('retry once with dual-stack', async t => { + t = tspl(t, { plan: 2 }) + + const requestOptions = { + method: 'GET', + path: '/', + headers: { + 'content-type': 'application/json' + } + } + + let counter = 0 + const client = new Agent().compose([ + dispatch => { + return (opts, handler) => { + counter++ + return dispatch(opts, handler) + } + }, + dns({ + lookup: (_origin, _opts, cb) => { + cb(null, [ + { + address: '127.0.0.1', + port: 3669, + family: 4, + ttl: 1000 + }, + { + address: '::1', + port: 3669, + family: 6, + ttl: 1000 + } + ]) + } + }) + ]) + + after(async () => { + await client.close() + }) + + await t.rejects(client.request({ + ...requestOptions, + origin: 'http://localhost' + }), 'ECONNREFUSED') + + t.equal(counter, 2) +}) + test('Should handle ENOTFOUND response error', async t => { t = tspl(t, { plan: 3 }) let lookupCounter = 0