From e79291de3d4bbc65910535a4ee25d731e996b701 Mon Sep 17 00:00:00 2001 From: Friedel Ziegelmayer Date: Mon, 22 Aug 2016 22:52:36 +0200 Subject: [PATCH 1/5] basics for move to fetch --- package.json | 7 +- src/api/object.js | 4 +- src/api/util/url-add.js | 5 +- src/get-dagnode.js | 2 +- src/request-api.js | 108 ++++++++++++++++++------------ test/ipfs-api/request-api.spec.js | 2 +- 6 files changed, 77 insertions(+), 51 deletions(-) diff --git a/package.json b/package.json index 913d5c9d0..c0f920b7a 100644 --- a/package.json +++ b/package.json @@ -17,11 +17,13 @@ "coverage-publish": "aegir-coverage publish" }, "dependencies": { + "arraybuffer-to-buffer": "0.0.4", "async": "^2.0.1", "babel-runtime": "^6.11.6", "bl": "^1.1.2", "bs58": "^3.0.0", "detect-node": "^2.0.3", + "fetch-ponyfill": "^1.0.0", "flatmap": "0.0.3", "glob": "^7.0.5", "ipfs-merkle-dag": "^0.6.0", @@ -32,8 +34,7 @@ "ndjson": "^1.4.3", "promisify-es6": "^1.0.1", "qs": "^6.2.1", - "tar-stream": "^1.5.2", - "wreck": "^9.0.0" + "tar-stream": "^1.5.2" }, "engines": { "node": ">=4.2.2" @@ -99,4 +100,4 @@ "url": "https://github.com/ipfs/js-ipfs-api/issues" }, "homepage": "https://github.com/ipfs/js-ipfs-api" -} \ No newline at end of file +} diff --git a/src/api/object.js b/src/api/object.js index 787b36276..df9cd78b5 100644 --- a/src/api/object.js +++ b/src/api/object.js @@ -1,7 +1,7 @@ 'use strict' -const DAGNode = require('ipfs-merkle-dag').DAGNode -const DAGLink = require('ipfs-merkle-dag').DAGLink +const DAGNode = require('ipfs-merkle-dag/lib/dag-node') +const DAGLink = require('ipfs-merkle-dag/lib/dag-link') const promisify = require('promisify-es6') const bs58 = require('bs58') const bl = require('bl') diff --git a/src/api/util/url-add.js b/src/api/util/url-add.js index f8707ca9d..6e51200eb 100644 --- a/src/api/util/url-add.js +++ b/src/api/util/url-add.js @@ -1,6 +1,7 @@ 'use strict' -const Wreck = require('wreck') +// const Wreck = require('wreck') +const fetch = require('fetch-ponyfill') const addToDagNodesTransform = require('./../../add-to-dagnode-transform') const promisify = require('promisify-es6') @@ -28,7 +29,7 @@ module.exports = (send) => { const sendWithTransform = send.withTransform(addToDagNodesTransform) - Wreck.request('GET', url, null, (err, res) => { + fetch('GET', url, null, (err, res) => { if (err) { return callback(err) } diff --git a/src/get-dagnode.js b/src/get-dagnode.js index 2d9e937e2..775412035 100644 --- a/src/get-dagnode.js +++ b/src/get-dagnode.js @@ -1,6 +1,6 @@ 'use strict' -const DAGNode = require('ipfs-merkle-dag').DAGNode +const DAGNode = require('ipfs-merkle-dag/lib/dag-node') const bl = require('bl') const parallel = require('async/parallel') diff --git a/src/request-api.js b/src/request-api.js index 2467a005b..5d69fd6ef 100644 --- a/src/request-api.js +++ b/src/request-api.js @@ -1,69 +1,85 @@ 'use strict' -const Wreck = require('wreck') +require('throw-rejects')() +const fetch = require('fetch-ponyfill')() const Qs = require('qs') -const ndjson = require('ndjson') +const toBuffer = require('arraybuffer-to-buffer') +const Readable = require('stream').Readable const getFilesStream = require('./get-files-stream') const isNode = require('detect-node') // -- Internal -function parseChunkedJson (res, cb) { - const parsed = [] - res - .pipe(ndjson.parse()) - .on('data', (obj) => { - parsed.push(obj) - }) - .on('end', () => { - cb(null, parsed) - }) +// node-fetch has res.buffer +// window.fetch has res.arrayBuffer +function bufferReturn (res) { + if (res.buffer) { + return res.buffer() + } else if (res.arrayBuffer) { + return res.arrayBuffer().then(toBuffer) + } else { + return res.text().then(Buffer) + } } -function onRes (buffer, cb, uri) { - return (err, res) => { - if (err) { - return cb(err) - } - const stream = Boolean(res.headers['x-stream-output']) - const chunkedObjects = Boolean(res.headers['x-chunked-output']) - const isJson = res.headers['content-type'] && - res.headers['content-type'].indexOf('application/json') === 0 +function onRes (buffer) { + return (res) => { + const stream = Boolean(res.headers.get('x-stream-output')) + const chunkedObjects = Boolean(res.headers.get('x-chunked-output')) + const isJson = res.headers.has('content-type') && + res.headers.get('content-type').indexOf('application/json') === 0 - if (res.statusCode >= 400 || !res.statusCode) { + if (!res.ok) { const error = new Error(`Server responded with ${res.statusCode}`) - return Wreck.read(res, {json: true}, (err, payload) => { - if (err) { - return cb(err) - } - if (payload) { - error.code = payload.Code - error.message = payload.Message || payload.toString() - } - cb(error) - }) + return res.json() + .then((payload) => { + if (payload) { + error.code = payload.Code + error.message = payload.Message || payload.toString() + } + + throw error + }) } if (stream && !buffer) { - return cb(null, res) + return bufferReturn(res) + .then((raw) => { + // this makes me sad + const s = new Readable() + s.push(raw) + s.push(null) + return s + }) } if (chunkedObjects) { - if (isJson) { - return parseChunkedJson(res, cb) + if (!isJson) { + return bufferReturn(res) } - return Wreck.read(res, null, cb) + return bufferReturn(res) + .then((raw) => { + return raw + .toString() + .split('\n') + .map(JSON.parse) + }) } - Wreck.read(res, {json: isJson}, cb) + // Can't use res.json() as it throws on empty responses + return res.text().then((raw) => { + if (raw) { + return JSON.parse(raw) + } + }) } } -function requestAPI (config, options, callback) { +function requestAPI (config, options) { options.qs = options.qs || {} if (Array.isArray(options.files)) { @@ -114,14 +130,20 @@ function requestAPI (config, options, callback) { if (options.files) { if (!stream.boundary) { - return callback(new Error('No boundary in multipart stream')) + return Promise.reject(new Error('No boundary in multipart stream')) } opts.headers['Content-Type'] = `multipart/form-data; boundary=${stream.boundary}` - opts.payload = stream + opts.body = stream } - return Wreck.request(opts.method, opts.uri, opts, onRes(options.buffer, callback, opts.uri)) + return fetch(opts.uri, { + headers: opts.headers, + method: opts.method, + mode: 'cors', + body: opts.body + }) + .then(onRes(options.buffer)) } // @@ -142,7 +164,9 @@ exports = module.exports = function getRequestAPI (config) { return callback(new Error('no options were passed')) } - return requestAPI(config, options, callback) + return requestAPI(config, options) + .then((res) => callback(null, res)) + .catch((err) => callback(err)) } // Wraps the 'send' function such that an asynchronous diff --git a/test/ipfs-api/request-api.spec.js b/test/ipfs-api/request-api.spec.js index 79219b7d7..0f3657fdc 100644 --- a/test/ipfs-api/request-api.spec.js +++ b/test/ipfs-api/request-api.spec.js @@ -6,7 +6,7 @@ const isNode = require('detect-node') const ipfsAPI = require('./../../src/index.js') const noop = () => {} -describe('ipfsAPI request tests', () => { +describe.skip('ipfsAPI request tests', () => { describe('requestAPI', () => { const apiAddrs = require('./../setup/tmp-disposable-nodes-addrs.json') const apiAddr = apiAddrs.a.split('/') From c28770ac93882e4163d903a694943614b9e21547 Mon Sep 17 00:00:00 2001 From: Friedel Ziegelmayer Date: Tue, 23 Aug 2016 17:17:12 +0200 Subject: [PATCH 2/5] node tests passing, browser blocked --- package.json | 6 ++- src/api/util/url-add.js | 31 ++++++------ src/buffer-return.js | 13 ++++++ src/request-api.js | 70 ++++++++++++++-------------- test/interface-ipfs-core/log.spec.js | 4 +- 5 files changed, 72 insertions(+), 52 deletions(-) create mode 100644 src/buffer-return.js diff --git a/package.json b/package.json index c0f920b7a..d545b01ab 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "bl": "^1.1.2", "bs58": "^3.0.0", "detect-node": "^2.0.3", - "fetch-ponyfill": "^1.0.0", + "fetch-ponyfill": "github:dignifiedquire/fetch-ponyfill#build", "flatmap": "0.0.3", "glob": "^7.0.5", "ipfs-merkle-dag": "^0.6.0", @@ -34,7 +34,9 @@ "ndjson": "^1.4.3", "promisify-es6": "^1.0.1", "qs": "^6.2.1", - "tar-stream": "^1.5.2" + "streamifier": "^0.1.1", + "tar-stream": "^1.5.2", + "to-arraybuffer": "^1.0.1" }, "engines": { "node": ">=4.2.2" diff --git a/src/api/util/url-add.js b/src/api/util/url-add.js index 6e51200eb..25b20a696 100644 --- a/src/api/util/url-add.js +++ b/src/api/util/url-add.js @@ -1,8 +1,8 @@ 'use strict' -// const Wreck = require('wreck') -const fetch = require('fetch-ponyfill') +const fetch = require('fetch-ponyfill')() const addToDagNodesTransform = require('./../../add-to-dagnode-transform') +const bufferReturn = require('../../buffer-return') const promisify = require('promisify-es6') @@ -29,16 +29,21 @@ module.exports = (send) => { const sendWithTransform = send.withTransform(addToDagNodesTransform) - fetch('GET', url, null, (err, res) => { - if (err) { - return callback(err) - } - - sendWithTransform({ - path: 'add', - qs: opts, - files: res - }, callback) - }) + fetch(url) + .then((res) => { + if (!res.ok) { + throw new Error(`Failed to fetch: ${url}`) + } + + return bufferReturn(res) + }) + .then((content) => { + sendWithTransform({ + path: 'add', + qs: opts, + files: content + }, callback) + }) + .catch(callback) }) } diff --git a/src/buffer-return.js b/src/buffer-return.js new file mode 100644 index 000000000..f01bf926d --- /dev/null +++ b/src/buffer-return.js @@ -0,0 +1,13 @@ +'use strict' + +const toBuffer = require('arraybuffer-to-buffer') + +// node-fetch has res.buffer +// window.fetch has res.arrayBuffer +module.exports = function bufferReturn (res) { + if (res.buffer) { + return res.buffer() + } else { + return res.arrayBuffer().then(toBuffer) + } +} diff --git a/src/request-api.js b/src/request-api.js index 5d69fd6ef..411201642 100644 --- a/src/request-api.js +++ b/src/request-api.js @@ -1,28 +1,17 @@ 'use strict' -require('throw-rejects')() +// require('throw-rejects')() const fetch = require('fetch-ponyfill')() const Qs = require('qs') -const toBuffer = require('arraybuffer-to-buffer') -const Readable = require('stream').Readable -const getFilesStream = require('./get-files-stream') - +const toReadStream = require('streamifier').createReadStream const isNode = require('detect-node') +const bl = require('bl') +const toArrayBuffer = require('to-arraybuffer') -// -- Internal - -// node-fetch has res.buffer -// window.fetch has res.arrayBuffer -function bufferReturn (res) { - if (res.buffer) { - return res.buffer() - } else if (res.arrayBuffer) { - return res.arrayBuffer().then(toBuffer) - } else { - return res.text().then(Buffer) - } -} +const getFilesStream = require('./get-files-stream') +const bufferReturn = require('./buffer-return') +// -- Internal function onRes (buffer) { return (res) => { @@ -46,14 +35,7 @@ function onRes (buffer) { } if (stream && !buffer) { - return bufferReturn(res) - .then((raw) => { - // this makes me sad - const s = new Readable() - s.push(raw) - s.push(null) - return s - }) + return bufferReturn(res).then(toReadStream) } if (chunkedObjects) { @@ -63,10 +45,15 @@ function onRes (buffer) { return bufferReturn(res) .then((raw) => { - return raw - .toString() - .split('\n') - .map(JSON.parse) + const parts = raw.toString().split('\n').filter(Boolean) + try { + return parts + .map(JSON.parse) + } catch (err) { + console.error(parts) + console.error(err.stack) + throw err + } }) } @@ -137,12 +124,23 @@ function requestAPI (config, options) { opts.body = stream } - return fetch(opts.uri, { - headers: opts.headers, - method: opts.method, - mode: 'cors', - body: opts.body - }) + return Promise.resolve(opts.body) + .then((body) => { + if (!body || !body.pipe || isNode) return body + + return new Promise((resolve, reject) => { + body.pipe(bl((err, buf) => { + if (err) return reject(err) + resolve(toArrayBuffer(buf)) + })) + }) + }) + .then((body) => fetch(opts.uri, { + headers: opts.headers, + method: opts.method, + mode: 'cors', + body: body + })) .then(onRes(options.buffer)) } diff --git a/test/interface-ipfs-core/log.spec.js b/test/interface-ipfs-core/log.spec.js index 11710b780..6ba6e6f67 100644 --- a/test/interface-ipfs-core/log.spec.js +++ b/test/interface-ipfs-core/log.spec.js @@ -10,7 +10,9 @@ const FactoryClient = require('../factory/factory-client') const isPhantom = !isNode && typeof navigator !== 'undefined' && navigator.userAgent.match(/PhantomJS/) if (!isPhantom) { - describe('.log', () => { + // How did this work before + // our polyfill didn't have real streaming support + describe.skip('.log', () => { let ipfs let fc From de9e51128a7e3647bdda52422c8e339121309234 Mon Sep 17 00:00:00 2001 From: Friedel Ziegelmayer Date: Tue, 23 Aug 2016 17:56:11 +0200 Subject: [PATCH 3/5] better fetch --- package.json | 3 +-- src/api/util/url-add.js | 2 +- src/index.js | 1 + src/request-api.js | 3 +-- test/ipfs-api/request-api.spec.js | 45 +++---------------------------- 5 files changed, 7 insertions(+), 47 deletions(-) diff --git a/package.json b/package.json index d545b01ab..b96b0bc1c 100644 --- a/package.json +++ b/package.json @@ -23,15 +23,14 @@ "bl": "^1.1.2", "bs58": "^3.0.0", "detect-node": "^2.0.3", - "fetch-ponyfill": "github:dignifiedquire/fetch-ponyfill#build", "flatmap": "0.0.3", "glob": "^7.0.5", "ipfs-merkle-dag": "^0.6.0", "is-ipfs": "^0.2.0", + "isomorphic-fetch": "^2.2.1", "isstream": "^0.1.2", "multiaddr": "^2.0.2", "multipart-stream": "^2.0.1", - "ndjson": "^1.4.3", "promisify-es6": "^1.0.1", "qs": "^6.2.1", "streamifier": "^0.1.1", diff --git a/src/api/util/url-add.js b/src/api/util/url-add.js index 25b20a696..83f87f2d4 100644 --- a/src/api/util/url-add.js +++ b/src/api/util/url-add.js @@ -1,6 +1,6 @@ +/* globals fetch:false */ 'use strict' -const fetch = require('fetch-ponyfill')() const addToDagNodesTransform = require('./../../add-to-dagnode-transform') const bufferReturn = require('../../buffer-return') diff --git a/src/index.js b/src/index.js index e5b4f3399..d2738edac 100644 --- a/src/index.js +++ b/src/index.js @@ -1,5 +1,6 @@ 'use strict' +require('isomorphic-fetch') const multiaddr = require('multiaddr') const loadCommands = require('./load-commands') const getConfig = require('./default-config') diff --git a/src/request-api.js b/src/request-api.js index 411201642..536ca2b50 100644 --- a/src/request-api.js +++ b/src/request-api.js @@ -1,7 +1,6 @@ +/* globals fetch:false */ 'use strict' -// require('throw-rejects')() -const fetch = require('fetch-ponyfill')() const Qs = require('qs') const toReadStream = require('streamifier').createReadStream const isNode = require('detect-node') diff --git a/test/ipfs-api/request-api.spec.js b/test/ipfs-api/request-api.spec.js index 0f3657fdc..3a5237dce 100644 --- a/test/ipfs-api/request-api.spec.js +++ b/test/ipfs-api/request-api.spec.js @@ -1,54 +1,15 @@ /* eslint-env mocha */ +/* eslint max-nested-callbacks: ["error", 8] */ 'use strict' const expect = require('chai').expect const isNode = require('detect-node') const ipfsAPI = require('./../../src/index.js') -const noop = () => {} -describe.skip('ipfsAPI request tests', () => { +describe('ipfsAPI request tests', () => { describe('requestAPI', () => { - const apiAddrs = require('./../setup/tmp-disposable-nodes-addrs.json') - const apiAddr = apiAddrs.a.split('/') - - it('excludes port from URL if config.port is falsy', (done) => { - const Wreck = require('wreck') - const request = Wreck.request - - Wreck.request = (method, uri, opts, cb) => { - Wreck.request = request - expect(uri).to.not.contain(/:\d/) - done() - } - - ipfsAPI({ - host: apiAddr[2], - port: null, - protocol: 'http' - }).id(noop) - }) - - it('includes port in URL if config.port is truthy', (done) => { - const Wreck = require('wreck') - const request = Wreck.request - - Wreck.request = (method, uri, opts, cb) => { - Wreck.request = request - expect(uri).to.contain(':' + apiAddr[4]) - done() - } - - ipfsAPI({ - host: apiAddr[2], - port: apiAddr[4], - protocol: 'http' - }).id(noop) - }) - it('does not crash if no content-type header is provided', (done) => { - if (!isNode) { - return done() - } + if (!isNode) return done() // go-ipfs always (currently) adds a content-type header, even if no content is present, // the standard behaviour for an http-api is to omit this header if no content is present From b502ccefe63f24c5463b7f64e4a6744b86c9374b Mon Sep 17 00:00:00 2001 From: Friedel Ziegelmayer Date: Tue, 23 Aug 2016 18:00:13 +0200 Subject: [PATCH 4/5] drop memory flag --- package.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index b96b0bc1c..3ffbc43bb 100644 --- a/package.json +++ b/package.json @@ -5,14 +5,14 @@ "main": "lib/index.js", "jsnext:main": "src/index.js", "scripts": { - "test": "node --max_old_space_size=4096 node_modules/.bin/gulp test:node", + "test": "gulp test", "test:node": "gulp test:node", - "test:browser": "node --max_old_space_size=4096 node_modules/.bin/gulp test:browser", + "test:browser": "gulp test:browser", "lint": "aegir-lint", "build": "gulp build", - "release": "node --max_old_space_size=4096 node_modules/.bin/gulp release", - "release-minor": "node --max_old_space_size=4096 node_modules/.bin/gulp release --type minor", - "release-major": "node --max_old_space_size=4096 node_modules/.bin/gulp release --type major", + "release": "gulp release", + "release-minor": "gulp release --type minor", + "release-major": "gulp release --type major", "coverage": "gulp coverage", "coverage-publish": "aegir-coverage publish" }, From 25bf1d15731df128bc9826b65c5527eabedebb51 Mon Sep 17 00:00:00 2001 From: Friedel Ziegelmayer Date: Wed, 24 Aug 2016 12:57:57 +0200 Subject: [PATCH 5/5] fix: log works in Node.js and Chrome --- package.json | 4 ++ src/api/log.js | 11 ++--- src/request-api.js | 66 +++++++++++++++++++++++++--- test/interface-ipfs-core/log.spec.js | 4 +- 4 files changed, 70 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index 3ffbc43bb..3ec5bcede 100644 --- a/package.json +++ b/package.json @@ -32,7 +32,11 @@ "multiaddr": "^2.0.2", "multipart-stream": "^2.0.1", "promisify-es6": "^1.0.1", + "pull-split": "^0.2.0", + "pull-stream": "^3.4.3", + "pull-stream-to-stream": "^1.3.0", "qs": "^6.2.1", + "stream-to-pull-stream": "^1.7.0", "streamifier": "^0.1.1", "tar-stream": "^1.5.2", "to-arraybuffer": "^1.0.1" diff --git a/src/api/log.js b/src/api/log.js index e4da33864..fec6e7502 100644 --- a/src/api/log.js +++ b/src/api/log.js @@ -1,19 +1,14 @@ 'use strict' -const ndjson = require('ndjson') const promisify = require('promisify-es6') module.exports = (send) => { return { tail: promisify((callback) => { send({ - path: 'log/tail' - }, (err, response) => { - if (err) { - return callback(err) - } - callback(null, response.pipe(ndjson.parse())) - }) + path: 'log/tail', + ndjson: true + }, callback) }) } } diff --git a/src/request-api.js b/src/request-api.js index 536ca2b50..eca3b86f4 100644 --- a/src/request-api.js +++ b/src/request-api.js @@ -2,17 +2,73 @@ 'use strict' const Qs = require('qs') -const toReadStream = require('streamifier').createReadStream const isNode = require('detect-node') const bl = require('bl') const toArrayBuffer = require('to-arraybuffer') +const pull = require('pull-stream') +const toStream = require('pull-stream-to-stream') +const toPull = require('stream-to-pull-stream') +const toBuffer = require('arraybuffer-to-buffer') +const split = require('pull-split') const getFilesStream = require('./get-files-stream') const bufferReturn = require('./buffer-return') // -- Internal -function onRes (buffer) { +function streamReturn (res, ndjson) { + let stream + if (res.body && res.body.getReader) { + // Chrome implements ReadbleStream + const reader = res.body.getReader() + let ended = false + stream = (end, cb) => { + if (end) ended = end + if (ended) { + reader.cancel() + return cb(ended) + } + + reader.read() + .then((result) => { + console.log('got result', result) + if (result.done) ended = true + if (ended) return cb(ended) + + const val = result.value + cb(null, Buffer.isBuffer(val) ? val : toBuffer(val)) + }) + .catch((err) => { + ended = err + cb(ended) + }) + } + } + + // node-fetch has PassThrough stream as body + if (res.body && res.body.readable) { + if (!ndjson) { + return res.body + } + + stream = toPull.source(res.body) + } + + if (stream) { + if (ndjson) { + return toStream.source(pull( + stream, + split('\n', JSON.parse) + )) + } else { + return toStream.source(stream) + } + } + + throw new Error('Streaming is not supported by our browser') +} + +function onRes (buffer, ndjson) { return (res) => { const stream = Boolean(res.headers.get('x-stream-output')) const chunkedObjects = Boolean(res.headers.get('x-chunked-output')) @@ -34,7 +90,7 @@ function onRes (buffer) { } if (stream && !buffer) { - return bufferReturn(res).then(toReadStream) + return streamReturn(res, ndjson) } if (chunkedObjects) { @@ -49,8 +105,6 @@ function onRes (buffer) { return parts .map(JSON.parse) } catch (err) { - console.error(parts) - console.error(err.stack) throw err } }) @@ -140,7 +194,7 @@ function requestAPI (config, options) { mode: 'cors', body: body })) - .then(onRes(options.buffer)) + .then(onRes(options.buffer, options.ndjson)) } // diff --git a/test/interface-ipfs-core/log.spec.js b/test/interface-ipfs-core/log.spec.js index 6ba6e6f67..6e1b52307 100644 --- a/test/interface-ipfs-core/log.spec.js +++ b/test/interface-ipfs-core/log.spec.js @@ -12,7 +12,7 @@ const isPhantom = !isNode && typeof navigator !== 'undefined' && navigator.userA if (!isPhantom) { // How did this work before // our polyfill didn't have real streaming support - describe.skip('.log', () => { + describe('.log', () => { let ipfs let fc @@ -36,6 +36,7 @@ if (!isPhantom) { expect(req).to.exist res.once('data', (obj) => { + res.end() expect(obj).to.be.an('object') done() }) @@ -47,6 +48,7 @@ if (!isPhantom) { return ipfs.log.tail() .then((res) => { res.once('data', (obj) => { + res.end() expect(obj).to.be.an('object') }) })