Skip to content

Commit

Permalink
feat: Support autoSelectFamily when connecting. (nodejs#1914)
Browse files Browse the repository at this point in the history
* feat: Support autoSelectFamily when connecting.

* test: Fixed test.
  • Loading branch information
ShogunPanda authored and crysmags committed Feb 27, 2024
1 parent 8deb1e8 commit 16a8d68
Show file tree
Hide file tree
Showing 28 changed files with 263 additions and 49 deletions.
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,18 @@ implementations in Deno and Cloudflare Workers.

Refs: https://fetch.spec.whatwg.org/#atomic-http-redirect-handling

## Workarounds

### Network address family autoselection.

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`.

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.

## Collaborators

* [__Daniele Belardi__](https://github.com/dnlup), <https://www.npmjs.com/~dnlup>
Expand Down
2 changes: 2 additions & 0 deletions docs/api/Client.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ Returns: `Client`
* **connect** `ConnectOptions | Function | null` (optional) - Default: `null`.
* **strictContentLength** `Boolean` (optional) - Default: `true` - Whether to treat request content length mismatches as errors. If true, an error is thrown when the request content-length header doesn't match the length of the request body.
* **interceptors** `{ Client: DispatchInterceptor[] }` - Default: `[RedirectInterceptor]` - A list of interceptors that are applied to the dispatch method. Additional logic can be applied (such as, but not limited to: 302 status code handling, authentication, cookies, compression and caching). Note that the behavior of interceptors is Experimental and might change at any given time.
* **autoSelectFamily**: `boolean` (optional) - Default: depends on local Node version, on Node 18.13.0 and above is `false`. Enables a family autodetection algorithm that loosely implements section 5 of [RFC 8305](https://tools.ietf.org/html/rfc8305#section-5). See [here](https://nodejs.org/api/net.html#socketconnectoptions-connectlistener) for more details. This option is ignored if not supported by the current Node version.
* **autoSelectFamilyAttemptTimeout**: `number` - Default: depends on local Node version, on Node 18.13.0 and above is `250`. The amount of time in milliseconds to wait for a connection attempt to finish before trying the next address when using the `autoSelectFamily` option. See [here](https://nodejs.org/api/net.html#socketconnectoptions-connectlistener) for more details.

#### Parameter: `ConnectOptions`

Expand Down
10 changes: 3 additions & 7 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,6 @@ const DecoratorHandler = require('./lib/handler/DecoratorHandler')
const RedirectHandler = require('./lib/handler/RedirectHandler')
const createRedirectInterceptor = require('./lib/interceptor/redirectInterceptor')

const nodeVersion = process.versions.node.split('.')
const nodeMajor = Number(nodeVersion[0])
const nodeMinor = Number(nodeVersion[1])

let hasCrypto
try {
require('crypto')
Expand Down Expand Up @@ -100,7 +96,7 @@ function makeDispatcher (fn) {
module.exports.setGlobalDispatcher = setGlobalDispatcher
module.exports.getGlobalDispatcher = getGlobalDispatcher

if (nodeMajor > 16 || (nodeMajor === 16 && nodeMinor >= 8)) {
if (util.nodeMajor > 16 || (util.nodeMajor === 16 && util.nodeMinor >= 8)) {
let fetchImpl = null
module.exports.fetch = async function fetch (resource) {
if (!fetchImpl) {
Expand All @@ -127,7 +123,7 @@ if (nodeMajor > 16 || (nodeMajor === 16 && nodeMinor >= 8)) {
module.exports.getGlobalOrigin = getGlobalOrigin
}

if (nodeMajor >= 16) {
if (util.nodeMajor >= 16) {
const { deleteCookie, getCookies, getSetCookies, setCookie } = require('./lib/cookies')

module.exports.deleteCookie = deleteCookie
Expand All @@ -141,7 +137,7 @@ if (nodeMajor >= 16) {
module.exports.serializeAMimeType = serializeAMimeType
}

if (nodeMajor >= 18 && hasCrypto) {
if (util.nodeMajor >= 18 && hasCrypto) {
const { WebSocket } = require('./lib/websocket/websocket')

module.exports.WebSocket = WebSocket
Expand Down
12 changes: 11 additions & 1 deletion lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,9 @@ class Client extends DispatcherBase {
connect,
maxRequestsPerClient,
localAddress,
maxResponseSize
maxResponseSize,
autoSelectFamily,
autoSelectFamilyAttemptTimeout
} = {}) {
super()

Expand Down Expand Up @@ -185,12 +187,20 @@ class Client extends DispatcherBase {
throw new InvalidArgumentError('maxResponseSize must be a positive number')
}

if (
autoSelectFamilyAttemptTimeout != null &&
(!Number.isInteger(autoSelectFamilyAttemptTimeout) || autoSelectFamilyAttemptTimeout < -1)
) {
throw new InvalidArgumentError('autoSelectFamilyAttemptTimeout must be a positive number')
}

if (typeof connect !== 'function') {
connect = buildConnector({
...tls,
maxCachedSessions,
socketPath,
timeout: connectTimeout,
...(util.nodeHasAutoSelectFamily && autoSelectFamily ? { autoSelectFamily, autoSelectFamilyAttemptTimeout } : undefined),
...connect
})
}
Expand Down
6 changes: 1 addition & 5 deletions lib/core/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ const channels = {}

let extractBody

const nodeVersion = process.versions.node.split('.')
const nodeMajor = Number(nodeVersion[0])
const nodeMinor = Number(nodeVersion[1])

try {
const diagnosticsChannel = require('diagnostics_channel')
channels.create = diagnosticsChannel.channel('undici:request:create')
Expand Down Expand Up @@ -172,7 +168,7 @@ class Request {
}

if (util.isFormDataLike(this.body)) {
if (nodeMajor < 16 || (nodeMajor === 16 && nodeMinor < 8)) {
if (util.nodeMajor < 16 || (util.nodeMajor === 16 && util.nodeMinor < 8)) {
throw new InvalidArgumentError('Form-Data bodies are only supported in node v16.8 and newer.')
}

Expand Down
7 changes: 6 additions & 1 deletion lib/core/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ const { Blob } = require('buffer')
const nodeUtil = require('util')
const { stringify } = require('querystring')

const [nodeMajor, nodeMinor] = process.versions.node.split('.').map(v => Number(v))

function nop () {}

function isStream (obj) {
Expand Down Expand Up @@ -420,5 +422,8 @@ module.exports = {
validateHandler,
getSocketInfo,
isFormDataLike,
buildURL
buildURL,
nodeMajor,
nodeMinor,
nodeHasAutoSelectFamily: nodeMajor > 18 || (nodeMajor === 18 && nodeMinor >= 13)
}
6 changes: 1 addition & 5 deletions lib/fetch/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ const {
const { kHeadersList } = require('../core/symbols')
const EE = require('events')
const { Readable, pipeline } = require('stream')
const { isErrored, isReadable } = require('../core/util')
const { isErrored, isReadable, nodeMajor, nodeMinor } = require('../core/util')
const { dataURLProcessor, serializeAMimeType } = require('./dataURL')
const { TransformStream } = require('stream/web')
const { getGlobalDispatcher } = require('../global')
Expand All @@ -64,10 +64,6 @@ const { STATUS_CODES } = require('http')
let resolveObjectURL
let ReadableStream = globalThis.ReadableStream

const nodeVersion = process.versions.node.split('.')
const nodeMajor = Number(nodeVersion[0])
const nodeMinor = Number(nodeVersion[1])

class Fetch extends EE {
constructor (dispatcher) {
super()
Expand Down
3 changes: 3 additions & 0 deletions lib/pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ class Pool extends PoolBase {
tls,
maxCachedSessions,
socketPath,
autoSelectFamily,
autoSelectFamilyAttemptTimeout,
...options
} = {}) {
super()
Expand All @@ -54,6 +56,7 @@ class Pool extends PoolBase {
maxCachedSessions,
socketPath,
timeout: connectTimeout == null ? 10e3 : connectTimeout,
...(util.nodeHasAutoSelectFamily && autoSelectFamily ? { autoSelectFamily, autoSelectFamilyAttemptTimeout } : undefined),
...connect
})
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
"concurrently": "^7.1.0",
"cronometro": "^1.0.5",
"delay": "^5.0.0",
"dns-packet": "^5.4.0",
"docsify-cli": "^4.4.3",
"form-data": "^4.0.0",
"formdata-node": "^4.3.1",
Expand All @@ -91,7 +92,6 @@
"pre-commit": "^1.2.2",
"proxy": "^1.0.2",
"proxyquire": "^2.1.3",
"semver": "^7.3.5",
"sinon": "^15.0.0",
"snazzy": "^9.0.0",
"standard": "^17.0.0",
Expand Down
187 changes: 187 additions & 0 deletions test/autoselectfamily.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
'use strict'

const { test } = require('tap')
const dgram = require('dgram')
const { Resolver } = require('dns')
const dnsPacket = require('dns-packet')
const { createServer } = require('http')
const { Client, Agent, request } = require('..')
const { nodeHasAutoSelectFamily } = require('../lib/core/util')

function _lookup (resolver, hostname, options, cb) {
resolver.resolve(hostname, 'ANY', (err, replies) => {
if (err) {
return cb(err)
}

const hosts = replies
.map((r) => ({ address: r.address, family: r.type === 'AAAA' ? 6 : 4 }))
.sort((a, b) => b.family - a.family)

if (options.all === true) {
return cb(null, hosts)
}

return cb(null, hosts[0].address, hosts[0].family)
})
}

function createDnsServer (ipv6Addr, ipv4Addr, cb) {
// Create a DNS server which replies with a AAAA and a A record for the same host
const socket = dgram.createSocket('udp4')

socket.on('message', (msg, { address, port }) => {
const parsed = dnsPacket.decode(msg)

const response = dnsPacket.encode({
type: 'answer',
id: parsed.id,
questions: parsed.questions,
answers: [
{ type: 'AAAA', class: 'IN', name: 'example.org', data: '::1', ttl: 123 },
{ type: 'A', class: 'IN', name: 'example.org', data: '127.0.0.1', ttl: 123 }
]
})

socket.send(response, port, address)
})

socket.bind(0, () => {
const resolver = new Resolver()
resolver.setServers([`127.0.0.1:${socket.address().port}`])

cb(null, { dnsServer: socket, lookup: _lookup.bind(null, resolver) })
})
}

if (nodeHasAutoSelectFamily) {
test('with autoSelectFamily enable the request succeeds when using request', (t) => {
t.plan(3)

createDnsServer('::1', '127.0.0.1', function (_, { dnsServer, lookup }) {
const server = createServer((req, res) => {
res.end('hello')
})

t.teardown(() => {
server.close()
dnsServer.close()
})

server.listen(0, '127.0.0.1', () => {
const agent = new Agent({ connect: { lookup }, autoSelectFamily: true })

request(
`http://example.org:${server.address().port}/`, {
method: 'GET',
dispatcher: agent
}, (err, { statusCode, body }) => {
t.error(err)

let response = Buffer.alloc(0)

body.on('data', chunk => {
response = Buffer.concat([response, chunk])
})

body.on('end', () => {
t.strictSame(statusCode, 200)
t.strictSame(response.toString('utf-8'), 'hello')
})
})
})
})
})

