From b029fd2092f3fe9a34c6d537e7bfbc161786ea88 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Thu, 25 Jul 2019 13:29:40 +0100 Subject: [PATCH 01/20] feat: enable pubsub in the browser License: MIT Signed-off-by: Alan Shaw --- package.json | 8 +- src/lib/callbackify.js | 17 +++ src/lib/configure.browser.js | 46 +++++++ src/lib/configure.js | 44 ++++++ src/lib/fetch.js | 53 ++++++++ src/lib/multiaddr.js | 16 +++ src/lib/querystring.js | 16 +++ src/pubsub.js | 212 ----------------------------- src/pubsub/index.js | 16 +++ src/pubsub/ls.js | 20 +++ src/pubsub/peers.js | 29 ++++ src/pubsub/publish.js | 51 +++++++ src/pubsub/subscribe.js | 67 +++++++++ src/pubsub/subscription-tracker.js | 52 +++++++ src/pubsub/unsubscribe.js | 10 ++ src/utils/pubsub-message-stream.js | 34 ----- src/utils/pubsub-message-utils.js | 39 ------ src/utils/stringlist-to-array.js | 9 -- test/interface.spec.js | 14 +- 19 files changed, 450 insertions(+), 303 deletions(-) create mode 100644 src/lib/callbackify.js create mode 100644 src/lib/configure.browser.js create mode 100644 src/lib/configure.js create mode 100644 src/lib/fetch.js create mode 100644 src/lib/multiaddr.js create mode 100644 src/lib/querystring.js delete mode 100644 src/pubsub.js create mode 100644 src/pubsub/index.js create mode 100644 src/pubsub/ls.js create mode 100644 src/pubsub/peers.js create mode 100644 src/pubsub/publish.js create mode 100644 src/pubsub/subscribe.js create mode 100644 src/pubsub/subscription-tracker.js create mode 100644 src/pubsub/unsubscribe.js delete mode 100644 src/utils/pubsub-message-stream.js delete mode 100644 src/utils/pubsub-message-utils.js delete mode 100644 src/utils/stringlist-to-array.js diff --git a/package.json b/package.json index 33cd7bde1..5e28ecf81 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,8 @@ "browser": { "glob": false, "fs": false, - "stream": "readable-stream" + "stream": "readable-stream", + "./src/lib/configure.js": "./src/lib/configure.browser.js" }, "repository": "github:ipfs/js-ipfs-http-client", "scripts": { @@ -33,6 +34,7 @@ "coverage": "npx nyc -r html npm run test:node -- --bail" }, "dependencies": { + "abort-controller": "^3.0.0", "async": "^2.6.1", "bignumber.js": "^9.0.0", "bl": "^3.0.0", @@ -44,6 +46,7 @@ "detect-node": "^2.0.4", "end-of-stream": "^1.4.1", "err-code": "^1.1.2", + "explain-error": "^1.0.4", "flatmap": "0.0.3", "glob": "^7.1.3", "ipfs-block": "~0.8.1", @@ -56,6 +59,7 @@ "is-stream": "^2.0.0", "iso-stream-http": "~0.1.2", "iso-url": "~0.4.6", + "iterable-ndjson": "^1.1.0", "just-kebab-case": "^1.1.0", "just-map-keys": "^1.1.0", "kind-of": "^6.0.2", @@ -65,6 +69,7 @@ "multicodec": "~0.5.1", "multihashes": "~0.4.14", "ndjson": "github:hugomrdias/ndjson#feat/readable-stream3", + "node-fetch": "^2.6.0", "once": "^1.4.0", "peer-id": "~0.12.3", "peer-info": "~0.15.1", @@ -74,6 +79,7 @@ "pull-to-stream": "~0.1.1", "pump": "^3.0.0", "qs": "^6.5.2", + "querystring": "^0.2.0", "readable-stream": "^3.1.1", "stream-to-pull-stream": "^1.7.2", "tar-stream": "^2.0.1", diff --git a/src/lib/callbackify.js b/src/lib/callbackify.js new file mode 100644 index 000000000..3a041612d --- /dev/null +++ b/src/lib/callbackify.js @@ -0,0 +1,17 @@ +'use strict' + +module.exports = (fn, opts) => { + opts = opts || {} + // Min number of non-callback args + opts.minArgs = opts.minArgs == null ? 0 : opts.minArgs + + return (...args) => { + const cb = args[args.length - 1] + + if (typeof cb !== 'function' || args.length === opts.minArgs) { + return fn(...args) + } + + fn(...args.slice(0, -1)).then(res => cb(null, res), cb) + } +} diff --git a/src/lib/configure.browser.js b/src/lib/configure.browser.js new file mode 100644 index 000000000..2943915cc --- /dev/null +++ b/src/lib/configure.browser.js @@ -0,0 +1,46 @@ +'use strict' +/* eslint-env browser */ + +const { toUri } = require('./multiaddr') + +// Set default configuration and call create function with them +module.exports = create => config => { + config = config || {} + + if (typeof config === 'string') { + config = { apiAddr: config } + } + + // Multiaddr instance + if (config.constructor && config.constructor.isMultiaddr) { + config = { apiAddr: config } + } + + config.fetch = config.fetch || require('./fetch').fetch + config.apiAddr = (config.apiAddr || getDefaultApiAddr(config)).toString() + config.apiAddr = config.apiAddr.startsWith('/') + ? toUri(config.apiAddr) + : config.apiAddr + config.apiPath = config.apiPath || config['api-path'] || '/api/v0' + + if (config.apiPath.endsWith('/')) { + config.apiPath = config.apiPath.slice(0, -1) + } + + config.headers = new Headers(config.headers) + + return create(config) +} + +function getDefaultApiAddr ({ protocol, host, port }) { + if (!protocol) { + protocol = location.protocol.startsWith('http') + ? location.protocol.split(':')[0] + : 'http' + } + + host = host || location.hostname + port = port || location.port + + return `${protocol}://${host}${port ? ':' + port : ''}` +} diff --git a/src/lib/configure.js b/src/lib/configure.js new file mode 100644 index 000000000..3557945ee --- /dev/null +++ b/src/lib/configure.js @@ -0,0 +1,44 @@ +'use strict' + +const { Headers } = require('node-fetch') +const { toUri } = require('./multiaddr') +const pkg = require('../../package.json') + +// Set default configuration and call create function with them +module.exports = create => config => { + config = config || {} + + if (typeof config === 'string') { + config = { apiAddr: config } + } + + // Multiaddr instance + if (config.constructor && config.constructor.isMultiaddr) { + config = { apiAddr: config } + } + + config.fetch = config.fetch || require('./fetch').fetch + + if (config.protocol || config.host || config.port) { + const port = config.port ? `:${config.port}` : '' + config.apiAddr = `${config.protocol || 'http'}://${config.host || 'localhost'}${port}` + } + + config.apiAddr = (config.apiAddr || 'http://localhost:5001').toString() + config.apiAddr = config.apiAddr.startsWith('/') + ? toUri(config.apiAddr) + : config.apiAddr + config.apiPath = config.apiPath || config['api-path'] || '/api/v0' + + if (config.apiPath.endsWith('/')) { + config.apiPath = config.apiPath.slice(0, -1) + } + + config.headers = new Headers(config.headers) + + if (!config.headers.has('User-Agent')) { + config.headers.append('User-Agent', `${pkg.name}/${pkg.version}`) + } + + return create(config) +} diff --git a/src/lib/fetch.js b/src/lib/fetch.js new file mode 100644 index 000000000..b3cf03cbb --- /dev/null +++ b/src/lib/fetch.js @@ -0,0 +1,53 @@ +'use strict' + +const explain = require('explain-error') + +exports.fetch = require('node-fetch') + +// Ensure fetch response is ok (200) +// and if not, attempt to JSON parse body, extract error message and throw +exports.ok = async res => { + res = await res + + if (!res.ok) { + const { status } = res + const defaultMsg = `unexpected status ${status}` + let msg + try { + let data = await res.text() + try { + data = JSON.parse(data) + msg = data.message || data.Message + } catch (err) { + msg = data + } + } catch (err) { + throw Object.assign(explain(err, defaultMsg), { status }) + } + throw Object.assign(new Error(msg || defaultMsg), { status }) + } + + return res +} + +exports.toIterable = body => { + if (body[Symbol.asyncIterator]) return body + + if (body.getReader) { + return (async function * () { + const reader = body.getReader() + + try { + while (true) { + const { done, value } = await reader.read() + if (done) return + yield value + } + } finally { + reader.releaseLock() + } + })() + } + + throw new Error('unknown stream') +} diff --git a/src/lib/multiaddr.js b/src/lib/multiaddr.js new file mode 100644 index 000000000..1ccfe72c8 --- /dev/null +++ b/src/lib/multiaddr.js @@ -0,0 +1,16 @@ +// Convert a multiaddr to a URI +// Assumes multiaddr is in a format that can be converted to a HTTP(s) URI +exports.toUri = ma => { + const parts = `${ma}`.split('/') + const port = getPort(parts) + return `${getProtocol(parts)}://${parts[2]}${port == null ? '' : ':' + port}` +} + +function getProtocol (maParts) { + return maParts.indexOf('https') === -1 ? 'http' : 'https' +} + +function getPort (maParts) { + const tcpIndex = maParts.indexOf('tcp') + return tcpIndex === -1 ? null : maParts[tcpIndex + 1] +} diff --git a/src/lib/querystring.js b/src/lib/querystring.js new file mode 100644 index 000000000..df35b0a6e --- /dev/null +++ b/src/lib/querystring.js @@ -0,0 +1,16 @@ +'use strict' + +const QueryString = require('querystring') + +// Convert an object to a query string INCLUDING leading ? +// Excludes null/undefined values +exports.objectToQuery = obj => { + if (!obj) return '' + + const qs = Object.entries(obj).reduce((obj, [key, value]) => { + if (value != null) obj[key] = value + return obj + }, {}) + + return Object.keys(qs).length ? `?${QueryString.stringify(qs)}` : '' +} diff --git a/src/pubsub.js b/src/pubsub.js deleted file mode 100644 index 6b298351d..000000000 --- a/src/pubsub.js +++ /dev/null @@ -1,212 +0,0 @@ -'use strict' - -const promisify = require('promisify-es6') -const EventEmitter = require('events') -const eos = require('end-of-stream') -const isNode = require('detect-node') -const setImmediate = require('async/setImmediate') -const PubsubMessageStream = require('./utils/pubsub-message-stream') -const stringlistToArray = require('./utils/stringlist-to-array') -const moduleConfig = require('./utils/module-config') - -const NotSupportedError = () => new Error('pubsub is currently not supported when run in the browser') - -/* Public API */ -module.exports = (arg) => { - const send = moduleConfig(arg) - - /* Internal subscriptions state and functions */ - const ps = new EventEmitter() - const subscriptions = {} - ps.id = Math.random() - return { - subscribe: (topic, handler, options, callback) => { - const defaultOptions = { - discover: false - } - - if (typeof options === 'function') { - callback = options - options = defaultOptions - } - - if (!options) { - options = defaultOptions - } - - // Throw an error if ran in the browsers - if (!isNode) { - if (!callback) { - return Promise.reject(NotSupportedError()) - } - - return setImmediate(() => callback(NotSupportedError())) - } - - // promisify doesn't work as we always pass a - // function as last argument (`handler`) - if (!callback) { - return new Promise((resolve, reject) => { - subscribe(topic, handler, options, (err) => { - if (err) { - return reject(err) - } - resolve() - }) - }) - } - - subscribe(topic, handler, options, callback) - }, - unsubscribe: (topic, handler, callback) => { - if (!isNode) { - if (!callback) { - return Promise.reject(NotSupportedError()) - } - - return setImmediate(() => callback(NotSupportedError())) - } - - if (ps.listenerCount(topic) === 0 || !subscriptions[topic]) { - const err = new Error(`Not subscribed to '${topic}'`) - - if (!callback) { - return Promise.reject(err) - } - - return setImmediate(() => callback(err)) - } - - if (!handler && !callback) { - ps.removeAllListeners(topic) - } else { - ps.removeListener(topic, handler) - } - - // Drop the request once we are actually done - if (ps.listenerCount(topic) === 0) { - if (!callback) { - return new Promise((resolve, reject) => { - // When the response stream has ended, resolve the promise - eos(subscriptions[topic].res, (err) => { - // FIXME: Artificial timeout needed to ensure unsubscribed - setTimeout(() => { - if (err) return reject(err) - resolve() - }) - }) - subscriptions[topic].req.abort() - subscriptions[topic] = null - }) - } - - // When the response stream has ended, call the callback - eos(subscriptions[topic].res, (err) => { - // FIXME: Artificial timeout needed to ensure unsubscribed - setTimeout(() => callback(err)) - }) - subscriptions[topic].req.abort() - subscriptions[topic] = null - return - } - - if (!callback) { - return Promise.resolve() - } - - setImmediate(() => callback()) - }, - publish: promisify((topic, data, callback) => { - if (!isNode) { - return callback(NotSupportedError()) - } - - if (!Buffer.isBuffer(data)) { - return callback(new Error('data must be a Buffer')) - } - - const request = { - path: 'pubsub/pub', - args: [topic, data] - } - - send(request, callback) - }), - ls: promisify((callback) => { - if (!isNode) { - return callback(NotSupportedError()) - } - - const request = { - path: 'pubsub/ls' - } - - send.andTransform(request, stringlistToArray, callback) - }), - peers: promisify((topic, callback) => { - if (!isNode) { - return callback(NotSupportedError()) - } - - const request = { - path: 'pubsub/peers', - args: [topic] - } - - send.andTransform(request, stringlistToArray, callback) - }), - setMaxListeners (n) { - return ps.setMaxListeners(n) - } - } - - function subscribe (topic, handler, options, callback) { - ps.on(topic, handler) - - if (subscriptions[topic]) { - // TODO: should a callback error be returned? - return callback() - } - - // Request params - const request = { - path: 'pubsub/sub', - args: [topic], - qs: { - discover: options.discover - } - } - - // Start the request and transform the response - // stream to Pubsub messages stream - subscriptions[topic] = {} - subscriptions[topic].req = send.andTransform(request, PubsubMessageStream.from, (err, stream) => { - if (err) { - subscriptions[topic] = null - ps.removeListener(topic, handler) - return callback(err) - } - - subscriptions[topic].res = stream - - stream.on('data', (msg) => { - ps.emit(topic, msg) - }) - - stream.on('error', (err) => { - ps.emit('error', err) - }) - - eos(stream, (err) => { - if (err) { - ps.emit('error', err) - } - - subscriptions[topic] = null - ps.removeListener(topic, handler) - }) - - callback() - }) - } -} diff --git a/src/pubsub/index.js b/src/pubsub/index.js new file mode 100644 index 000000000..7f2897ea7 --- /dev/null +++ b/src/pubsub/index.js @@ -0,0 +1,16 @@ +const callbackify = require('../lib/callbackify') + +// This file is temporary and for compatibility with legacy usage +module.exports = (send, options) => { + if (typeof send !== 'function') { + options = send + } + + return { + ls: callbackify(require('./ls')(options)), + peers: callbackify(require('./peers')(options)), + publish: callbackify(require('./publish')(options)), + subscribe: callbackify(require('./subscribe')(options), { minArgs: 2 }), + unsubscribe: callbackify(require('./unsubscribe')(options), { minArgs: 2 }) + } +} diff --git a/src/pubsub/ls.js b/src/pubsub/ls.js new file mode 100644 index 000000000..bfbf8239e --- /dev/null +++ b/src/pubsub/ls.js @@ -0,0 +1,20 @@ +'use strict' + +const configure = require('../lib/configure') +const { ok } = require('../lib/fetch') +const { objectToQuery } = require('../lib/querystring') + +module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { + return async (options) => { + options = options || {} + + const qs = objectToQuery(options.qs) + const url = `${apiAddr}${apiPath}/pubsub/ls${qs}` + const res = await ok(fetch(url, { + signal: options.signal, + headers: options.headers || headers + })) + const data = await res.json() + return data.Strings || [] + } +}) diff --git a/src/pubsub/peers.js b/src/pubsub/peers.js new file mode 100644 index 000000000..7ee860e33 --- /dev/null +++ b/src/pubsub/peers.js @@ -0,0 +1,29 @@ +'use strict' + +const { objectToQuery } = require('../lib/querystring') +const configure = require('../lib/configure') +const { ok } = require('../lib/fetch') + +module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { + return async (topic, options) => { + if (!options && typeof topic === 'object') { + options = topic + topic = null + } + + options = options || {} + + const qs = objectToQuery({ + arg: topic, + ...(options.qs || {}) + }) + + const url = `${apiAddr}${apiPath}/pubsub/peers${qs}` + const res = await ok(fetch(url, { + signal: options.signal, + headers: options.headers || headers + })) + const data = await res.json() + return data.Strings || [] + } +}) diff --git a/src/pubsub/publish.js b/src/pubsub/publish.js new file mode 100644 index 000000000..d214ddbc8 --- /dev/null +++ b/src/pubsub/publish.js @@ -0,0 +1,51 @@ +'use strict' + +const { Buffer } = require('buffer') +const configure = require('../lib/configure') +const { objectToQuery } = require('../lib/querystring') +const { ok } = require('../lib/fetch') + +module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { + return async (topic, data, options) => { + options = options || {} + + if (!Buffer.isBuffer(data)) { + throw new Error('data must be a Buffer') + } + + let qs = objectToQuery(options.qs) + qs = qs ? `&${qs.slice(1)}` : qs + + const url = `${apiAddr}${apiPath}/pubsub/pub?arg=${encodeURIComponent(topic)}&arg=${encodeBuffer(data)}${qs}` + const res = await ok(fetch(url, { + method: 'POST', + signal: options.signal, + headers: options.headers || headers + })) + + return res.text() + } +}) + +function encodeBuffer (buf) { + let uriEncoded = '' + for (const byte of buf) { + // https://tools.ietf.org/html/rfc3986#page-14 + // ALPHA (%41-%5A and %61-%7A), DIGIT (%30-%39), hyphen (%2D), period (%2E), + // underscore (%5F), or tilde (%7E) + if ( + (byte >= 0x41 && byte <= 0x5A) || + (byte >= 0x61 && byte <= 0x7A) || + (byte >= 0x30 && byte <= 0x39) || + (byte === 0x2D) || + (byte === 0x2E) || + (byte === 0x5F) || + (byte === 0x7E) + ) { + uriEncoded += String.fromCharCode(byte) + } else { + uriEncoded += `%${byte.toString(16).padStart(2, '0')}` + } + } + return uriEncoded +} diff --git a/src/pubsub/subscribe.js b/src/pubsub/subscribe.js new file mode 100644 index 000000000..f2a303937 --- /dev/null +++ b/src/pubsub/subscribe.js @@ -0,0 +1,67 @@ +'use strict' + +const ndjson = require('iterable-ndjson') +const explain = require('explain-error') +const bs58 = require('bs58') +const { Buffer } = require('buffer') +const { objectToQuery } = require('../lib/querystring') +const configure = require('../lib/configure') +const { ok, toIterable } = require('../lib/fetch') +const SubscriptionTracker = require('./subscription-tracker') + +module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { + const subsTracker = SubscriptionTracker.singleton() + + return async (topic, handler, options) => { + options = options || {} + options.signal = subsTracker.subscribe(topic, handler, options.signal) + + const qs = objectToQuery({ + arg: topic, + discover: options.discover, + ...(options.qs || {}) + }) + + const url = `${apiAddr}${apiPath}/pubsub/sub${qs}` + let res + + try { + res = await ok(fetch(url, { + method: 'POST', + signal: options.signal, + headers: options.headers || headers + })) + } catch (err) { // Initial subscribe fail, ensure we clean up + subsTracker.unsubscribe(topic, handler) + throw err + } + + // eslint-disable-next-line no-console + const onError = options.onError || (err => console.error(err)) + + ;(async () => { + try { + for await (const msg of ndjson(toIterable(res.body))) { + try { + handler({ + from: bs58.encode(Buffer.from(msg.from, 'base64')).toString(), + data: Buffer.from(msg.data, 'base64'), + seqno: Buffer.from(msg.seqno, 'base64'), + topicIDs: msg.topicIDs + }) + } catch (err) { + onError(explain(err, 'Failed to parse pubsub message'), false) // Not fatal + } + } + } catch (err) { + // FIXME: In testing with Chrome, err.type is undefined (should not be!) + // Temporarily use the name property instead. + if (err.type !== 'aborted' && err.name !== 'AbortError') { + onError(err, true) // Fatal + } + } finally { + subsTracker.unsubscribe(topic, handler) + } + })() + } +}) diff --git a/src/pubsub/subscription-tracker.js b/src/pubsub/subscription-tracker.js new file mode 100644 index 000000000..bbd7c2d7a --- /dev/null +++ b/src/pubsub/subscription-tracker.js @@ -0,0 +1,52 @@ +'use strict' + +const AbortController = require('abort-controller') + +class SubscriptionTracker { + constructor () { + this._subs = new Map() + } + + static singleton () { + if (SubscriptionTracker.instance) return SubscriptionTracker.instance + SubscriptionTracker.instance = new SubscriptionTracker() + return SubscriptionTracker.instance + } + + subscribe (topic, handler, signal) { + const topicSubs = this._subs.get(topic) || [] + + if (topicSubs.find(s => s.handler === handler)) { + throw new Error(`Already subscribed to ${topic} with this handler`) + } + + // Create controller so a call to unsubscribe can cancel the request + const controller = new AbortController() + + this._subs.set(topic, [{ handler, controller }].concat(topicSubs)) + + // If there is an external signal, forward the abort event + if (signal) { + signal.addEventListener('abort', () => this.unsubscribe(topic, handler)) + } + + return controller.signal + } + + unsubscribe (topic, handler) { + const subs = this._subs.get(topic) || [] + let unsubs + + if (handler) { + this._subs.set(topic, subs.filter(s => s.handler !== handler)) + unsubs = subs.filter(s => s.handler === handler) + } else { + this._subs.set(topic, []) + unsubs = subs + } + + unsubs.forEach(s => s.controller.abort()) + } +} + +module.exports = SubscriptionTracker diff --git a/src/pubsub/unsubscribe.js b/src/pubsub/unsubscribe.js new file mode 100644 index 000000000..a8bd14944 --- /dev/null +++ b/src/pubsub/unsubscribe.js @@ -0,0 +1,10 @@ +'use strict' + +const configure = require('../lib/configure') +const SubscriptionTracker = require('./subscription-tracker') + +module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { + const subsTracker = SubscriptionTracker.singleton() + // eslint-disable-next-line require-await + return async (topic, handler) => subsTracker.unsubscribe(topic, handler) +}) diff --git a/src/utils/pubsub-message-stream.js b/src/utils/pubsub-message-stream.js deleted file mode 100644 index d5925f714..000000000 --- a/src/utils/pubsub-message-stream.js +++ /dev/null @@ -1,34 +0,0 @@ -'use strict' - -const TransformStream = require('readable-stream').Transform -const PubsubMessage = require('./pubsub-message-utils') - -class PubsubMessageStream extends TransformStream { - constructor (options) { - const opts = Object.assign(options || {}, { objectMode: true }) - super(opts) - } - - static from (inputStream, callback) { - const outputStream = inputStream.pipe(new PubsubMessageStream()) - inputStream.on('end', () => outputStream.emit('end')) - callback(null, outputStream) - } - - _transform (obj, enc, callback) { - // go-ipfs returns '{}' as the very first object atm, we skip that - if (Object.keys(obj).length === 0) { - return callback() - } - - try { - const msg = PubsubMessage.deserialize(obj, 'base64') - this.push(msg) - callback() - } catch (err) { - return callback(err) - } - } -} - -module.exports = PubsubMessageStream diff --git a/src/utils/pubsub-message-utils.js b/src/utils/pubsub-message-utils.js deleted file mode 100644 index 53d1e397a..000000000 --- a/src/utils/pubsub-message-utils.js +++ /dev/null @@ -1,39 +0,0 @@ -'use strict' - -const bs58 = require('bs58') - -module.exports = { - deserialize (data, enc) { - enc = enc ? enc.toLowerCase() : 'json' - - if (enc === 'json') { - return deserializeFromJson(data) - } else if (enc === 'base64') { - return deserializeFromBase64(data) - } - - throw new Error(`Unsupported encoding: '${enc}'`) - } -} - -function deserializeFromJson (data) { - const json = JSON.parse(data) - return deserializeFromBase64(json) -} - -function deserializeFromBase64 (obj) { - if (!isPubsubMessage(obj)) { - throw new Error(`Not a pubsub message`) - } - - return { - from: bs58.encode(Buffer.from(obj.from, 'base64')).toString(), - seqno: Buffer.from(obj.seqno, 'base64'), - data: Buffer.from(obj.data, 'base64'), - topicIDs: obj.topicIDs || obj.topicCIDs - } -} - -function isPubsubMessage (obj) { - return obj && obj.from && obj.seqno && obj.data && (obj.topicIDs || obj.topicCIDs) -} diff --git a/src/utils/stringlist-to-array.js b/src/utils/stringlist-to-array.js deleted file mode 100644 index df28ee6df..000000000 --- a/src/utils/stringlist-to-array.js +++ /dev/null @@ -1,9 +0,0 @@ -'use strict' - -// Converts a go-ipfs "stringList" to an array -// { Strings: ['A', 'B'] } --> ['A', 'B'] -function stringlistToArray (res, cb) { - cb(null, res.Strings || []) -} - -module.exports = stringlistToArray diff --git a/test/interface.spec.js b/test/interface.spec.js index 220c79aa2..9305acc4c 100644 --- a/test/interface.spec.js +++ b/test/interface.spec.js @@ -270,19 +270,17 @@ describe('interface-ipfs-core tests', () => { initOptions: { bits: 1024 } } }), { - skip: isNode ? [ + skip: isWindows ? [ // pubsub.subscribe - isWindows ? { + { name: 'should send/receive 100 messages', reason: 'FIXME https://github.com/ipfs/interface-ipfs-core/pull/188#issuecomment-354673246 and https://github.com/ipfs/go-ipfs/issues/4778' - } : null, - isWindows ? { + }, + { name: 'should receive multiple messages', reason: 'FIXME https://github.com/ipfs/interface-ipfs-core/pull/188#issuecomment-354673246 and https://github.com/ipfs/go-ipfs/issues/4778' - } : null - ] : { - reason: 'FIXME pubsub is not supported in the browser https://github.com/ipfs/js-ipfs-http-client/issues/518' - } + } + ] : null }) tests.repo(defaultCommonFactory) From 724d79fa85ef717649d6b35479774af6587f0607 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Thu, 25 Jul 2019 13:59:57 +0100 Subject: [PATCH 02/20] fix: tests License: MIT Signed-off-by: Alan Shaw --- src/lib/configure.browser.js | 7 +- src/lib/configure.js | 7 +- test/interface.spec.js | 4 +- test/pubsub-in-browser.spec.js | 162 --------------------------------- 4 files changed, 8 insertions(+), 172 deletions(-) delete mode 100644 test/pubsub-in-browser.spec.js diff --git a/src/lib/configure.browser.js b/src/lib/configure.browser.js index 2943915cc..3597554ec 100644 --- a/src/lib/configure.browser.js +++ b/src/lib/configure.browser.js @@ -9,11 +9,10 @@ module.exports = create => config => { if (typeof config === 'string') { config = { apiAddr: config } - } - - // Multiaddr instance - if (config.constructor && config.constructor.isMultiaddr) { + } else if (config.constructor && config.constructor.isMultiaddr) { config = { apiAddr: config } + } else { + config = { ...config } } config.fetch = config.fetch || require('./fetch').fetch diff --git a/src/lib/configure.js b/src/lib/configure.js index 3557945ee..9dd48b32a 100644 --- a/src/lib/configure.js +++ b/src/lib/configure.js @@ -10,11 +10,10 @@ module.exports = create => config => { if (typeof config === 'string') { config = { apiAddr: config } - } - - // Multiaddr instance - if (config.constructor && config.constructor.isMultiaddr) { + } else if (config.constructor && config.constructor.isMultiaddr) { config = { apiAddr: config } + } else { + config = { ...config } } config.fetch = config.fetch || require('./fetch').fetch diff --git a/test/interface.spec.js b/test/interface.spec.js index 9305acc4c..86ffac21d 100644 --- a/test/interface.spec.js +++ b/test/interface.spec.js @@ -226,7 +226,7 @@ describe('interface-ipfs-core tests', () => { tests.namePubsub(CommonFactory.create({ spawnOptions: { args: ['--enable-namesys-pubsub'], - initOptions: { bits: 1024 } + initOptions: { bits: 1024, profile: 'test' } } }), { skip: [ @@ -267,7 +267,7 @@ describe('interface-ipfs-core tests', () => { tests.pubsub(CommonFactory.create({ spawnOptions: { args: ['--enable-pubsub-experiment'], - initOptions: { bits: 1024 } + initOptions: { bits: 1024, profile: 'test' } } }), { skip: isWindows ? [ diff --git a/test/pubsub-in-browser.spec.js b/test/pubsub-in-browser.spec.js deleted file mode 100644 index ff1a22347..000000000 --- a/test/pubsub-in-browser.spec.js +++ /dev/null @@ -1,162 +0,0 @@ -/* - We currently don't support pubsub when run in the browser, - and we test it with separate set of tests to make sure - if it's being used in the browser, pubsub errors. - - More info: https://github.com/ipfs/js-ipfs-http-client/issues/518 - - This means: - - You can use pubsub from js-ipfs-http-client in Node.js - - You can use pubsub from js-ipfs-http-client in Electron - (when js-ipfs-http-client is ran in the main process of Electron) - - - You can't use pubsub from js-ipfs-http-client in the browser - - You can't use pubsub from js-ipfs-http-client in Electron's - renderer process - - - You can use pubsub from js-ipfs in the browsers - - You can use pubsub from js-ipfs in Node.js - - You can use pubsub from js-ipfs in Electron - (in both the main process and the renderer process) - - See https://github.com/ipfs/js-ipfs for details on - pubsub in js-ipfs -*/ - -/* eslint-env mocha */ -/* eslint max-nested-callbacks: ['error', 8] */ -'use strict' - -const isNode = require('detect-node') -const chai = require('chai') -const dirtyChai = require('dirty-chai') -const expect = chai.expect -chai.use(dirtyChai) - -const ipfsClient = require('../src') -const f = require('./utils/factory') - -const expectedError = 'pubsub is currently not supported when run in the browser' - -describe('.pubsub is not supported in the browser, yet!', function () { - this.timeout(50 * 1000) - - if (isNode) { return } - - const topic = 'pubsub-tests' - let ipfs - let ipfsd - - before((done) => { - f.spawn({ initOptions: { bits: 1024, profile: 'test' } }, (err, _ipfsd) => { - expect(err).to.not.exist() - ipfsd = _ipfsd - ipfs = ipfsClient(_ipfsd.apiAddr) - done() - }) - }) - - after((done) => { - if (!ipfsd) return done() - ipfsd.stop(done) - }) - - describe('everything errors', () => { - describe('Callback API', () => { - describe('.publish', () => { - it('throws an error if called in the browser', (done) => { - ipfs.pubsub.publish(topic, 'hello friend', (err, topics) => { - expect(err).to.exist() - expect(err.message).to.equal(expectedError) - done() - }) - }) - }) - - describe('.subscribe', () => { - const handler = () => {} - it('throws an error if called in the browser', (done) => { - ipfs.pubsub.subscribe(topic, handler, {}, (err, topics) => { - expect(err).to.exist() - expect(err.message).to.equal(expectedError) - done() - }) - }) - }) - - describe('.peers', () => { - it('throws an error if called in the browser', (done) => { - ipfs.pubsub.peers(topic, (err, topics) => { - expect(err).to.exist() - expect(err.message).to.equal(expectedError) - done() - }) - }) - }) - - describe('.ls', () => { - it('throws an error if called in the browser', (done) => { - ipfs.pubsub.ls((err, topics) => { - expect(err).to.exist() - expect(err.message).to.equal(expectedError) - done() - }) - }) - }) - }) - - describe('Promise API', () => { - describe('.publish', () => { - it('throws an error if called in the browser', () => { - return ipfs.pubsub.publish(topic, 'hello friend') - .catch((err) => { - expect(err).to.exist() - expect(err.message).to.equal(expectedError) - }) - }) - }) - - describe('.subscribe', () => { - const handler = () => {} - it('throws an error if called in the browser', (done) => { - ipfs.pubsub.subscribe(topic, handler, {}) - .catch((err) => { - expect(err).to.exist() - expect(err.message).to.equal(expectedError) - done() - }) - }) - }) - - describe('.peers', () => { - it('throws an error if called in the browser', (done) => { - ipfs.pubsub.peers(topic) - .catch((err) => { - expect(err).to.exist() - expect(err.message).to.equal(expectedError) - done() - }) - }) - }) - - describe('.ls', () => { - it('throws an error if called in the browser', () => { - return ipfs.pubsub.ls() - .catch((err) => { - expect(err).to.exist() - expect(err.message).to.equal(expectedError) - }) - }) - }) - }) - - describe('.unsubscribe', () => { - it('throws an error if called in the browser', (done) => { - ipfs.pubsub.unsubscribe('test', () => {}, (err) => { - expect(err).to.exist() - expect(err.message).to.equal(expectedError) - done() - }) - }) - }) - }) -}) From e540e8f4b07529e8d85ae4f193569b012bfbcfc8 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Thu, 25 Jul 2019 14:16:29 +0100 Subject: [PATCH 03/20] fix: use included querystring module License: MIT Signed-off-by: Alan Shaw --- package.json | 1 - src/lib/querystring.js | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 5e28ecf81..75044cf00 100644 --- a/package.json +++ b/package.json @@ -79,7 +79,6 @@ "pull-to-stream": "~0.1.1", "pump": "^3.0.0", "qs": "^6.5.2", - "querystring": "^0.2.0", "readable-stream": "^3.1.1", "stream-to-pull-stream": "^1.7.2", "tar-stream": "^2.0.1", diff --git a/src/lib/querystring.js b/src/lib/querystring.js index df35b0a6e..07de0edda 100644 --- a/src/lib/querystring.js +++ b/src/lib/querystring.js @@ -1,6 +1,6 @@ 'use strict' -const QueryString = require('querystring') +const Qs = require('qs') // Convert an object to a query string INCLUDING leading ? // Excludes null/undefined values @@ -12,5 +12,5 @@ exports.objectToQuery = obj => { return obj }, {}) - return Object.keys(qs).length ? `?${QueryString.stringify(qs)}` : '' + return Object.keys(qs).length ? `?${Qs.stringify(qs, { arrayFormat: 'repeat' })}` : '' } From 75e1e6b03101175897f79a5dccca162327d0b9e6 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Thu, 25 Jul 2019 14:29:09 +0100 Subject: [PATCH 04/20] chore: appease linter License: MIT Signed-off-by: Alan Shaw --- src/lib/multiaddr.js | 2 ++ src/pubsub/index.js | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/lib/multiaddr.js b/src/lib/multiaddr.js index 1ccfe72c8..09462ab34 100644 --- a/src/lib/multiaddr.js +++ b/src/lib/multiaddr.js @@ -1,3 +1,5 @@ +'use strict' + // Convert a multiaddr to a URI // Assumes multiaddr is in a format that can be converted to a HTTP(s) URI exports.toUri = ma => { diff --git a/src/pubsub/index.js b/src/pubsub/index.js index 7f2897ea7..8562e9ea3 100644 --- a/src/pubsub/index.js +++ b/src/pubsub/index.js @@ -1,3 +1,5 @@ +'use strict' + const callbackify = require('../lib/callbackify') // This file is temporary and for compatibility with legacy usage From 408d1ad1e57f18a45f1b332b683f4067c7497f55 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Thu, 25 Jul 2019 14:54:39 +0100 Subject: [PATCH 05/20] chore: update interface-ipfs-core License: MIT Signed-off-by: Alan Shaw --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 75044cf00..08c385fa4 100644 --- a/package.json +++ b/package.json @@ -91,7 +91,7 @@ "cross-env": "^5.2.0", "dirty-chai": "^2.0.1", "go-ipfs-dep": "0.4.21", - "interface-ipfs-core": "^0.109.0", + "interface-ipfs-core": "^0.111.0", "ipfsd-ctl": "~0.43.0", "nock": "^10.0.2", "stream-equal": "^1.1.1" From fcf1efb73b0b60a6063c5b5cfd4d9e44ea2ff1c8 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Thu, 25 Jul 2019 15:21:56 +0100 Subject: [PATCH 06/20] chore: skip test License: MIT Signed-off-by: Alan Shaw --- test/interface.spec.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/interface.spec.js b/test/interface.spec.js index 86ffac21d..d6f55f6e8 100644 --- a/test/interface.spec.js +++ b/test/interface.spec.js @@ -169,6 +169,10 @@ describe('interface-ipfs-core tests', () => { name: 'should ls with a base58 encoded CID', reason: 'FIXME https://github.com/ipfs/js-ipfs-http-client/issues/339' }, + { + name: 'should ls directory with long option', + reason: 'TODO unskip when go-ipfs supports --long https://github.com/ipfs/go-ipfs/pull/6528' + }, // .lsPullStream isNode ? null : { name: 'should pull stream ls with a base58 encoded CID', From e9a1b2642a176cbeb2ef75cd660ba3593a97f00e Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Thu, 25 Jul 2019 15:37:57 +0100 Subject: [PATCH 07/20] fix: skip in the right place License: MIT Signed-off-by: Alan Shaw --- test/interface.spec.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/interface.spec.js b/test/interface.spec.js index d6f55f6e8..86ffac21d 100644 --- a/test/interface.spec.js +++ b/test/interface.spec.js @@ -169,10 +169,6 @@ describe('interface-ipfs-core tests', () => { name: 'should ls with a base58 encoded CID', reason: 'FIXME https://github.com/ipfs/js-ipfs-http-client/issues/339' }, - { - name: 'should ls directory with long option', - reason: 'TODO unskip when go-ipfs supports --long https://github.com/ipfs/go-ipfs/pull/6528' - }, // .lsPullStream isNode ? null : { name: 'should pull stream ls with a base58 encoded CID', From e39b90f8a66aa57f330e2d3ce22765eed2366782 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Thu, 25 Jul 2019 18:19:31 +0100 Subject: [PATCH 08/20] refactor: more readable code for consuming message stream License: MIT Signed-off-by: Alan Shaw --- src/pubsub/subscribe.js | 54 +++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/src/pubsub/subscribe.js b/src/pubsub/subscribe.js index f2a303937..7e431c87c 100644 --- a/src/pubsub/subscribe.js +++ b/src/pubsub/subscribe.js @@ -36,32 +36,38 @@ module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { throw err } - // eslint-disable-next-line no-console - const onError = options.onError || (err => console.error(err)) + readMessages(ndjson(toIterable(res.body)), { + onMessage: handler, + onEnd: () => subsTracker.unsubscribe(topic, handler), + onError: options.onError + }) + } +}) + +async function readMessages (msgStream, { onMessage, onEnd, onError }) { + // eslint-disable-next-line no-console + onError = onError || (err => console.error(err)) - ;(async () => { + try { + for await (const msg of msgStream) { try { - for await (const msg of ndjson(toIterable(res.body))) { - try { - handler({ - from: bs58.encode(Buffer.from(msg.from, 'base64')).toString(), - data: Buffer.from(msg.data, 'base64'), - seqno: Buffer.from(msg.seqno, 'base64'), - topicIDs: msg.topicIDs - }) - } catch (err) { - onError(explain(err, 'Failed to parse pubsub message'), false) // Not fatal - } - } + onMessage({ + from: bs58.encode(Buffer.from(msg.from, 'base64')).toString(), + data: Buffer.from(msg.data, 'base64'), + seqno: Buffer.from(msg.seqno, 'base64'), + topicIDs: msg.topicIDs + }) } catch (err) { - // FIXME: In testing with Chrome, err.type is undefined (should not be!) - // Temporarily use the name property instead. - if (err.type !== 'aborted' && err.name !== 'AbortError') { - onError(err, true) // Fatal - } - } finally { - subsTracker.unsubscribe(topic, handler) + onError(explain(err, 'Failed to parse pubsub message'), false) // Not fatal } - })() + } + } catch (err) { + // FIXME: In testing with Chrome, err.type is undefined (should not be!) + // Temporarily use the name property instead. + if (err.type !== 'aborted' && err.name !== 'AbortError') { + onError(err, true) // Fatal + } + } finally { + onEnd() } -}) +} From 5ccb0caced7d7f304fc564056934fb2e99b555ec Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Fri, 26 Jul 2019 10:05:33 +0100 Subject: [PATCH 09/20] fix: add workaround for subscribe in Firefox License: MIT Signed-off-by: Alan Shaw --- src/pubsub/subscribe.js | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/pubsub/subscribe.js b/src/pubsub/subscribe.js index 7e431c87c..90eeaed0f 100644 --- a/src/pubsub/subscribe.js +++ b/src/pubsub/subscribe.js @@ -4,6 +4,7 @@ const ndjson = require('iterable-ndjson') const explain = require('explain-error') const bs58 = require('bs58') const { Buffer } = require('buffer') +const log = require('debug')('ipfs-http-client:pubsub:subscribe') const { objectToQuery } = require('../lib/querystring') const configure = require('../lib/configure') const { ok, toIterable } = require('../lib/fetch') @@ -11,6 +12,7 @@ const SubscriptionTracker = require('./subscription-tracker') module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { const subsTracker = SubscriptionTracker.singleton() + const publish = require('./publish')({ fetch, apiAddr, apiPath, headers }) return async (topic, handler, options) => { options = options || {} @@ -25,6 +27,18 @@ module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { const url = `${apiAddr}${apiPath}/pubsub/sub${qs}` let res + // In Firefox, the initial call to fetch does not resolve until some data + // is received. If this doesn't happen within 1 second send an empty message + // to kickstart the process. + const ffWorkaround = setTimeout(async () => { + log(`Publishing empty message to "${topic}" to resolve subscription request`) + try { + await publish(topic, Buffer.alloc(0), options) + } catch (err) { + log('Failed to publish empty message', err) + } + }, 1000) + try { res = await ok(fetch(url, { method: 'POST', @@ -36,6 +50,8 @@ module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { throw err } + clearTimeout(ffWorkaround) + readMessages(ndjson(toIterable(res.body)), { onMessage: handler, onEnd: () => subsTracker.unsubscribe(topic, handler), @@ -45,8 +61,7 @@ module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { }) async function readMessages (msgStream, { onMessage, onEnd, onError }) { - // eslint-disable-next-line no-console - onError = onError || (err => console.error(err)) + onError = onError || log try { for await (const msg of msgStream) { @@ -58,7 +73,7 @@ async function readMessages (msgStream, { onMessage, onEnd, onError }) { topicIDs: msg.topicIDs }) } catch (err) { - onError(explain(err, 'Failed to parse pubsub message'), false) // Not fatal + onError(explain(err, 'Failed to parse pubsub message'), false, msg) // Not fatal } } } catch (err) { From b346479accb530bd14896b8c6e58056ab806ba06 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Fri, 26 Jul 2019 17:09:28 +0100 Subject: [PATCH 10/20] refactor: use promise-nodeify License: MIT Signed-off-by: Alan Shaw --- package.json | 1 + src/lib/callbackify.js | 17 ---------------- src/pubsub/index.js | 44 ++++++++++++++++++++++++++++++++++++------ 3 files changed, 39 insertions(+), 23 deletions(-) delete mode 100644 src/lib/callbackify.js diff --git a/package.json b/package.json index 08c385fa4..aaf41d084 100644 --- a/package.json +++ b/package.json @@ -73,6 +73,7 @@ "once": "^1.4.0", "peer-id": "~0.12.3", "peer-info": "~0.15.1", + "promise-nodeify": "^3.0.1", "promisify-es6": "^1.0.3", "pull-defer": "~0.2.3", "pull-stream": "^3.6.9", diff --git a/src/lib/callbackify.js b/src/lib/callbackify.js deleted file mode 100644 index 3a041612d..000000000 --- a/src/lib/callbackify.js +++ /dev/null @@ -1,17 +0,0 @@ -'use strict' - -module.exports = (fn, opts) => { - opts = opts || {} - // Min number of non-callback args - opts.minArgs = opts.minArgs == null ? 0 : opts.minArgs - - return (...args) => { - const cb = args[args.length - 1] - - if (typeof cb !== 'function' || args.length === opts.minArgs) { - return fn(...args) - } - - fn(...args.slice(0, -1)).then(res => cb(null, res), cb) - } -} diff --git a/src/pubsub/index.js b/src/pubsub/index.js index 8562e9ea3..2738bd5ac 100644 --- a/src/pubsub/index.js +++ b/src/pubsub/index.js @@ -1,6 +1,6 @@ 'use strict' -const callbackify = require('../lib/callbackify') +const nodeify = require('promise-nodeify') // This file is temporary and for compatibility with legacy usage module.exports = (send, options) => { @@ -8,11 +8,43 @@ module.exports = (send, options) => { options = send } + const ls = require('./ls')(options) + const peers = require('./peers')(options) + const publish = require('./publish')(options) + const subscribe = require('./subscribe')(options) + const unsubscribe = require('./unsubscribe')(options) + return { - ls: callbackify(require('./ls')(options)), - peers: callbackify(require('./peers')(options)), - publish: callbackify(require('./publish')(options)), - subscribe: callbackify(require('./subscribe')(options), { minArgs: 2 }), - unsubscribe: callbackify(require('./unsubscribe')(options), { minArgs: 2 }) + ls: (options, callback) => { + if (typeof options === 'function') { + callback = options + options = {} + } + return nodeify(ls(options), callback) + }, + peers: (topic, options, callback) => { + if (typeof options === 'function') { + callback = options + options = {} + } + return nodeify(peers(topic, options), callback) + }, + publish: (topic, data, options, callback) => { + if (typeof options === 'function') { + callback = options + options = {} + } + return nodeify(publish(topic, data, options), callback) + }, + subscribe: (topic, handler, options, callback) => { + if (typeof options === 'function') { + callback = options + options = {} + } + return nodeify(subscribe(topic, handler, options), callback) + }, + unsubscribe: (topic, handler, callback) => { + return nodeify(unsubscribe(topic, handler), callback) + } } } From cb9238c3b8538ef664a86b8d5542f21fb9805477 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Fri, 26 Jul 2019 22:06:16 +0100 Subject: [PATCH 11/20] test: add tests for lib fns License: MIT Signed-off-by: Alan Shaw --- test/lib.configure.spec.js | 60 +++++++++++++++++++++++++ test/lib.fetch.spec.js | 92 ++++++++++++++++++++++++++++++++++++++ test/utils/throws-async.js | 8 ++++ 3 files changed, 160 insertions(+) create mode 100644 test/lib.configure.spec.js create mode 100644 test/lib.fetch.spec.js create mode 100644 test/utils/throws-async.js diff --git a/test/lib.configure.spec.js b/test/lib.configure.spec.js new file mode 100644 index 000000000..81a2dbee9 --- /dev/null +++ b/test/lib.configure.spec.js @@ -0,0 +1,60 @@ +/* eslint-env mocha */ +'use strict' + +const chai = require('chai') +const dirtyChai = require('dirty-chai') +const expect = chai.expect +chai.use(dirtyChai) +const Multiaddr = require('multiaddr') + +const configure = require('../src/lib/configure') + +describe('lib/configure', () => { + it('should accept no config', () => { + configure(config => { + expect(config.apiAddr).to.eql('http://localhost:5001') + })() + }) + + it('should accept string multiaddr', () => { + const input = '/ip4/127.0.0.1/tcp/5001' + configure(config => { + expect(config.apiAddr).to.eql('http://127.0.0.1:5001') + })(input) + }) + + it('should accept multiaddr instance', () => { + const input = Multiaddr('/ip4/127.0.0.1') + configure(config => { + expect(config.apiAddr).to.eql('http://127.0.0.1') + })(input) + }) + + it('should accept object with protocol, host and port', () => { + const input = { protocol: 'https', host: 'ipfs.io', port: 138 } + configure(config => { + expect(config.apiAddr).to.eql('https://ipfs.io:138') + })(input) + }) + + it('should accept object with protocol only', () => { + const input = { protocol: 'https' } + configure(config => { + expect(config.apiAddr).to.eql('https://localhost') + })(input) + }) + + it('should accept object with host only', () => { + const input = { host: 'ipfs.io' } + configure(config => { + expect(config.apiAddr).to.eql('http://ipfs.io') + })(input) + }) + + it('should accept object with port only', () => { + const input = { port: 138 } + configure(config => { + expect(config.apiAddr).to.eql('http://localhost:138') + })(input) + }) +}) diff --git a/test/lib.fetch.spec.js b/test/lib.fetch.spec.js new file mode 100644 index 000000000..1db560113 --- /dev/null +++ b/test/lib.fetch.spec.js @@ -0,0 +1,92 @@ +/* eslint-env mocha */ +'use strict' + +const chai = require('chai') +const dirtyChai = require('dirty-chai') +const expect = chai.expect +chai.use(dirtyChai) +const throwsAsync = require('./utils/throws-async') + +const { ok, toIterable } = require('../src/lib/fetch') + +describe('lib/fetch', () => { + describe('ok', () => { + it('should parse json error response', async () => { + const res = { + ok: false, + text: () => Promise.resolve(JSON.stringify({ + Message: 'boom', + Code: 0, + Type: 'error' + })), + status: 500 + } + + const err = await throwsAsync(ok(res)) + + expect(err.message).to.eql('boom') + expect(err.status).to.eql(500) + }) + + it('should gracefully fail on parse json', async () => { + const res = { + ok: false, + text: () => 'boom', // not valid json! + status: 500 + } + + const err = await throwsAsync(ok(res)) + + expect(err.message).to.eql('boom') + expect(err.status).to.eql(500) + }) + + it('should gracefully fail on read text', async () => { + const res = { + ok: false, + text: () => Promise.reject(new Error('boom')), + status: 500 + } + + const err = await throwsAsync(ok(res)) + + expect(err.message).to.eql('unexpected status 500') + expect(err.status).to.eql(500) + }) + }) + + describe('toIterable', () => { + it('should return input if already async iterable', () => { + const input = { [Symbol.asyncIterator] () { return this } } + expect(toIterable(input)).to.equal(input) + }) + + it('should convert reader to async iterable', async () => { + const inputData = [2, 31, 3, 4] + const input = { + getReader () { + let i = 0 + return { + read: async () => { + return i === inputData.length + ? { done: true } + : { value: inputData[i++] } + }, + releaseLock: () => {} + } + } + } + + const chunks = [] + for await (const chunk of toIterable(input)) { + chunks.push(chunk) + } + + expect(chunks).to.eql(inputData) + }) + + it('should throw on unknown stream', () => { + expect(() => toIterable({})).to.throw('unknown stream') + }) + }) +}) diff --git a/test/utils/throws-async.js b/test/utils/throws-async.js new file mode 100644 index 000000000..6d44ae972 --- /dev/null +++ b/test/utils/throws-async.js @@ -0,0 +1,8 @@ +module.exports = async fnOrPromise => { + try { + await (fnOrPromise.then ? fnOrPromise : fnOrPromise()) + } catch (err) { + return err + } + throw new Error('did not throw') +} From ce2f0c4632b28b1fb1d0d26002fb247bd4079cd5 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Fri, 26 Jul 2019 22:10:55 +0100 Subject: [PATCH 12/20] chore: appease linter License: MIT Signed-off-by: Alan Shaw --- test/lib.fetch.spec.js | 4 ++-- test/utils/throws-async.js | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/test/lib.fetch.spec.js b/test/lib.fetch.spec.js index 1db560113..3b260a022 100644 --- a/test/lib.fetch.spec.js +++ b/test/lib.fetch.spec.js @@ -67,12 +67,12 @@ describe('lib/fetch', () => { getReader () { let i = 0 return { - read: async () => { + read () { return i === inputData.length ? { done: true } : { value: inputData[i++] } }, - releaseLock: () => {} + releaseLock () {} } } } diff --git a/test/utils/throws-async.js b/test/utils/throws-async.js index 6d44ae972..0d4e677fd 100644 --- a/test/utils/throws-async.js +++ b/test/utils/throws-async.js @@ -1,3 +1,5 @@ +'use strict' + module.exports = async fnOrPromise => { try { await (fnOrPromise.then ? fnOrPromise : fnOrPromise()) From cc60c0b7111f648ad1d5f68bd60b395cc9ba5428 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Fri, 26 Jul 2019 22:40:52 +0100 Subject: [PATCH 13/20] fix: tests in browser License: MIT Signed-off-by: Alan Shaw --- test/lib.configure.spec.js | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/test/lib.configure.spec.js b/test/lib.configure.spec.js index 81a2dbee9..e28abd6c6 100644 --- a/test/lib.configure.spec.js +++ b/test/lib.configure.spec.js @@ -1,4 +1,4 @@ -/* eslint-env mocha */ +/* eslint-env mocha, browser */ 'use strict' const chai = require('chai') @@ -6,13 +6,18 @@ const dirtyChai = require('dirty-chai') const expect = chai.expect chai.use(dirtyChai) const Multiaddr = require('multiaddr') +const { isBrowser, isWebWorker } = require('ipfs-utils/src/env') const configure = require('../src/lib/configure') describe('lib/configure', () => { it('should accept no config', () => { configure(config => { - expect(config.apiAddr).to.eql('http://localhost:5001') + if (isBrowser || isWebWorker) { + expect(config.apiAddr).to.eql(location.origin) + } else { + expect(config.apiAddr).to.eql('http://localhost:5001') + } })() }) @@ -40,21 +45,33 @@ describe('lib/configure', () => { it('should accept object with protocol only', () => { const input = { protocol: 'https' } configure(config => { - expect(config.apiAddr).to.eql('https://localhost') + if (isBrowser || isWebWorker) { + expect(config.apiAddr).to.eql(`https://${location.host}`) + } else { + expect(config.apiAddr).to.eql('https://localhost') + } })(input) }) it('should accept object with host only', () => { const input = { host: 'ipfs.io' } configure(config => { - expect(config.apiAddr).to.eql('http://ipfs.io') + if (isBrowser || isWebWorker) { + expect(config.apiAddr).to.eql(`http://ipfs.io:${location.port}`) + } else { + expect(config.apiAddr).to.eql('http://ipfs.io') + } })(input) }) it('should accept object with port only', () => { const input = { port: 138 } configure(config => { - expect(config.apiAddr).to.eql('http://localhost:138') + if (isBrowser || isWebWorker) { + expect(config.apiAddr).to.eql(`http://${location.hostname}:138`) + } else { + expect(config.apiAddr).to.eql('http://localhost:138') + } })(input) }) }) From 89db565ac298eb93318bead6f33c9a051181120f Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Mon, 29 Jul 2019 09:42:32 +0100 Subject: [PATCH 14/20] perf: use URLSearchParams in the browser License: MIT Signed-off-by: Alan Shaw --- package.json | 3 ++- src/lib/querystring.browser.js | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 src/lib/querystring.browser.js diff --git a/package.json b/package.json index aaf41d084..072558b40 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,8 @@ "glob": false, "fs": false, "stream": "readable-stream", - "./src/lib/configure.js": "./src/lib/configure.browser.js" + "./src/lib/configure.js": "./src/lib/configure.browser.js", + "./src/lib/querystring.js": "./src/lib/querystring.browser.js" }, "repository": "github:ipfs/js-ipfs-http-client", "scripts": { diff --git a/src/lib/querystring.browser.js b/src/lib/querystring.browser.js new file mode 100644 index 000000000..d71843dbe --- /dev/null +++ b/src/lib/querystring.browser.js @@ -0,0 +1,23 @@ +'use strict' + +// Convert an object to a query string INCLUDING leading ? +// Excludes null/undefined values +exports.objectToQuery = obj => { + if (!obj) return '' + + let qs = new URLSearchParams() + + for (const [key, value] of Object.entries(obj)) { + if (value != null) { + if (Array.isArray(value)) { + value.forEach(v => qs.append(key, v)) + } else { + qs.append(key, value) + } + } + } + + qs = qs.toString() + + return qs ? `?${qs}` : qs +} From 6a7f92d12a8d557a5476f0662ecd7f275ab8a052 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Mon, 29 Jul 2019 09:55:55 +0100 Subject: [PATCH 15/20] chore: appease linter License: MIT Signed-off-by: Alan Shaw --- src/lib/querystring.browser.js | 6 ++---- src/lib/querystring.js | 6 ++++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib/querystring.browser.js b/src/lib/querystring.browser.js index d71843dbe..4c4c5d2a8 100644 --- a/src/lib/querystring.browser.js +++ b/src/lib/querystring.browser.js @@ -5,9 +5,7 @@ exports.objectToQuery = obj => { if (!obj) return '' - let qs = new URLSearchParams() - - for (const [key, value] of Object.entries(obj)) { + let qs = Object.entries(obj).forEach(([key, value]) => { if (value != null) { if (Array.isArray(value)) { value.forEach(v => qs.append(key, v)) @@ -15,7 +13,7 @@ exports.objectToQuery = obj => { qs.append(key, value) } } - } + }) qs = qs.toString() diff --git a/src/lib/querystring.js b/src/lib/querystring.js index 07de0edda..e7d64e152 100644 --- a/src/lib/querystring.js +++ b/src/lib/querystring.js @@ -7,10 +7,12 @@ const Qs = require('qs') exports.objectToQuery = obj => { if (!obj) return '' - const qs = Object.entries(obj).reduce((obj, [key, value]) => { + let qs = Object.entries(obj).reduce((obj, [key, value]) => { if (value != null) obj[key] = value return obj }, {}) - return Object.keys(qs).length ? `?${Qs.stringify(qs, { arrayFormat: 'repeat' })}` : '' + qs = Qs.stringify(qs, { arrayFormat: 'repeat' }) + + return qs ? `?${qs}` : qs } From 5d134d533cab558a015d18636c16c6ed0143ff1a Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Mon, 29 Jul 2019 10:30:32 +0100 Subject: [PATCH 16/20] fix: oops, removed more code than I should have License: MIT Signed-off-by: Alan Shaw --- src/lib/querystring.browser.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lib/querystring.browser.js b/src/lib/querystring.browser.js index 4c4c5d2a8..c9e737818 100644 --- a/src/lib/querystring.browser.js +++ b/src/lib/querystring.browser.js @@ -5,7 +5,9 @@ exports.objectToQuery = obj => { if (!obj) return '' - let qs = Object.entries(obj).forEach(([key, value]) => { + let qs = new URLSearchParams() + + Object.entries(obj).forEach(([key, value]) => { if (value != null) { if (Array.isArray(value)) { value.forEach(v => qs.append(key, v)) From 5dbeb30b8c2a1327737c0f52ea8394f8d6fe40d7 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Tue, 30 Jul 2019 11:53:22 +0100 Subject: [PATCH 17/20] refactor: use ky License: MIT Signed-off-by: Alan Shaw --- package.json | 7 +-- src/lib/configure.browser.js | 45 -------------- src/lib/configure.js | 50 +++++++++++----- src/lib/error-handler.js | 31 ++++++++++ src/lib/fetch.js | 53 ----------------- src/lib/querystring.browser.js | 23 -------- src/lib/querystring.js | 18 ------ src/lib/stream-to-iterable.js | 25 ++++++++ src/pubsub/ls.js | 18 +++--- src/pubsub/peers.js | 23 ++++---- src/pubsub/publish.js | 19 +++--- src/pubsub/subscribe.js | 25 ++++---- src/pubsub/unsubscribe.js | 2 +- test/lib.error-handler.spec.js | 54 +++++++++++++++++ test/lib.fetch.spec.js | 92 ----------------------------- test/lib.stream-to-iterable.spec.js | 43 ++++++++++++++ 16 files changed, 228 insertions(+), 300 deletions(-) delete mode 100644 src/lib/configure.browser.js create mode 100644 src/lib/error-handler.js delete mode 100644 src/lib/fetch.js delete mode 100644 src/lib/querystring.browser.js delete mode 100644 src/lib/querystring.js create mode 100644 src/lib/stream-to-iterable.js create mode 100644 test/lib.error-handler.spec.js delete mode 100644 test/lib.fetch.spec.js create mode 100644 test/lib.stream-to-iterable.spec.js diff --git a/package.json b/package.json index 072558b40..16dc4850b 100644 --- a/package.json +++ b/package.json @@ -17,9 +17,7 @@ "browser": { "glob": false, "fs": false, - "stream": "readable-stream", - "./src/lib/configure.js": "./src/lib/configure.browser.js", - "./src/lib/querystring.js": "./src/lib/querystring.browser.js" + "stream": "readable-stream" }, "repository": "github:ipfs/js-ipfs-http-client", "scripts": { @@ -64,13 +62,14 @@ "just-kebab-case": "^1.1.0", "just-map-keys": "^1.1.0", "kind-of": "^6.0.2", + "ky": "^0.11.2", + "ky-universal": "^0.2.2", "lru-cache": "^5.1.1", "multiaddr": "^6.0.6", "multibase": "~0.6.0", "multicodec": "~0.5.1", "multihashes": "~0.4.14", "ndjson": "github:hugomrdias/ndjson#feat/readable-stream3", - "node-fetch": "^2.6.0", "once": "^1.4.0", "peer-id": "~0.12.3", "peer-info": "~0.15.1", diff --git a/src/lib/configure.browser.js b/src/lib/configure.browser.js deleted file mode 100644 index 3597554ec..000000000 --- a/src/lib/configure.browser.js +++ /dev/null @@ -1,45 +0,0 @@ -'use strict' -/* eslint-env browser */ - -const { toUri } = require('./multiaddr') - -// Set default configuration and call create function with them -module.exports = create => config => { - config = config || {} - - if (typeof config === 'string') { - config = { apiAddr: config } - } else if (config.constructor && config.constructor.isMultiaddr) { - config = { apiAddr: config } - } else { - config = { ...config } - } - - config.fetch = config.fetch || require('./fetch').fetch - config.apiAddr = (config.apiAddr || getDefaultApiAddr(config)).toString() - config.apiAddr = config.apiAddr.startsWith('/') - ? toUri(config.apiAddr) - : config.apiAddr - config.apiPath = config.apiPath || config['api-path'] || '/api/v0' - - if (config.apiPath.endsWith('/')) { - config.apiPath = config.apiPath.slice(0, -1) - } - - config.headers = new Headers(config.headers) - - return create(config) -} - -function getDefaultApiAddr ({ protocol, host, port }) { - if (!protocol) { - protocol = location.protocol.startsWith('http') - ? location.protocol.split(':')[0] - : 'http' - } - - host = host || location.hostname - port = port || location.port - - return `${protocol}://${host}${port ? ':' + port : ''}` -} diff --git a/src/lib/configure.js b/src/lib/configure.js index 9dd48b32a..3acb10db6 100644 --- a/src/lib/configure.js +++ b/src/lib/configure.js @@ -1,8 +1,10 @@ 'use strict' +/* eslint-env browser */ -const { Headers } = require('node-fetch') +const ky = require('ky-universal') +const { isBrowser, isWebWorker } = require('ipfs-utils/src/env') const { toUri } = require('./multiaddr') -const pkg = require('../../package.json') +const errorHandler = require('./error-handler') // Set default configuration and call create function with them module.exports = create => config => { @@ -16,28 +18,46 @@ module.exports = create => config => { config = { ...config } } - config.fetch = config.fetch || require('./fetch').fetch - if (config.protocol || config.host || config.port) { const port = config.port ? `:${config.port}` : '' config.apiAddr = `${config.protocol || 'http'}://${config.host || 'localhost'}${port}` } - config.apiAddr = (config.apiAddr || 'http://localhost:5001').toString() - config.apiAddr = config.apiAddr.startsWith('/') - ? toUri(config.apiAddr) - : config.apiAddr + config.apiAddr = (config.apiAddr || getDefaultApiAddr(config)).toString() + config.apiAddr = config.apiAddr.startsWith('/') ? toUri(config.apiAddr) : config.apiAddr config.apiPath = config.apiPath || config['api-path'] || '/api/v0' - if (config.apiPath.endsWith('/')) { - config.apiPath = config.apiPath.slice(0, -1) - } + return create({ + // TODO configure ky to use config.fetch when this is released: + // https://github.com/sindresorhus/ky/pull/153 + ky: ky.extend({ + prefixUrl: config.apiAddr + config.apiPath, + timeout: config.timeout || 60 * 1000, + headers: config.headers, + hooks: { + afterResponse: [errorHandler] + } + }) + }) +} + +function getDefaultApiAddr ({ protocol, host, port }) { + if (isBrowser || isWebWorker) { + if (!protocol && !host && !port) { // Use current origin + return '' + } + + if (!protocol) { + protocol = location.protocol.startsWith('http') + ? location.protocol.split(':')[0] + : 'http' + } - config.headers = new Headers(config.headers) + host = host || location.hostname + port = port || location.port - if (!config.headers.has('User-Agent')) { - config.headers.append('User-Agent', `${pkg.name}/${pkg.version}`) + return `${protocol}://${host}${port ? ':' + port : ''}` } - return create(config) + return `${protocol || 'http'}://${host || 'localhost'}:${port || 5001}` } diff --git a/src/lib/error-handler.js b/src/lib/error-handler.js new file mode 100644 index 000000000..1e788227c --- /dev/null +++ b/src/lib/error-handler.js @@ -0,0 +1,31 @@ +'use strict' + +const { HTTPError } = require('ky-universal') +const log = require('debug')('ipfs-http-client:lib:error-handler') + +function isJsonResponse (res) { + return (res.headers.get('Content-Type') || '').startsWith('application/json') +} + +module.exports = async function errorHandler (response) { + if (response.ok) return + + let msg + + try { + if (isJsonResponse(response)) { + const data = await response.json() + log(data) + msg = data.Message || data.message + } else { + msg = await response.text() + } + } catch (err) { + log('Failed to parse error response', err) + // Failed to extract/parse error message from response + throw new HTTPError(response) + } + + if (!msg) throw new HTTPError(response) + throw Object.assign(new Error(msg), { status: response.status }) +} diff --git a/src/lib/fetch.js b/src/lib/fetch.js deleted file mode 100644 index b3cf03cbb..000000000 --- a/src/lib/fetch.js +++ /dev/null @@ -1,53 +0,0 @@ -'use strict' - -const explain = require('explain-error') - -exports.fetch = require('node-fetch') - -// Ensure fetch response is ok (200) -// and if not, attempt to JSON parse body, extract error message and throw -exports.ok = async res => { - res = await res - - if (!res.ok) { - const { status } = res - const defaultMsg = `unexpected status ${status}` - let msg - try { - let data = await res.text() - try { - data = JSON.parse(data) - msg = data.message || data.Message - } catch (err) { - msg = data - } - } catch (err) { - throw Object.assign(explain(err, defaultMsg), { status }) - } - throw Object.assign(new Error(msg || defaultMsg), { status }) - } - - return res -} - -exports.toIterable = body => { - if (body[Symbol.asyncIterator]) return body - - if (body.getReader) { - return (async function * () { - const reader = body.getReader() - - try { - while (true) { - const { done, value } = await reader.read() - if (done) return - yield value - } - } finally { - reader.releaseLock() - } - })() - } - - throw new Error('unknown stream') -} diff --git a/src/lib/querystring.browser.js b/src/lib/querystring.browser.js deleted file mode 100644 index c9e737818..000000000 --- a/src/lib/querystring.browser.js +++ /dev/null @@ -1,23 +0,0 @@ -'use strict' - -// Convert an object to a query string INCLUDING leading ? -// Excludes null/undefined values -exports.objectToQuery = obj => { - if (!obj) return '' - - let qs = new URLSearchParams() - - Object.entries(obj).forEach(([key, value]) => { - if (value != null) { - if (Array.isArray(value)) { - value.forEach(v => qs.append(key, v)) - } else { - qs.append(key, value) - } - } - }) - - qs = qs.toString() - - return qs ? `?${qs}` : qs -} diff --git a/src/lib/querystring.js b/src/lib/querystring.js deleted file mode 100644 index e7d64e152..000000000 --- a/src/lib/querystring.js +++ /dev/null @@ -1,18 +0,0 @@ -'use strict' - -const Qs = require('qs') - -// Convert an object to a query string INCLUDING leading ? -// Excludes null/undefined values -exports.objectToQuery = obj => { - if (!obj) return '' - - let qs = Object.entries(obj).reduce((obj, [key, value]) => { - if (value != null) obj[key] = value - return obj - }, {}) - - qs = Qs.stringify(qs, { arrayFormat: 'repeat' }) - - return qs ? `?${qs}` : qs -} diff --git a/src/lib/stream-to-iterable.js b/src/lib/stream-to-iterable.js new file mode 100644 index 000000000..5e06a99c6 --- /dev/null +++ b/src/lib/stream-to-iterable.js @@ -0,0 +1,25 @@ +'use strict' + +module.exports = function toIterable (body) { + // Node.js stream + if (body[Symbol.asyncIterator]) return body + + // Browser ReadableStream + if (body.getReader) { + return (async function * () { + const reader = body.getReader() + + try { + while (true) { + const { done, value } = await reader.read() + if (done) return + yield value + } + } finally { + reader.releaseLock() + } + })() + } + + throw new Error('unknown stream') +} diff --git a/src/pubsub/ls.js b/src/pubsub/ls.js index bfbf8239e..177dcd491 100644 --- a/src/pubsub/ls.js +++ b/src/pubsub/ls.js @@ -1,20 +1,18 @@ 'use strict' const configure = require('../lib/configure') -const { ok } = require('../lib/fetch') -const { objectToQuery } = require('../lib/querystring') -module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { +module.exports = configure(({ ky }) => { return async (options) => { options = options || {} - const qs = objectToQuery(options.qs) - const url = `${apiAddr}${apiPath}/pubsub/ls${qs}` - const res = await ok(fetch(url, { + const { Strings } = await ky.get('pubsub/ls', { + timeout: options.timeout, signal: options.signal, - headers: options.headers || headers - })) - const data = await res.json() - return data.Strings || [] + headers: options.headers, + searchParams: options.searchParams + }).json() + + return Strings || [] } }) diff --git a/src/pubsub/peers.js b/src/pubsub/peers.js index 7ee860e33..bdeca60e4 100644 --- a/src/pubsub/peers.js +++ b/src/pubsub/peers.js @@ -1,10 +1,8 @@ 'use strict' -const { objectToQuery } = require('../lib/querystring') const configure = require('../lib/configure') -const { ok } = require('../lib/fetch') -module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { +module.exports = configure(({ ky }) => { return async (topic, options) => { if (!options && typeof topic === 'object') { options = topic @@ -13,17 +11,16 @@ module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { options = options || {} - const qs = objectToQuery({ - arg: topic, - ...(options.qs || {}) - }) + const searchParams = new URLSearchParams(options.searchParams) + searchParams.set('arg', topic) - const url = `${apiAddr}${apiPath}/pubsub/peers${qs}` - const res = await ok(fetch(url, { + const { Strings } = await ky.get('pubsub/peers', { + timeout: options.timeout, signal: options.signal, - headers: options.headers || headers - })) - const data = await res.json() - return data.Strings || [] + headers: options.headers, + searchParams + }).json() + + return Strings || [] } }) diff --git a/src/pubsub/publish.js b/src/pubsub/publish.js index d214ddbc8..3e63ee6f4 100644 --- a/src/pubsub/publish.js +++ b/src/pubsub/publish.js @@ -2,10 +2,8 @@ const { Buffer } = require('buffer') const configure = require('../lib/configure') -const { objectToQuery } = require('../lib/querystring') -const { ok } = require('../lib/fetch') -module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { +module.exports = configure(({ ky }) => { return async (topic, data, options) => { options = options || {} @@ -13,17 +11,14 @@ module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { throw new Error('data must be a Buffer') } - let qs = objectToQuery(options.qs) - qs = qs ? `&${qs.slice(1)}` : qs + const searchParams = new URLSearchParams(options.searchParams) + searchParams.set('arg', topic) - const url = `${apiAddr}${apiPath}/pubsub/pub?arg=${encodeURIComponent(topic)}&arg=${encodeBuffer(data)}${qs}` - const res = await ok(fetch(url, { - method: 'POST', + return ky.post(`pubsub/pub?${searchParams}&arg=${encodeBuffer(data)}`, { + timeout: options.timeout, signal: options.signal, - headers: options.headers || headers - })) - - return res.text() + headers: options.headers + }).text() } }) diff --git a/src/pubsub/subscribe.js b/src/pubsub/subscribe.js index 90eeaed0f..ae95ec5c8 100644 --- a/src/pubsub/subscribe.js +++ b/src/pubsub/subscribe.js @@ -5,26 +5,22 @@ const explain = require('explain-error') const bs58 = require('bs58') const { Buffer } = require('buffer') const log = require('debug')('ipfs-http-client:pubsub:subscribe') -const { objectToQuery } = require('../lib/querystring') const configure = require('../lib/configure') -const { ok, toIterable } = require('../lib/fetch') +const toIterable = require('../lib/stream-to-iterable') const SubscriptionTracker = require('./subscription-tracker') -module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { +module.exports = configure(({ ky }) => { const subsTracker = SubscriptionTracker.singleton() - const publish = require('./publish')({ fetch, apiAddr, apiPath, headers }) + const publish = require('./publish')({ ky }) return async (topic, handler, options) => { options = options || {} options.signal = subsTracker.subscribe(topic, handler, options.signal) - const qs = objectToQuery({ - arg: topic, - discover: options.discover, - ...(options.qs || {}) - }) + const searchParams = new URLSearchParams(options.searchParams) + searchParams.set('arg', topic) + if (options.discover != null) searchParams.set('discover', options.discover) - const url = `${apiAddr}${apiPath}/pubsub/sub${qs}` let res // In Firefox, the initial call to fetch does not resolve until some data @@ -40,11 +36,12 @@ module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { }, 1000) try { - res = await ok(fetch(url, { - method: 'POST', + res = await ky.post('pubsub/sub', { + timeout: options.timeout, signal: options.signal, - headers: options.headers || headers - })) + headers: options.headers, + searchParams + }) } catch (err) { // Initial subscribe fail, ensure we clean up subsTracker.unsubscribe(topic, handler) throw err diff --git a/src/pubsub/unsubscribe.js b/src/pubsub/unsubscribe.js index a8bd14944..6e7c727f4 100644 --- a/src/pubsub/unsubscribe.js +++ b/src/pubsub/unsubscribe.js @@ -3,7 +3,7 @@ const configure = require('../lib/configure') const SubscriptionTracker = require('./subscription-tracker') -module.exports = configure(({ fetch, apiAddr, apiPath, headers }) => { +module.exports = configure(({ ky }) => { const subsTracker = SubscriptionTracker.singleton() // eslint-disable-next-line require-await return async (topic, handler) => subsTracker.unsubscribe(topic, handler) diff --git a/test/lib.error-handler.spec.js b/test/lib.error-handler.spec.js new file mode 100644 index 000000000..4e97260ec --- /dev/null +++ b/test/lib.error-handler.spec.js @@ -0,0 +1,54 @@ +/* eslint-env mocha */ +'use strict' + +const chai = require('chai') +const dirtyChai = require('dirty-chai') +const { HTTPError } = require('ky-universal') +const expect = chai.expect +chai.use(dirtyChai) +const throwsAsync = require('./utils/throws-async') +const errorHandler = require('../src/lib/error-handler') + +describe('lib/error-handler', () => { + it('should parse json error response', async () => { + const res = { + ok: false, + headers: { get: () => 'application/json' }, + json: () => Promise.resolve({ + Message: 'boom', + Code: 0, + Type: 'error' + }), + status: 500 + } + + const err = await throwsAsync(errorHandler(res)) + + expect(err.message).to.eql('boom') + expect(err.status).to.eql(500) + }) + + it('should gracefully fail on parse json', async () => { + const res = { + ok: false, + headers: { get: () => 'application/json' }, + json: () => 'boom', // not valid json! + status: 500 + } + + const err = await throwsAsync(errorHandler(res)) + expect(err instanceof HTTPError).to.be.true() + }) + + it('should gracefully fail on read text', async () => { + const res = { + ok: false, + headers: { get: () => 'text/plain' }, + text: () => Promise.reject(new Error('boom')), + status: 500 + } + + const err = await throwsAsync(errorHandler(res)) + expect(err instanceof HTTPError).to.be.true() + }) +}) diff --git a/test/lib.fetch.spec.js b/test/lib.fetch.spec.js deleted file mode 100644 index 3b260a022..000000000 --- a/test/lib.fetch.spec.js +++ /dev/null @@ -1,92 +0,0 @@ -/* eslint-env mocha */ -'use strict' - -const chai = require('chai') -const dirtyChai = require('dirty-chai') -const expect = chai.expect -chai.use(dirtyChai) -const throwsAsync = require('./utils/throws-async') - -const { ok, toIterable } = require('../src/lib/fetch') - -describe('lib/fetch', () => { - describe('ok', () => { - it('should parse json error response', async () => { - const res = { - ok: false, - text: () => Promise.resolve(JSON.stringify({ - Message: 'boom', - Code: 0, - Type: 'error' - })), - status: 500 - } - - const err = await throwsAsync(ok(res)) - - expect(err.message).to.eql('boom') - expect(err.status).to.eql(500) - }) - - it('should gracefully fail on parse json', async () => { - const res = { - ok: false, - text: () => 'boom', // not valid json! - status: 500 - } - - const err = await throwsAsync(ok(res)) - - expect(err.message).to.eql('boom') - expect(err.status).to.eql(500) - }) - - it('should gracefully fail on read text', async () => { - const res = { - ok: false, - text: () => Promise.reject(new Error('boom')), - status: 500 - } - - const err = await throwsAsync(ok(res)) - - expect(err.message).to.eql('unexpected status 500') - expect(err.status).to.eql(500) - }) - }) - - describe('toIterable', () => { - it('should return input if already async iterable', () => { - const input = { [Symbol.asyncIterator] () { return this } } - expect(toIterable(input)).to.equal(input) - }) - - it('should convert reader to async iterable', async () => { - const inputData = [2, 31, 3, 4] - const input = { - getReader () { - let i = 0 - return { - read () { - return i === inputData.length - ? { done: true } - : { value: inputData[i++] } - }, - releaseLock () {} - } - } - } - - const chunks = [] - for await (const chunk of toIterable(input)) { - chunks.push(chunk) - } - - expect(chunks).to.eql(inputData) - }) - - it('should throw on unknown stream', () => { - expect(() => toIterable({})).to.throw('unknown stream') - }) - }) -}) diff --git a/test/lib.stream-to-iterable.spec.js b/test/lib.stream-to-iterable.spec.js new file mode 100644 index 000000000..6c14cac94 --- /dev/null +++ b/test/lib.stream-to-iterable.spec.js @@ -0,0 +1,43 @@ +/* eslint-env mocha */ +'use strict' + +const chai = require('chai') +const dirtyChai = require('dirty-chai') +const expect = chai.expect +chai.use(dirtyChai) +const toIterable = require('../src/lib/stream-to-iterable') + +describe('lib/stream-to-iterable', () => { + it('should return input if already async iterable', () => { + const input = { [Symbol.asyncIterator] () { return this } } + expect(toIterable(input)).to.equal(input) + }) + + it('should convert reader to async iterable', async () => { + const inputData = [2, 31, 3, 4] + const input = { + getReader () { + let i = 0 + return { + read () { + return i === inputData.length + ? { done: true } + : { value: inputData[i++] } + }, + releaseLock () {} + } + } + } + + const chunks = [] + for await (const chunk of toIterable(input)) { + chunks.push(chunk) + } + + expect(chunks).to.eql(inputData) + }) + + it('should throw on unknown stream', () => { + expect(() => toIterable({})).to.throw('unknown stream') + }) +}) From a600dce9f0b39f8c423578077bc634144fb0e472 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Tue, 30 Jul 2019 12:15:25 +0100 Subject: [PATCH 18/20] fix: configure tests License: MIT Signed-off-by: Alan Shaw --- src/lib/configure.js | 10 +++------- test/lib.configure.spec.js | 6 +++--- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/lib/configure.js b/src/lib/configure.js index 3acb10db6..a9036d1cd 100644 --- a/src/lib/configure.js +++ b/src/lib/configure.js @@ -1,7 +1,7 @@ 'use strict' /* eslint-env browser */ -const ky = require('ky-universal') +const ky = require('ky-universal').default const { isBrowser, isWebWorker } = require('ipfs-utils/src/env') const { toUri } = require('./multiaddr') const errorHandler = require('./error-handler') @@ -18,11 +18,6 @@ module.exports = create => config => { config = { ...config } } - if (config.protocol || config.host || config.port) { - const port = config.port ? `:${config.port}` : '' - config.apiAddr = `${config.protocol || 'http'}://${config.host || 'localhost'}${port}` - } - config.apiAddr = (config.apiAddr || getDefaultApiAddr(config)).toString() config.apiAddr = config.apiAddr.startsWith('/') ? toUri(config.apiAddr) : config.apiAddr config.apiPath = config.apiPath || config['api-path'] || '/api/v0' @@ -37,7 +32,8 @@ module.exports = create => config => { hooks: { afterResponse: [errorHandler] } - }) + }), + ...config }) } diff --git a/test/lib.configure.spec.js b/test/lib.configure.spec.js index e28abd6c6..f58ca4de7 100644 --- a/test/lib.configure.spec.js +++ b/test/lib.configure.spec.js @@ -14,7 +14,7 @@ describe('lib/configure', () => { it('should accept no config', () => { configure(config => { if (isBrowser || isWebWorker) { - expect(config.apiAddr).to.eql(location.origin) + expect(config.apiAddr).to.eql('') } else { expect(config.apiAddr).to.eql('http://localhost:5001') } @@ -48,7 +48,7 @@ describe('lib/configure', () => { if (isBrowser || isWebWorker) { expect(config.apiAddr).to.eql(`https://${location.host}`) } else { - expect(config.apiAddr).to.eql('https://localhost') + expect(config.apiAddr).to.eql('https://localhost:5001') } })(input) }) @@ -59,7 +59,7 @@ describe('lib/configure', () => { if (isBrowser || isWebWorker) { expect(config.apiAddr).to.eql(`http://ipfs.io:${location.port}`) } else { - expect(config.apiAddr).to.eql('http://ipfs.io') + expect(config.apiAddr).to.eql('http://ipfs.io:5001') } })(input) }) From d34850b09531b96c92cc7385cc787e00b5c125b9 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Tue, 30 Jul 2019 12:23:07 +0100 Subject: [PATCH 19/20] chore: appease linter License: MIT Signed-off-by: Alan Shaw --- src/pubsub/publish.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/pubsub/publish.js b/src/pubsub/publish.js index 3e63ee6f4..c13130e7f 100644 --- a/src/pubsub/publish.js +++ b/src/pubsub/publish.js @@ -14,11 +14,13 @@ module.exports = configure(({ ky }) => { const searchParams = new URLSearchParams(options.searchParams) searchParams.set('arg', topic) - return ky.post(`pubsub/pub?${searchParams}&arg=${encodeBuffer(data)}`, { + const res = await ky.post(`pubsub/pub?${searchParams}&arg=${encodeBuffer(data)}`, { timeout: options.timeout, signal: options.signal, headers: options.headers }).text() + + return res } }) From 6b293cd266d5bdf87bfb24b1f76a361539541a88 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Wed, 28 Aug 2019 10:19:58 +0100 Subject: [PATCH 20/20] feat: support string data in pubsub.publish License: MIT Signed-off-by: Alan Shaw --- src/pubsub/publish.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/pubsub/publish.js b/src/pubsub/publish.js index c13130e7f..a41c8fba0 100644 --- a/src/pubsub/publish.js +++ b/src/pubsub/publish.js @@ -6,10 +6,7 @@ const configure = require('../lib/configure') module.exports = configure(({ ky }) => { return async (topic, data, options) => { options = options || {} - - if (!Buffer.isBuffer(data)) { - throw new Error('data must be a Buffer') - } + data = Buffer.from(data) const searchParams = new URLSearchParams(options.searchParams) searchParams.set('arg', topic)