-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: limit concurrent HTTP requests in browser #2304
Changes from all commits
5c5f891
45465ff
7e422e9
7171981
6c404ea
3ea284d
d5fd304
a6d09c1
1999de7
c4d0a83
a074cbb
4af8560
b5038ec
857769a
79ae97f
0f6b543
e29e42b
c239abd
e7534ff
62dbdc3
e49143e
af208c3
ad65329
11ab304
ddd49ce
2e82b2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only cache when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't: negative DNSLink result is returned as HTTP 500: |
||
} | ||
] | ||
} | ||
}) | ||
|
||
const ipfsPath = (response) => { | ||
if (response.Path) return response.Path | ||
throw new Error(response.Message) | ||
} | ||
|
||
module.exports = (fqdn, opts = {}, cb) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this be promise-first please? E.g. make this an async method but export a nodeified version. This will make future refactors to remove callbacks easier. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think its already done? the async method is inside: const resolveDnslink = async (fqdn, opts = {}) => {
...
}
return nodeify(resolveDnslink(fqdn, opts), cb) when we refactor to remove callbacks we just remove the wrapper. |
||
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) | ||
} |
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One day we will use something like https://github.com/ipfs/js-ipfs-http-client/blob/master/src/lib/stream-to-iterable.js to stream the response.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That day is today. Well, yesterday. Since we merged #2379 you should be able to pass iterables to
ipfs.add
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@achingbrain seems that
ky
does not expose ReadableStream yet:sindresorhus/ky#3 (comment)
To avoid blocking this PR, I'd keep this as-is and add streaming in separate PR, after support in
ky
lands.