test('with autoSelectFamily enable the request succeeds when using a client', (t) => {
t.plan(3)

createDnsServer('::1', '127.0.0.1', function (_, { dnsServer, lookup }) {
const server = createServer((req, res) => {
res.end('hello')
})

t.teardown(() => {
server.close()
dnsServer.close()
})

server.listen(0, '127.0.0.1', () => {
const client = new Client(`http://example.org:${server.address().port}`, { connect: { lookup }, autoSelectFamily: true })

t.teardown(client.destroy.bind(client))

client.request({
path: '/',
method: 'GET'
}, (err, { statusCode, body }) => {
t.error(err)

let response = Buffer.alloc(0)

body.on('data', chunk => {
response = Buffer.concat([response, chunk])
})

body.on('end', () => {
t.strictSame(statusCode, 200)
t.strictSame(response.toString('utf-8'), 'hello')
})
})
})
})
})
}

test('with autoSelectFamily disabled the request fails when using request', (t) => {
t.plan(1)

createDnsServer('::1', '127.0.0.1', function (_, { dnsServer, lookup }) {
const server = createServer((req, res) => {
res.end('hello')
})

t.teardown(() => {
server.close()
dnsServer.close()
})

server.listen(0, '127.0.0.1', () => {
const agent = new Agent({ connect: { lookup } })

request(`http://example.org:${server.address().port}`, {
method: 'GET',
dispatcher: agent
}, (err, { statusCode, body }) => {
t.strictSame(err.code, 'ECONNREFUSED')
})
})
})
})

