From 2b3a577fd9f03fb5de8bd42ca5150098256ad933 Mon Sep 17 00:00:00 2001 From: Bernard Mordan Date: Wed, 6 Sep 2017 16:30:39 +0100 Subject: [PATCH 01/11] Adds wiring for a progress bar Passes the progress bar option for tranfers over http. Progress is shown for your payload being streamed upto the server. There is a pause. Then you get your list of files. --- src/files/add.js | 2 +- src/utils/request-api.js | 13 ++++++++++++- test/files.spec.js | 9 +++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/files/add.js b/src/files/add.js index 81e8734f4..7f019eadb 100644 --- a/src/files/add.js +++ b/src/files/add.js @@ -41,7 +41,7 @@ module.exports = (send) => { qs.hash = opts.hashAlg } - const request = { path: 'add', files: files, qs: qs } + const request = { path: 'add', files: files, qs: opts, progress: opts.progress } // Transform the response stream to DAGNode values const transform = (res, callback) => DAGNodeStream.streamToValue(send, res, callback) diff --git a/src/utils/request-api.js b/src/utils/request-api.js index 5f78b1e07..0bb84fb61 100644 --- a/src/utils/request-api.js +++ b/src/utils/request-api.js @@ -10,6 +10,7 @@ const getFilesStream = require('./get-files-stream') const streamToValue = require('./stream-to-value') const streamToJsonValue = require('./stream-to-json-value') const request = require('./request') +const Transform = require('readable-stream').Transform // -- Internal @@ -160,7 +161,17 @@ function requestAPI (config, options, callback) { }) if (options.files) { - stream.pipe(req) + if (options.progress && typeof options.progress === 'function') { + const progressStream = new Transform({ + transform: (chunk, encoding, cb) => { + options.progress(chunk.byteLength) + cb(null, chunk) + } + }) + stream.pipe(progressStream).pipe(req) + } else { + stream.pipe(req) + } } else { req.end() } diff --git a/test/files.spec.js b/test/files.spec.js index 629e5e9b0..3da16ffd0 100644 --- a/test/files.spec.js +++ b/test/files.spec.js @@ -87,6 +87,15 @@ describe('.files (the MFS API part)', function () { }) }) + it.only('files.add with progress options', (done) => { + ipfs.files.add(testfile, {progress: false}, (err, res) => { + expect(err).to.not.exist() + + expect(res).to.have.length(1) + done() + }) + }) + HASH_ALGS.forEach((name) => { it(`files.add with hash=${name} and raw-leaves=false`, (done) => { const content = String(Math.random() + Date.now()) From c749c363880de4fc5810ed70156673011c9d309f Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Fri, 8 Sep 2017 12:56:46 +0100 Subject: [PATCH 02/11] feat: Support specify hash algorithm in files.add (#597) * support support specify hash alg * Add test for add with --hash option. Pass raw cid/multihash to ipfs object get/data * Allow object get/data to accept CID * Var naming tweaks from review --- test/files.spec.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/files.spec.js b/test/files.spec.js index 3da16ffd0..36ec10faa 100644 --- a/test/files.spec.js +++ b/test/files.spec.js @@ -87,6 +87,25 @@ describe('.files (the MFS API part)', function () { }) }) + HASH_ALGS.forEach((name) => { + it(`files.add with hash=${name} and raw-leaves=false`, (done) => { + const content = String(Math.random() + Date.now()) + const file = { + path: content + '.txt', + content: Buffer.from(content) + } + const options = { hash: name, 'raw-leaves': false } + + ipfs.files.add([file], options, (err, res) => { + if (err) return done(err) + expect(res).to.have.length(1) + const cid = new CID(res[0].hash) + expect(mh.decode(cid.multihash).name).to.equal(name) + done() + }) + }) + }) + it.only('files.add with progress options', (done) => { ipfs.files.add(testfile, {progress: false}, (err, res) => { expect(err).to.not.exist() From 10b0bde35a2dc106b93d52057b91265cb73a4614 Mon Sep 17 00:00:00 2001 From: Dmitriy Ryajov Date: Sun, 1 Oct 2017 18:31:44 -0600 Subject: [PATCH 03/11] feat: track progress events --- examples/upload-file-via-browser/package.json | 2 +- examples/upload-file-via-browser/src/App.js | 2 +- src/files/add.js | 6 ++- src/utils/progress-stream.js | 49 +++++++++++++++++++ src/utils/request-api.js | 16 ++---- test/files.spec.js | 15 +++++- 6 files changed, 72 insertions(+), 18 deletions(-) create mode 100644 src/utils/progress-stream.js diff --git a/examples/upload-file-via-browser/package.json b/examples/upload-file-via-browser/package.json index d4fc658ca..4b53ccd8c 100644 --- a/examples/upload-file-via-browser/package.json +++ b/examples/upload-file-via-browser/package.json @@ -13,7 +13,7 @@ "devDependencies": { "babel-core": "^5.4.7", "babel-loader": "^5.1.2", - "ipfs-api": "^12.1.7", + "ipfs-api": "../../", "json-loader": "^0.5.4", "react": "^15.4.2", "react-dom": "^15.4.2", diff --git a/examples/upload-file-via-browser/src/App.js b/examples/upload-file-via-browser/src/App.js index 7b913f7f0..102d86e14 100644 --- a/examples/upload-file-via-browser/src/App.js +++ b/examples/upload-file-via-browser/src/App.js @@ -29,7 +29,7 @@ class App extends React.Component { saveToIpfs (reader) { let ipfsId const buffer = Buffer.from(reader.result) - this.ipfsApi.add(buffer) + this.ipfsApi.add(buffer, { progress: (prog) => console.log(`received: ${prog}`) }) .then((response) => { console.log(response) ipfsId = response[0].hash diff --git a/src/files/add.js b/src/files/add.js index 7f019eadb..cfd9a279e 100644 --- a/src/files/add.js +++ b/src/files/add.js @@ -3,6 +3,7 @@ const isStream = require('is-stream') const promisify = require('promisify-es6') const DAGNodeStream = require('../utils/dagnode-stream') +const ProgressStream = require('../utils/progress-stream') module.exports = (send) => { return promisify((files, opts, callback) => { @@ -41,10 +42,11 @@ module.exports = (send) => { qs.hash = opts.hashAlg } - const request = { path: 'add', files: files, qs: opts, progress: opts.progress } + const request = { path: 'add', files: files, qs: qs, progress: opts.progress } // Transform the response stream to DAGNode values - const transform = (res, callback) => DAGNodeStream.streamToValue(send, res, callback) + const transform = (res, callback) => DAGNodeStream + .streamToValue(send, ProgressStream.fromStream(opts.progress, res), callback) send.andTransform(request, transform, callback) }) } diff --git a/src/utils/progress-stream.js b/src/utils/progress-stream.js new file mode 100644 index 000000000..272c47c6c --- /dev/null +++ b/src/utils/progress-stream.js @@ -0,0 +1,49 @@ +'use strict' + +const Transform = require('readable-stream').Transform + +/* + A transform stream to track progress events on file upload + + When the progress flag is passed to the HTTP api, the stream + emits progress events like such: + + { + Name string + Hash string `json:",omitempty"` + Bytes int64 `json:",omitempty"` + Size string `json:",omitempty"` + } + + This class will take care of detecting such + events and calling the associated track method + with the bytes sent so far as parameter. It will + also skip them from the stream, emitting only + when the final object has been uploaded and we + got a hash. +*/ +class ProgressStream extends Transform { + constructor (opts) { + opts = Object.assign(opts || {}, { objectMode: true }) + super(opts) + this._track = opts.track || (() => {}) + } + + static fromStream (track, stream) { + const prog = new ProgressStream({ track }) + return stream.pipe(prog) + } + + _transform (chunk, encoding, callback) { + if (chunk && + typeof chunk.Bytes !== 'undefined' && + typeof chunk.Hash === 'undefined') { + this._track(chunk.Bytes) + return callback() + } + + callback(null, chunk) + } +} + +module.exports = ProgressStream diff --git a/src/utils/request-api.js b/src/utils/request-api.js index 0bb84fb61..085cf94ca 100644 --- a/src/utils/request-api.js +++ b/src/utils/request-api.js @@ -10,7 +10,6 @@ const getFilesStream = require('./get-files-stream') const streamToValue = require('./stream-to-value') const streamToJsonValue = require('./stream-to-json-value') const request = require('./request') -const Transform = require('readable-stream').Transform // -- Internal @@ -82,6 +81,9 @@ function requestAPI (config, options, callback) { if (options.files && !Array.isArray(options.files)) { options.files = [options.files] } + if (options.progress) { + options.qs.progress = true + } if (options.qs.r) { options.qs.recursive = options.qs.r @@ -161,17 +163,7 @@ function requestAPI (config, options, callback) { }) if (options.files) { - if (options.progress && typeof options.progress === 'function') { - const progressStream = new Transform({ - transform: (chunk, encoding, cb) => { - options.progress(chunk.byteLength) - cb(null, chunk) - } - }) - stream.pipe(progressStream).pipe(req) - } else { - stream.pipe(req) - } + stream.pipe(req) } else { req.end() } diff --git a/test/files.spec.js b/test/files.spec.js index 36ec10faa..a61c15352 100644 --- a/test/files.spec.js +++ b/test/files.spec.js @@ -106,8 +106,19 @@ describe('.files (the MFS API part)', function () { }) }) - it.only('files.add with progress options', (done) => { - ipfs.files.add(testfile, {progress: false}, (err, res) => { + it('files.add with progress options', (done) => { + let progress = 0 + ipfs.files.add(testfile, { progress: (p) => { progress = p } }, (err, res) => { + expect(err).to.not.exist() + + expect(res).to.have.length(1) + expect(progress).to.be.greaterThan(0) + done() + }) + }) + + it('files.add without progress options', (done) => { + ipfs.files.add(testfile, (err, res) => { expect(err).to.not.exist() expect(res).to.have.length(1) From 8885ddbb0f5379e122cf9fc3b46d3fe6ca4d55fe Mon Sep 17 00:00:00 2001 From: David Dias Date: Fri, 6 Oct 2017 15:42:11 +0300 Subject: [PATCH 04/11] tesT: add more tests --- test/files.spec.js | 61 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 5 deletions(-) diff --git a/test/files.spec.js b/test/files.spec.js index a61c15352..03fd88fc7 100644 --- a/test/files.spec.js +++ b/test/files.spec.js @@ -48,7 +48,7 @@ describe('.files (the MFS API part)', function () { after((done) => fc.dismantle(done)) - describe('Callback API', function () { + describe.only('Callback API', function () { this.timeout(120 * 1000) it('add file for testing', (done) => { @@ -106,13 +106,64 @@ describe('.files (the MFS API part)', function () { }) }) - it('files.add with progress options', (done) => { - let progress = 0 - ipfs.files.add(testfile, { progress: (p) => { progress = p } }, (err, res) => { + it('files.add file with progress option', (done) => { + let progress + let progressCount = 0 + + const progressHandler = (p) => { + progressCount += 1 + progress = p + } + + ipfs.files.add(testfile, { progress: progressHandler }, (err, res) => { + expect(err).to.not.exist() + + expect(res).to.have.length(1) + expect(progress).to.be.equal(100) + expect(progressCount).to.be.greaterThan(0) + + done() + }) + }) + + it('files.add big file with progress option', (done) => { + let progress + let progressCount = 0 + + const progressHandler = (p) => { + progressCount += 1 + progress = p + } + + // TODO: needs to be using a big file + ipfs.files.add(testfile, { progress: progressHandler }, (err, res) => { expect(err).to.not.exist() expect(res).to.have.length(1) - expect(progress).to.be.greaterThan(0) + expect(progress).to.be.equal(100) + expect(progressCount).to.be.greaterThan(0) + + done() + }) + }) + + it('files.add directory with progress option', (done) => { + let progress + let progressCount = 0 + + const progressHandler = (p) => { + progressCount += 1 + progress = p + } + + // TODO: needs to be using a directory + ipfs.files.add(testfile, { progress: progressHandler }, (err, res) => { + expect(err).to.not.exist() + + expect(res).to.have.length(1) + expect(progress).to.be.equal(100) + expect(progressCount).to.be.greaterThan(0) + done() }) }) From 2ff8312d49be55bd3c8f8b710350f5b089014644 Mon Sep 17 00:00:00 2001 From: Dmitriy Ryajov Date: Tue, 10 Oct 2017 02:43:58 -0600 Subject: [PATCH 05/11] fix: tidy up tests --- src/utils/progress-stream.js | 5 +++++ test/files.spec.js | 16 ++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/utils/progress-stream.js b/src/utils/progress-stream.js index 272c47c6c..eea6042e6 100644 --- a/src/utils/progress-stream.js +++ b/src/utils/progress-stream.js @@ -42,6 +42,11 @@ class ProgressStream extends Transform { return callback() } + if (typeof chunk.Message !== 'undefined' || + typeof chunk.Code !== 'undefined') { + this.emit('error', chunk) + } + callback(null, chunk) } } diff --git a/test/files.spec.js b/test/files.spec.js index 03fd88fc7..69508af3f 100644 --- a/test/files.spec.js +++ b/test/files.spec.js @@ -119,15 +119,15 @@ describe('.files (the MFS API part)', function () { expect(err).to.not.exist() expect(res).to.have.length(1) - expect(progress).to.be.equal(100) - expect(progressCount).to.be.greaterThan(0) + expect(progress).to.be.equal(testfile.byteLength) + expect(progressCount).to.be.equal(1) done() }) }) it('files.add big file with progress option', (done) => { - let progress + let progress = 0 let progressCount = 0 const progressHandler = (p) => { @@ -140,15 +140,15 @@ describe('.files (the MFS API part)', function () { expect(err).to.not.exist() expect(res).to.have.length(1) - expect(progress).to.be.equal(100) - expect(progressCount).to.be.greaterThan(0) + expect(progress).to.be.equal(testfile.byteLength) + expect(progressCount).to.be.equal(1) done() }) }) it('files.add directory with progress option', (done) => { - let progress + let progress = 0 let progressCount = 0 const progressHandler = (p) => { @@ -161,8 +161,8 @@ describe('.files (the MFS API part)', function () { expect(err).to.not.exist() expect(res).to.have.length(1) - expect(progress).to.be.equal(100) - expect(progressCount).to.be.greaterThan(0) + expect(progress).to.be.equal(testfile.byteLength) + expect(progressCount).to.be.equal(1) done() }) From f6c0e2115f838755fbd99c76f07b92378b2f6029 Mon Sep 17 00:00:00 2001 From: Dmitriy Ryajov Date: Wed, 11 Oct 2017 22:48:09 -0700 Subject: [PATCH 06/11] feat: send errors as trailer headers --- src/utils/progress-stream.js | 5 ----- src/utils/request-api.js | 13 +++++++++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/utils/progress-stream.js b/src/utils/progress-stream.js index eea6042e6..272c47c6c 100644 --- a/src/utils/progress-stream.js +++ b/src/utils/progress-stream.js @@ -42,11 +42,6 @@ class ProgressStream extends Transform { return callback() } - if (typeof chunk.Message !== 'undefined' || - typeof chunk.Code !== 'undefined') { - this.emit('error', chunk) - } - callback(null, chunk) } } diff --git a/src/utils/request-api.js b/src/utils/request-api.js index 085cf94ca..e196d1705 100644 --- a/src/utils/request-api.js +++ b/src/utils/request-api.js @@ -48,6 +48,19 @@ function onRes (buffer, cb) { // Return a stream of JSON objects if (chunkedObjects && isJson) { const outputStream = pump(res, ndjson.parse()) + res.on('end', () => { + // if we got an error here, we have to throw + // a) we can't `outputStream.destroy(err)` because its already closed + // b) the callback has been already called, no point in calling it again + let err = res.trailers['x-stream-error'] + if (err) { + err = JSON.parse(err) + const error = new Error(`Server responded with 500`) + error.code = err.Code + error.message = err.Message + throw error + } + }) return cb(null, outputStream) } From 843e699a8aed8f7b21c953fc93ce1648270e6147 Mon Sep 17 00:00:00 2001 From: Dmitriy Ryajov Date: Wed, 11 Oct 2017 22:57:18 -0700 Subject: [PATCH 07/11] fix: remove .only from tests --- src/utils/request-api.js | 3 --- test/files.spec.js | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/utils/request-api.js b/src/utils/request-api.js index e196d1705..66483bef7 100644 --- a/src/utils/request-api.js +++ b/src/utils/request-api.js @@ -49,9 +49,6 @@ function onRes (buffer, cb) { if (chunkedObjects && isJson) { const outputStream = pump(res, ndjson.parse()) res.on('end', () => { - // if we got an error here, we have to throw - // a) we can't `outputStream.destroy(err)` because its already closed - // b) the callback has been already called, no point in calling it again let err = res.trailers['x-stream-error'] if (err) { err = JSON.parse(err) diff --git a/test/files.spec.js b/test/files.spec.js index 69508af3f..3bfc4bf87 100644 --- a/test/files.spec.js +++ b/test/files.spec.js @@ -48,7 +48,7 @@ describe('.files (the MFS API part)', function () { after((done) => fc.dismantle(done)) - describe.only('Callback API', function () { + describe('Callback API', function () { this.timeout(120 * 1000) it('add file for testing', (done) => { From 1845f2b742c26024cd3cd3df7760b6c035d9fedf Mon Sep 17 00:00:00 2001 From: Dmitriy Ryajov Date: Sat, 14 Oct 2017 11:37:14 -0700 Subject: [PATCH 08/11] feat: signal error on reponse stream if x-stream-error --- src/utils/request-api.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/utils/request-api.js b/src/utils/request-api.js index 66483bef7..412524aaf 100644 --- a/src/utils/request-api.js +++ b/src/utils/request-api.js @@ -48,6 +48,12 @@ function onRes (buffer, cb) { // Return a stream of JSON objects if (chunkedObjects && isJson) { const outputStream = pump(res, ndjson.parse()) + // TODO: This needs reworking. + // this is a chicken and egg problem - + // 1) we can't get Trailer headers unless the response ends + // 2) we can't propagate the error, because the response stream + // is closed + // (perhaps we can workaround this using pull-streams) res.on('end', () => { let err = res.trailers['x-stream-error'] if (err) { @@ -55,7 +61,7 @@ function onRes (buffer, cb) { const error = new Error(`Server responded with 500`) error.code = err.Code error.message = err.Message - throw error + outputStream.destroy(error) // error is not going to be propagated } }) return cb(null, outputStream) From 1891bbf4a8c0b68b1595bdd84e13030e66a8f696 Mon Sep 17 00:00:00 2001 From: Dmitriy Ryajov Date: Sat, 14 Oct 2017 14:06:31 -0700 Subject: [PATCH 09/11] test: adding tests for trailer header errors --- test/request-api.spec.js | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/test/request-api.spec.js b/test/request-api.spec.js index 6588ccbbd..18e45d522 100644 --- a/test/request-api.spec.js +++ b/test/request-api.spec.js @@ -7,10 +7,14 @@ const expect = chai.expect chai.use(dirtyChai) const isNode = require('detect-node') const ipfsAPI = require('../src/index.js') +const ndjson = require('ndjson') +const pump = require('pump') describe('\'deal with HTTP weirdness\' tests', () => { 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 @@ -27,3 +31,34 @@ describe('\'deal with HTTP weirdness\' tests', () => { }) }) }) + +describe('trailer headers', () => { + it('should deal with trailer x-stream-error correctly', (done) => { + if (!isNode) { + return done() + } + + const server = require('http').createServer((req, res) => { + const resStream = pump(res, ndjson.stringify()) + res.setHeader('x-chunked-output', '1') + res.setHeader('content-type', 'application/json') + res.setHeader('Trailer', 'X-Stream-Error') + res.addTrailers({ 'X-Stream-Error': JSON.stringify({ Message: 'ups, something went wrong', Code: 500 }) }) + resStream.write({ Bytes: 1 }) + res.end() + }) + + server.listen(6001, () => { + const ipfs = ipfsAPI('/ip4/127.0.0.1/tcp/6001') + /* eslint-disable */ + ipfs.files.add(Buffer.from('Hello there!'), (err, res) => { + // TODO: error's are not being correctly + // propagated with Trailer headers yet + // expect(err).to.exist() + expect(res).to.not.equal(0) + server.close(done) + }) + /* eslint-enable */ + }) + }) +}) From ecbf3e64c841a896ac83523b2dfccb21cf72e4bd Mon Sep 17 00:00:00 2001 From: David Dias Date: Wed, 18 Oct 2017 10:20:47 +0100 Subject: [PATCH 10/11] chore: update interface-ipfs-core --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 9499fefb1..dca51f1c0 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,7 @@ "eslint-plugin-react": "^7.3.0", "gulp": "^3.9.1", "hapi": "^16.5.2", - "interface-ipfs-core": "~0.31.19", + "interface-ipfs-core": "~0.32.0", "ipfsd-ctl": "~0.23.0", "pre-commit": "^1.2.2", "socket.io": "^2.0.3", From 9a11a8308371c366ce2a5cccd1c0f98e0a0723ef Mon Sep 17 00:00:00 2001 From: Dmitriy Ryajov Date: Wed, 18 Oct 2017 06:37:07 -0700 Subject: [PATCH 11/11] chore: upgrade to latest interface-ipfs-core --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 89b0c6231..7a4577355 100644 --- a/package.json +++ b/package.json @@ -65,7 +65,7 @@ "dirty-chai": "^2.0.1", "eslint-plugin-react": "^7.4.0", "gulp": "^3.9.1", - "interface-ipfs-core": "~0.32.0", + "interface-ipfs-core": "~0.32.1", "hapi": "^16.6.2", "ipfsd-ctl": "~0.23.0", "pre-commit": "^1.2.2",