From b83764c0de82781dcafb15e10507d7fee2b8cae4 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 3 May 2022 10:18:48 +0200 Subject: [PATCH 1/4] Make the dispatcher truly global --- .husky/pre-commit | 2 +- index.js | 15 ++++++++++++--- package.json | 1 + test/agent.js | 40 +++++++++++----------------------------- 4 files changed, 25 insertions(+), 33 deletions(-) diff --git a/.husky/pre-commit b/.husky/pre-commit index d37daa075e2..b2302c0a1f9 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -1,4 +1,4 @@ #!/bin/sh . "$(dirname "$0")/_/husky.sh" -npx --no-install lint-staged +npx --no-install --yes lint-staged diff --git a/index.js b/index.js index 888eb6a5a4f..a3f34079cf3 100644 --- a/index.js +++ b/index.js @@ -20,6 +20,8 @@ const nodeVersion = process.versions.node.split('.') const nodeMajor = Number(nodeVersion[0]) const nodeMinor = Number(nodeVersion[1]) +const globalDispatcher = Symbol.for('undici.globalDispatcher') + Object.assign(Dispatcher.prototype, api) module.exports.Dispatcher = Dispatcher @@ -32,17 +34,24 @@ module.exports.ProxyAgent = ProxyAgent module.exports.buildConnector = buildConnector module.exports.errors = errors -let globalDispatcher = new Agent() +if (getGlobalDispatcher() === undefined) { + setGlobalDispatcher(new Agent()) +} function setGlobalDispatcher (agent) { if (!agent || typeof agent.dispatch !== 'function') { throw new InvalidArgumentError('Argument agent must implement Agent') } - globalDispatcher = agent + Object.defineProperty(globalThis, globalDispatcher, { + value: agent, + writable: true, + enumerable: false, + configurable: false + }) } function getGlobalDispatcher () { - return globalDispatcher + return globalThis[globalDispatcher] } function makeDispatcher (fn) { diff --git a/package.json b/package.json index b5b34d1a76b..796ccfd99ba 100644 --- a/package.json +++ b/package.json @@ -79,6 +79,7 @@ "formdata-node": "^4.3.1", "https-pem": "^2.0.0", "husky": "^7.0.2", + "import-fresh": "^3.3.0", "jest": "^28.0.1", "jsfuzz": "^1.0.15", "mocha": "^9.1.1", diff --git a/test/agent.js b/test/agent.js index f341a16dea8..bb85a55bc21 100644 --- a/test/agent.js +++ b/test/agent.js @@ -10,8 +10,10 @@ const { request, stream, pipeline, - setGlobalDispatcher + setGlobalDispatcher, + getGlobalDispatcher } = require('../') +const importFresh = require('import-fresh') test('setGlobalDispatcher', t => { t.plan(2) @@ -645,34 +647,6 @@ test('drain', t => { }) }) -// Port 80 is no accessible on CI. -// test('agent works with port 80', t => { -// t.plan(1) - -// const server = http.createServer((req, res) => { -// res.setHeader('Content-Type', 'text/plain') -// res.end() -// }) - -// t.teardown(server.close.bind(server)) - -// server.listen(80, async () => { -// const dispatcher = new Agent() - -// const origin = `http://localhost:${server.address().port}` - -// try { -// const { body } = await dispatcher.request({ origin, method: 'GET', path: '/' }) - -// body.on('end', () => { -// t.pass() -// }).resume() -// } catch (err) { -// t.error(err) -// } -// }) -// }) - test('global api', t => { t.plan(6 * 2) @@ -724,3 +698,11 @@ test('connect is not valid', t => { t.throws(() => new Agent({ connect: false }), errors.InvalidArgumentError, 'connect must be a function or an object') }) + +test('the dispatcher is truly global', t => { + const agent = getGlobalDispatcher() + const undiciFresh = importFresh('../index.js') + t.not(getGlobalDispatcher, undiciFresh.getGlobalDispatcher) + t.equal(agent, undiciFresh.getGlobalDispatcher()) + t.end() +}) From 7e911c3df66a836c23af8fb41b5dee3dc0951c7b Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 3 May 2022 11:29:57 +0200 Subject: [PATCH 2/4] make sure the global dispatcher is used for Node.js fetch --- index-fetch.js | 8 +++----- index.js | 23 +---------------------- lib/global.js | 30 ++++++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 27 deletions(-) create mode 100644 lib/global.js diff --git a/index-fetch.js b/index-fetch.js index aaf70af701d..2937144bbf8 100644 --- a/index-fetch.js +++ b/index-fetch.js @@ -1,12 +1,10 @@ 'use strict' -const Agent = require('./lib/agent') - -const globalDispatcher = new Agent() - +const { getGlobalDispatcher } = require('./lib/global') const fetchImpl = require('./lib/fetch') + module.exports.fetch = async function fetch (resource) { - return fetchImpl.apply(globalDispatcher, arguments) + return fetchImpl.apply(getGlobalDispatcher(), arguments) } module.exports.FormData = require('./lib/fetch/formdata').FormData module.exports.Headers = require('./lib/fetch/headers').Headers diff --git a/index.js b/index.js index a3f34079cf3..a77e40eb9f0 100644 --- a/index.js +++ b/index.js @@ -15,13 +15,12 @@ const MockAgent = require('./lib/mock/mock-agent') const MockPool = require('./lib/mock/mock-pool') const mockErrors = require('./lib/mock/mock-errors') const ProxyAgent = require('./lib/proxy-agent') +const { getGlobalDispatcher, setGlobalDispatcher } = require('./lib/global') const nodeVersion = process.versions.node.split('.') const nodeMajor = Number(nodeVersion[0]) const nodeMinor = Number(nodeVersion[1]) -const globalDispatcher = Symbol.for('undici.globalDispatcher') - Object.assign(Dispatcher.prototype, api) module.exports.Dispatcher = Dispatcher @@ -34,26 +33,6 @@ module.exports.ProxyAgent = ProxyAgent module.exports.buildConnector = buildConnector module.exports.errors = errors -if (getGlobalDispatcher() === undefined) { - setGlobalDispatcher(new Agent()) -} - -function setGlobalDispatcher (agent) { - if (!agent || typeof agent.dispatch !== 'function') { - throw new InvalidArgumentError('Argument agent must implement Agent') - } - Object.defineProperty(globalThis, globalDispatcher, { - value: agent, - writable: true, - enumerable: false, - configurable: false - }) -} - -function getGlobalDispatcher () { - return globalThis[globalDispatcher] -} - function makeDispatcher (fn) { return (url, opts, handler) => { if (typeof opts === 'function') { diff --git a/lib/global.js b/lib/global.js new file mode 100644 index 00000000000..6a28b916019 --- /dev/null +++ b/lib/global.js @@ -0,0 +1,30 @@ +'use strict' + +const globalDispatcher = Symbol.for('undici.globalDispatcher') +const { InvalidArgumentError } = require('./core/errors') +const Agent = require('./agent') + +if (getGlobalDispatcher() === undefined) { + setGlobalDispatcher(new Agent()) +} + +function setGlobalDispatcher (agent) { + if (!agent || typeof agent.dispatch !== 'function') { + throw new InvalidArgumentError('Argument agent must implement Agent') + } + Object.defineProperty(globalThis, globalDispatcher, { + value: agent, + writable: true, + enumerable: false, + configurable: false + }) +} + +function getGlobalDispatcher () { + return globalThis[globalDispatcher] +} + +module.exports = { + setGlobalDispatcher, + getGlobalDispatcher +} From 7eecb3015a4bdb2c835d6f5ad6c8811bada94cef Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 3 May 2022 12:04:00 +0200 Subject: [PATCH 3/4] Fix test --- test/agent.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/agent.js b/test/agent.js index bb85a55bc21..0d3c84d9d53 100644 --- a/test/agent.js +++ b/test/agent.js @@ -702,7 +702,6 @@ test('connect is not valid', t => { test('the dispatcher is truly global', t => { const agent = getGlobalDispatcher() const undiciFresh = importFresh('../index.js') - t.not(getGlobalDispatcher, undiciFresh.getGlobalDispatcher) t.equal(agent, undiciFresh.getGlobalDispatcher()) t.end() }) From fcb7dad40f10aee0c0e4411144a165420aed6f40 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 4 May 2022 10:15:32 +0200 Subject: [PATCH 4/4] added version number --- lib/global.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/global.js b/lib/global.js index 6a28b916019..18bfd73cc92 100644 --- a/lib/global.js +++ b/lib/global.js @@ -1,6 +1,8 @@ 'use strict' -const globalDispatcher = Symbol.for('undici.globalDispatcher') +// We include a version number for the Dispatcher API. In case of breaking changes, +// this version number must be increased to avoid conflicts. +const globalDispatcher = Symbol.for('undici.globalDispatcher.1') const { InvalidArgumentError } = require('./core/errors') const Agent = require('./agent')