From cf38aead2b0cb0b5f269daf265a2b868c50a81f8 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 10 Sep 2019 18:06:16 +0200 Subject: [PATCH] fix: limit concurrent HTTP requests in browser (#2304) Adds limit of concurrent HTTP requests sent to remote API by dns and preload calls when running in web browser contexts. Browsers limit connections per host (~6). This change mitigates the problem of expensive and long running calls of one type exhausting connection pool for other uses. It additionally limits the number of DNS lookup calls by introducing time-bound cache with eviction rules following what browser already do. This is similar to: https://github.com/libp2p/js-libp2p-delegated-content-routing/issues/12 --- .aegir.js | 2 +- package.json | 5 +- src/cli/commands/daemon.js | 3 +- .../components/files-regular/add-from-url.js | 49 ++++-------- src/core/config.js | 3 +- src/core/index.js | 3 +- src/core/runtime/dns-browser.js | 77 +++++++++++++------ src/core/runtime/fetch-browser.js | 3 - src/core/runtime/fetch-nodejs.js | 2 - src/core/runtime/preload-browser.js | 18 ++--- src/utils/tlru.js | 3 +- test/core/interface.spec.js | 3 - 12 files changed, 88 insertions(+), 83 deletions(-) delete mode 100644 src/core/runtime/fetch-browser.js delete mode 100644 src/core/runtime/fetch-nodejs.js diff --git a/.aegir.js b/.aegir.js index 8745b24a81..caa757d76f 100644 --- a/.aegir.js +++ b/.aegir.js @@ -10,7 +10,7 @@ const preloadNode = MockPreloadNode.createNode() const echoServer = EchoServer.createServer() module.exports = { - bundlesize: { maxSize: '689kB' }, + bundlesize: { maxSize: '692kB' }, webpack: { resolve: { mainFields: ['browser', 'main'], diff --git a/package.json b/package.json index f65201103f..b406d695a5 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,6 @@ "./src/core/runtime/add-from-fs-nodejs.js": "./src/core/runtime/add-from-fs-browser.js", "./src/core/runtime/config-nodejs.js": "./src/core/runtime/config-browser.js", "./src/core/runtime/dns-nodejs.js": "./src/core/runtime/dns-browser.js", - "./src/core/runtime/fetch-nodejs.js": "./src/core/runtime/fetch-browser.js", "./src/core/runtime/libp2p-nodejs.js": "./src/core/runtime/libp2p-browser.js", "./src/core/runtime/libp2p-pubsub-routers-nodejs.js": "./src/core/runtime/libp2p-pubsub-routers-browser.js", "./src/core/runtime/preload-nodejs.js": "./src/core/runtime/preload-browser.js", @@ -123,6 +122,8 @@ "it-to-stream": "^0.1.1", "just-safe-set": "^2.1.0", "kind-of": "^6.0.2", + "ky": "~0.13.0", + "ky-universal": "~0.3.0", "libp2p": "~0.26.1", "libp2p-bootstrap": "~0.9.3", "libp2p-crypto": "~0.16.0", @@ -151,7 +152,7 @@ "multicodec": "~0.5.5", "multihashes": "~0.4.14", "multihashing-async": "~0.6.0", - "node-fetch": "^2.3.0", + "p-queue": "^6.1.0", "peer-book": "~0.9.0", "peer-id": "~0.12.3", "peer-info": "~0.15.0", diff --git a/src/cli/commands/daemon.js b/src/cli/commands/daemon.js index 7bc4ee518c..a0b4aa0c8b 100644 --- a/src/cli/commands/daemon.js +++ b/src/cli/commands/daemon.js @@ -3,6 +3,7 @@ const os = require('os') const toUri = require('multiaddr-to-uri') const { ipfsPathHelp } = require('../utils') +const { isTest } = require('ipfs-utils/src/env') module.exports = { command: 'daemon', @@ -27,7 +28,7 @@ module.exports = { }) .option('enable-preload', { type: 'boolean', - default: true + default: !isTest // preload by default, unless in test env }) }, diff --git a/src/core/components/files-regular/add-from-url.js b/src/core/components/files-regular/add-from-url.js index 73697aa2cf..c9207c98cc 100644 --- a/src/core/components/files-regular/add-from-url.js +++ b/src/core/components/files-regular/add-from-url.js @@ -1,41 +1,22 @@ 'use strict' const { URL } = require('iso-url') -const fetch = require('../../runtime/fetch-nodejs') - -module.exports = (self) => { - return async (url, options, callback) => { - if (typeof options === 'function') { - callback = options - options = {} - } - - let files - - try { - const parsedUrl = new URL(url) - const res = await fetch(url) - - if (!res.ok) { - throw new Error('unexpected status code: ' + res.status) - } - - // TODO: use res.body when supported - const content = Buffer.from(await res.arrayBuffer()) - const path = decodeURIComponent(parsedUrl.pathname.split('/').pop()) - - files = await self.add({ content, path }, options) - } catch (err) { - if (callback) { - return callback(err) - } - throw err - } +const nodeify = require('promise-nodeify') +const { default: ky } = require('ky-universal') + +module.exports = (ipfs) => { + const addFromURL = async (url, opts = {}) => { + const res = await ky.get(url) + const path = decodeURIComponent(new URL(res.url).pathname.split('/').pop()) + const content = Buffer.from(await res.arrayBuffer()) + return ipfs.add({ content, path }, opts) + } - if (callback) { - callback(null, files) + return (name, opts = {}, cb) => { + if (typeof opts === 'function') { + cb = opts + opts = {} } - - return files + return nodeify(addFromURL(name, opts), cb) } } diff --git a/src/core/config.js b/src/core/config.js index c41fcaf737..ba419d47a8 100644 --- a/src/core/config.js +++ b/src/core/config.js @@ -3,6 +3,7 @@ const Multiaddr = require('multiaddr') const mafmt = require('mafmt') const { struct, superstruct } = require('superstruct') +const { isTest } = require('ipfs-utils/src/env') const { optional, union } = struct const s = superstruct({ @@ -31,7 +32,7 @@ const configSchema = s({ enabled: 'boolean?', addresses: optional(s(['multiaddr'])), interval: 'number?' - }, { enabled: true, interval: 30 * 1000 }), + }, { enabled: !isTest, interval: 30 * 1000 }), init: optional(union(['boolean', s({ bits: 'number?', emptyRepo: 'boolean?', diff --git a/src/core/index.js b/src/core/index.js index 7293ee8643..1d2483a3b9 100644 --- a/src/core/index.js +++ b/src/core/index.js @@ -27,6 +27,7 @@ const defaultRepo = require('./runtime/repo-nodejs') const preload = require('./preload') const mfsPreload = require('./mfs-preload') const ipldOptions = require('./runtime/ipld-nodejs') +const { isTest } = require('ipfs-utils/src/env') /** * @typedef { import("./ipns/index") } IPNS @@ -47,7 +48,7 @@ class IPFS extends EventEmitter { start: true, EXPERIMENTAL: {}, preload: { - enabled: true, + enabled: !isTest, // preload by default, unless in test env addresses: [ '/dnsaddr/node0.preload.ipfs.io/https', '/dnsaddr/node1.preload.ipfs.io/https' diff --git a/src/core/runtime/dns-browser.js b/src/core/runtime/dns-browser.js index 74d0a14cdb..dd2bdba338 100644 --- a/src/core/runtime/dns-browser.js +++ b/src/core/runtime/dns-browser.js @@ -1,33 +1,62 @@ -/* global self */ +/* eslint-env browser */ 'use strict' -module.exports = (domain, opts, callback) => { +const TLRU = require('../../utils/tlru') +const { default: PQueue } = require('p-queue') +const { default: ky } = require('ky-universal') +const nodeify = require('promise-nodeify') + +// Avoid sending multiple queries for the same hostname by caching results +const cache = new TLRU(1000) +// TODO: /api/v0/dns does not return TTL yet: https://github.com/ipfs/go-ipfs/issues/5884 +// However we know browsers themselves cache DNS records for at least 1 minute, +// which acts a provisional default ttl: https://stackoverflow.com/a/36917902/11518426 +const ttl = 60 * 1000 + +// browsers limit concurrent connections per host, +// we don't want preload calls to exhaust the limit (~6) +const httpQueue = new PQueue({ concurrency: 4 }) + +// Delegated HTTP resolver sending DNSLink queries to ipfs.io +// TODO: replace hardcoded host with configurable DNS over HTTPS: https://github.com/ipfs/js-ipfs/issues/2212 +const api = ky.create({ + prefixUrl: 'https://ipfs.io/api/v0/', + hooks: { + afterResponse: [ + async (input, options, response) => { + const query = new URL(response.url).search.slice(1) + const json = await response.json() + cache.set(query, json, ttl) + } + ] + } +}) + +const ipfsPath = (response) => { + if (response.Path) return response.Path + throw new Error(response.Message) +} + +module.exports = (fqdn, opts = {}, cb) => { if (typeof opts === 'function') { - callback = opts + cb = opts opts = {} } + const resolveDnslink = async (fqdn, opts = {}) => { + const searchParams = new URLSearchParams(opts) + searchParams.set('arg', fqdn) - opts = opts || {} - - domain = encodeURIComponent(domain) - let url = `https://ipfs.io/api/v0/dns?arg=${domain}` + // try cache first + const query = searchParams.toString() + if (!opts.nocache && cache.has(query)) { + const response = cache.get(query) + return ipfsPath(response) + } - Object.keys(opts).forEach(prop => { - url += `&${encodeURIComponent(prop)}=${encodeURIComponent(opts[prop])}` - }) + // fallback to delegated DNS resolver + const response = await httpQueue.add(() => api.get('dns', { searchParams }).json()) + return ipfsPath(response) + } - self.fetch(url, { mode: 'cors' }) - .then((response) => { - return response.json() - }) - .then((response) => { - if (response.Path) { - return callback(null, response.Path) - } else { - return callback(new Error(response.Message)) - } - }) - .catch((error) => { - callback(error) - }) + return nodeify(resolveDnslink(fqdn, opts), cb) } diff --git a/src/core/runtime/fetch-browser.js b/src/core/runtime/fetch-browser.js deleted file mode 100644 index b2f604db7c..0000000000 --- a/src/core/runtime/fetch-browser.js +++ /dev/null @@ -1,3 +0,0 @@ -/* eslint-env browser */ -'use strict' -module.exports = fetch diff --git a/src/core/runtime/fetch-nodejs.js b/src/core/runtime/fetch-nodejs.js deleted file mode 100644 index ca77ad8310..0000000000 --- a/src/core/runtime/fetch-nodejs.js +++ /dev/null @@ -1,2 +0,0 @@ -'use strict' -module.exports = require('node-fetch') diff --git a/src/core/runtime/preload-browser.js b/src/core/runtime/preload-browser.js index 81407f483f..81d2ad786e 100644 --- a/src/core/runtime/preload-browser.js +++ b/src/core/runtime/preload-browser.js @@ -1,27 +1,25 @@ /* eslint-env browser */ 'use strict' +const { default: PQueue } = require('p-queue') +const { default: ky } = require('ky-universal') const debug = require('debug') const log = debug('ipfs:preload') log.error = debug('ipfs:preload:error') +// browsers limit concurrent connections per host, +// we don't want preload calls to exhaust the limit (~6) +const httpQueue = new PQueue({ concurrency: 4 }) + module.exports = function preload (url, callback) { log(url) const controller = new AbortController() const signal = controller.signal + const cb = () => setImmediate(callback) // https://github.com/ipfs/js-ipfs/pull/2304#discussion_r320700893 - fetch(url, { signal }) - .then(res => { - if (!res.ok) { - log.error('failed to preload', url, res.status, res.statusText) - throw new Error(`failed to preload ${url}`) - } - return res.text() - }) - .then(() => callback()) - .catch(callback) + httpQueue.add(() => ky.get(url, { signal })).then(cb, cb) return { cancel: () => controller.abort() diff --git a/src/utils/tlru.js b/src/utils/tlru.js index ba3b26e8c6..569015c038 100644 --- a/src/utils/tlru.js +++ b/src/utils/tlru.js @@ -33,8 +33,9 @@ class TLRU { this.lru.remove(key) return undefined } + return value.value } - return value.value + return undefined } /** diff --git a/test/core/interface.spec.js b/test/core/interface.spec.js index a0f06d9580..10e5c225bd 100644 --- a/test/core/interface.spec.js +++ b/test/core/interface.spec.js @@ -64,9 +64,6 @@ describe('interface-ipfs-core tests', function () { }, { name: 'addFromFs', reason: 'Not designed to run in the browser' - }, { - name: 'addFromURL', - reason: 'Not designed to run in the browser' }] })