test('with autoSelectFamily disabled the request fails when using a client', (t) => {
t.plan(1)

createDnsServer('::1', '127.0.0.1', function (_, { dnsServer, lookup }) {
const server = createServer((req, res) => {
res.end('hello')
})

t.teardown(() => {
server.close()
dnsServer.close()
})

server.listen(0, '127.0.0.1', () => {
const client = new Client(`http://example.org:${server.address().port}`, { connect: { lookup } })
t.teardown(client.destroy.bind(client))

client.request({
path: '/',
method: 'GET'
}, (err, { statusCode, body }) => {
t.strictSame(err.code, 'ECONNREFUSED')
})
})
})
})
4 changes: 2 additions & 2 deletions test/balanced-pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

const { test } = require('tap')
const { BalancedPool, Pool, Client, errors } = require('..')
const { nodeMajor } = require('../lib/core/util')
const { createServer } = require('http')
const { promisify } = require('util')
const semver = require('semver')

test('throws when factory is not a function', (t) => {
t.plan(2)
Expand Down Expand Up @@ -437,7 +437,7 @@ const cases = [
expectedRatios: [0.34, 0.34, 0.32],

// Skip because the behavior of Node.js has changed
skip: semver.satisfies(process.version, '>= 19.0.0')
skip: nodeMajor >= 19
},

// 8
Expand Down
Loading

0 comments on commit 16a8d68

Please sign in to comment.