From c0a1f7ce9fa48d2ec31b7dd67c4fa6357ef8b15b Mon Sep 17 00:00:00 2001 From: Melvin Philips Date: Wed, 31 Jan 2018 23:54:21 -0800 Subject: [PATCH 01/12] feat: implement `ipfs ping` flags #928 --- src/cli/commands/ping.js | 44 +++++++++++++++++++++++++++++++++ src/core/components/ping.js | 30 ++++++++++++++++++++-- src/http/api/resources/index.js | 1 + src/http/api/resources/ping.js | 18 ++++++++++++++ src/http/api/routes/index.js | 1 + src/http/api/routes/ping.js | 19 ++++++++++++++ test/cli/commands.js | 5 ++-- 7 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 src/cli/commands/ping.js create mode 100644 src/http/api/resources/ping.js create mode 100644 src/http/api/routes/ping.js diff --git a/src/cli/commands/ping.js b/src/cli/commands/ping.js new file mode 100644 index 0000000000..e1103849f5 --- /dev/null +++ b/src/cli/commands/ping.js @@ -0,0 +1,44 @@ +'use strict' + +const print = require('../utils').print + +module.exports = { + command: 'ping ', + + describe: 'Measure the latency of a connection', + + builder: { + count: { + alias: 'n', + type: 'integer', + default: 10 + } + }, + + handler (argv) { + const peerId = argv.peerId + const count = argv.count || 10 + + print('PING ' + peerId) + + let noOfTimes = 0 + let totalTime = 0 + + const pingCb = (err, p) => { + if (err) { + throw err + } + let time = p.Time + totalTime = totalTime + time + noOfTimes = noOfTimes + 1 + print('Pong received: time=' + time + ' ms') + if (noOfTimes === count) { + print('Average latency: ' + totalTime / count + 'ms') + } + } + + for (let i = 0; i < count; i++) { + argv.ipfs.ping(peerId, pingCb) + } + } +} diff --git a/src/core/components/ping.js b/src/core/components/ping.js index eb7a45b34f..bcf7c10f6c 100644 --- a/src/core/components/ping.js +++ b/src/core/components/ping.js @@ -1,9 +1,35 @@ 'use strict' const promisify = require('promisify-es6') +const OFFLINE_ERROR = require('../utils').OFFLINE_ERROR +const PeerId = require('peer-id') +const PeerInfo = require('peer-info') +var Readable = require('stream').Readable module.exports = function ping (self) { - return promisify((callback) => { - callback(new Error('Not implemented')) + return promisify((peerId, cb) => { + if (!self.isOnline()) { + return cb(new Error(OFFLINE_ERROR)) + } + + var outputStream = new Readable() + outputStream._read = function (size) { + } + + let peer + try { + peer = self._libp2pNode.peerBook.get(peerId) + } catch (err) { + peer = new PeerInfo(PeerId.createFromB58String(peerId)) + } + + self._libp2pNode.ping(peer, (err, p) => { + p.once('ping', (time) => { + outputStream.push(JSON.stringify([{}, { Success: true, Time: time }, { Text: 'Average latency: ' + time + ' ms' }])) + outputStream.push(null) + p.stop() + cb(err, outputStream) + }) + }) }) } diff --git a/src/http/api/resources/index.js b/src/http/api/resources/index.js index 08d8d7f2a1..37f38f246b 100644 --- a/src/http/api/resources/index.js +++ b/src/http/api/resources/index.js @@ -3,6 +3,7 @@ exports.version = require('./version') exports.shutdown = require('./shutdown') exports.id = require('./id') +exports.ping = require('./ping') exports.bootstrap = require('./bootstrap') exports.repo = require('./repo') exports.object = require('./object') diff --git a/src/http/api/resources/ping.js b/src/http/api/resources/ping.js new file mode 100644 index 0000000000..b8eaef90f4 --- /dev/null +++ b/src/http/api/resources/ping.js @@ -0,0 +1,18 @@ +'use strict' + +const boom = require('boom') + +exports = module.exports + +exports.get = (request, reply) => { + const ipfs = request.server.app.ipfs + const peerId = request.query.arg + + ipfs.ping(peerId, (err, outputStream) => { + if (err) { + return reply(boom.badRequest(err)) + } + + return reply(outputStream).type('application/json').header('x-chunked-output', '1') + }) +} diff --git a/src/http/api/routes/index.js b/src/http/api/routes/index.js index 2eeb4b137a..d7c30851f7 100644 --- a/src/http/api/routes/index.js +++ b/src/http/api/routes/index.js @@ -9,6 +9,7 @@ module.exports = (server) => { require('./object')(server) require('./repo')(server) require('./config')(server) + require('./ping')(server) require('./swarm')(server) require('./bitswap')(server) require('./file')(server) diff --git a/src/http/api/routes/ping.js b/src/http/api/routes/ping.js new file mode 100644 index 0000000000..3d75a8580f --- /dev/null +++ b/src/http/api/routes/ping.js @@ -0,0 +1,19 @@ +'use strict' + +const resources = require('./../resources') + +module.exports = (server) => { + const api = server.select('API') + + api.route({ + method: '*', + path: '/api/v0/ping', + config: { + payload: { + parse: false, + output: 'stream' + }, + handler: resources.ping.get + } + }) +} diff --git a/test/cli/commands.js b/test/cli/commands.js index 06154cfac6..6886dac40a 100644 --- a/test/cli/commands.js +++ b/test/cli/commands.js @@ -4,7 +4,8 @@ const expect = require('chai').expect const runOnAndOff = require('../utils/on-and-off') -const commandCount = 73 +const commandCount = 74 + describe('commands', () => runOnAndOff((thing) => { let ipfs @@ -13,7 +14,7 @@ describe('commands', () => runOnAndOff((thing) => { ipfs = thing.ipfs }) - it('list the commands', () => { + it.only('list the commands', () => { return ipfs('commands').then((out) => { expect(out.split('\n')).to.have.length(commandCount + 1) }) From 1ecab081952444505710685271bc1793b1b0d665 Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Sun, 25 Mar 2018 21:28:10 +0100 Subject: [PATCH 02/12] feat: first shot at ping implementaion --- package.json | 1 + src/cli/commands/ping.js | 40 ++++++++++++++++------------------ src/core/components/ping.js | 39 ++++++++++++++++++++++++--------- src/http/api/resources/ping.js | 36 ++++++++++++++++++++++-------- src/http/api/routes/ping.js | 3 ++- 5 files changed, 78 insertions(+), 41 deletions(-) diff --git a/package.json b/package.json index 47d861fa06..14f256d181 100644 --- a/package.json +++ b/package.json @@ -156,6 +156,7 @@ "progress": "^2.0.0", "promisify-es6": "^1.0.3", "pull-abortable": "^4.1.1", + "pull-catch": "^1.0.0", "pull-defer": "^0.2.2", "pull-file": "^1.1.0", "pull-ndjson": "^0.1.1", diff --git a/src/cli/commands/ping.js b/src/cli/commands/ping.js index e1103849f5..53638a0423 100644 --- a/src/cli/commands/ping.js +++ b/src/cli/commands/ping.js @@ -1,5 +1,9 @@ 'use strict' +const pull = require('pull-stream/pull') +const drain = require('pull-stream/sinks/drain') +const pullCatch = require('pull-catch') + const print = require('../utils').print module.exports = { @@ -19,26 +23,20 @@ module.exports = { const peerId = argv.peerId const count = argv.count || 10 - print('PING ' + peerId) - - let noOfTimes = 0 - let totalTime = 0 - - const pingCb = (err, p) => { - if (err) { - throw err - } - let time = p.Time - totalTime = totalTime + time - noOfTimes = noOfTimes + 1 - print('Pong received: time=' + time + ' ms') - if (noOfTimes === count) { - print('Average latency: ' + totalTime / count + 'ms') - } - } - - for (let i = 0; i < count; i++) { - argv.ipfs.ping(peerId, pingCb) - } + pull( + argv.ipfs.pingPullStream(peerId, { count }), + pullCatch(err => { + throw err + }), + drain(({ Time, Text }) => { + // Check if it's a pong + if (Time) { + print(`Pong received: time=${Time} ms`) + // Status response + } else { + print(Text) + } + }) + ) } } diff --git a/src/core/components/ping.js b/src/core/components/ping.js index bcf7c10f6c..61e641b3f6 100644 --- a/src/core/components/ping.js +++ b/src/core/components/ping.js @@ -4,32 +4,51 @@ const promisify = require('promisify-es6') const OFFLINE_ERROR = require('../utils').OFFLINE_ERROR const PeerId = require('peer-id') const PeerInfo = require('peer-info') -var Readable = require('stream').Readable +const Readable = require('readable-stream').Readable + +function getPacket (msg) { + // Default msg + const basePacket = {Success: false, Time: 0, Text: ''} + // ndjson + return `${JSON.stringify(Object.assign({}, basePacket, msg))}\n` +} module.exports = function ping (self) { - return promisify((peerId, cb) => { + return promisify((peerId, count, cb) => { if (!self.isOnline()) { return cb(new Error(OFFLINE_ERROR)) } - var outputStream = new Readable() - outputStream._read = function (size) { - } + const source = new Readable({ + read: function () {} + }) let peer try { peer = self._libp2pNode.peerBook.get(peerId) } catch (err) { + // Conforming with go implemmentation, not sure if makes sense to log this + // since we perform no `findPeer` + source.push(getPacket({Success: true, Text: `Looking up peer ${peerId}`})) peer = new PeerInfo(PeerId.createFromB58String(peerId)) } self._libp2pNode.ping(peer, (err, p) => { - p.once('ping', (time) => { - outputStream.push(JSON.stringify([{}, { Success: true, Time: time }, { Text: 'Average latency: ' + time + ' ms' }])) - outputStream.push(null) - p.stop() - cb(err, outputStream) + let packetCount = 0 + let totalTime = 0 + source.push(getPacket({Success: true, Text: `PING ${peerId}`})) + p.on('ping', (time) => { + source.push(getPacket({ Success: true, Time: time })) + totalTime += time + packetCount++ + if (packetCount >= count) { + const average = totalTime/count + p.stop() + source.push(getPacket({ Success: false, Text: `Average latency: ${average}ms`})) + source.push(null) + } }) }) + cb(null, source) }) } diff --git a/src/http/api/resources/ping.js b/src/http/api/resources/ping.js index b8eaef90f4..50a5a92f29 100644 --- a/src/http/api/resources/ping.js +++ b/src/http/api/resources/ping.js @@ -1,18 +1,36 @@ 'use strict' +const Joi = require('joi') const boom = require('boom') +const toStream = require('pull-stream-to-stream') exports = module.exports -exports.get = (request, reply) => { - const ipfs = request.server.app.ipfs - const peerId = request.query.arg +exports.get = { + validate: { + query: Joi.object().keys({ + n: Joi.alternatives() + .when('count', { + is: true, then: Joi.any().forbidden(), + otherwise: Joi.number().greater(0) + }), + count: Joi.number().greater(0), + arg: Joi.string() + }) + }, + handler: (request, reply) => { + const ipfs = request.server.app.ipfs + const peerId = request.query.arg + // Default count to 10 + const count = request.query.n || request.query.count || 10 - ipfs.ping(peerId, (err, outputStream) => { - if (err) { - return reply(boom.badRequest(err)) - } + ipfs.ping(peerId, count, (err, sourceStream) => { + if (err) { + return reply(boom.badRequest(err)) + } + console.log(sourceStream) - return reply(outputStream).type('application/json').header('x-chunked-output', '1') - }) + return reply(sourceStream).type('application/json').header('x-chunked-output', '1') + }) + } } diff --git a/src/http/api/routes/ping.js b/src/http/api/routes/ping.js index 3d75a8580f..858e0bdca8 100644 --- a/src/http/api/routes/ping.js +++ b/src/http/api/routes/ping.js @@ -13,7 +13,8 @@ module.exports = (server) => { parse: false, output: 'stream' }, - handler: resources.ping.get + handler: resources.ping.get.handler, + validate: resources.ping.get.validate } }) } From 24681c93807de904d68c4404542cc38a1a99ee21 Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Mon, 26 Mar 2018 03:56:58 +0100 Subject: [PATCH 03/12] fix: ETOOMANYSTREAMS :cry: --- src/core/components/ping.js | 33 ++++++++++++++++++++++++++------- src/http/api/resources/ping.js | 15 +++++++++++---- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/src/core/components/ping.js b/src/core/components/ping.js index 61e641b3f6..189aae6595 100644 --- a/src/core/components/ping.js +++ b/src/core/components/ping.js @@ -4,13 +4,15 @@ const promisify = require('promisify-es6') const OFFLINE_ERROR = require('../utils').OFFLINE_ERROR const PeerId = require('peer-id') const PeerInfo = require('peer-info') -const Readable = require('readable-stream').Readable +const pull = require('pull-stream/pull') +const take = require('pull-stream/throughs/take') +const Pushable = require('pull-pushable') +const ndjson = require('pull-ndjson') function getPacket (msg) { // Default msg const basePacket = {Success: false, Time: 0, Text: ''} - // ndjson - return `${JSON.stringify(Object.assign({}, basePacket, msg))}\n` + return Object.assign({}, basePacket, msg) } module.exports = function ping (self) { @@ -19,10 +21,15 @@ module.exports = function ping (self) { return cb(new Error(OFFLINE_ERROR)) } - const source = new Readable({ - read: function () {} + const source = Pushable(function (err) { + console.log('stream closed!', err) }) + const response = pull( + source, + ndjson.serialize() + ) + let peer try { peer = self._libp2pNode.peerBook.get(peerId) @@ -34,21 +41,33 @@ module.exports = function ping (self) { } self._libp2pNode.ping(peer, (err, p) => { + if (err) { + console.log('ERROR', err) + return source.abort(err) + } let packetCount = 0 let totalTime = 0 source.push(getPacket({Success: true, Text: `PING ${peerId}`})) p.on('ping', (time) => { + console.log('ON PING') source.push(getPacket({ Success: true, Time: time })) totalTime += time packetCount++ + console.log(packetCount, count) if (packetCount >= count) { const average = totalTime/count p.stop() source.push(getPacket({ Success: false, Text: `Average latency: ${average}ms`})) - source.push(null) + source.end() } }) + console.log('Setup handler') + p.on('error', (err) => { + console.log('ERROR BATATA', err) + source.abort(err) + }) }) - cb(null, source) + + cb(null, response) }) } diff --git a/src/http/api/resources/ping.js b/src/http/api/resources/ping.js index 50a5a92f29..ef38af0e1e 100644 --- a/src/http/api/resources/ping.js +++ b/src/http/api/resources/ping.js @@ -3,6 +3,8 @@ const Joi = require('joi') const boom = require('boom') const toStream = require('pull-stream-to-stream') +const PassThrough = require('readable-stream').PassThrough +const pump = require('pump') exports = module.exports @@ -24,13 +26,18 @@ exports.get = { // Default count to 10 const count = request.query.n || request.query.count || 10 - ipfs.ping(peerId, count, (err, sourceStream) => { + ipfs.ping(peerId, count, (err, pullStream) => { if (err) { return reply(boom.badRequest(err)) } - console.log(sourceStream) - - return reply(sourceStream).type('application/json').header('x-chunked-output', '1') + // Streams from pull-stream-to-stream don't seem to be compatible + // with the stream2 readable interface + // see: https://github.com/hapijs/hapi/blob/c23070a3de1b328876d5e64e679a147fafb04b38/lib/response.js#L533 + // and: https://github.com/pull-stream/pull-stream-to-stream/blob/e436acee18b71af8e71d1b5d32eee642351517c7/index.js#L28 + const responseStream = toStream.source(pullStream) + const stream2 = new PassThrough() + pump(responseStream, stream2) + return reply(stream2).type('application/json').header('x-chunked-output', '1') }) } } From 1242317f7442c6ded10968dce803a0b3b26390be Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Wed, 4 Apr 2018 00:40:51 +0100 Subject: [PATCH 04/12] chore: cleanup on the ping component --- src/core/components/ping.js | 30 ++++++++++++++++++------------ src/http/api/resources/ping.js | 2 +- src/http/api/routes/ping.js | 4 ---- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/core/components/ping.js b/src/core/components/ping.js index 189aae6595..7e5f1a32e0 100644 --- a/src/core/components/ping.js +++ b/src/core/components/ping.js @@ -1,14 +1,17 @@ 'use strict' const promisify = require('promisify-es6') +const debug = require('debug') const OFFLINE_ERROR = require('../utils').OFFLINE_ERROR const PeerId = require('peer-id') const PeerInfo = require('peer-info') const pull = require('pull-stream/pull') -const take = require('pull-stream/throughs/take') const Pushable = require('pull-pushable') const ndjson = require('pull-ndjson') +const log = debug('jsipfs:ping') +log.error = debug('jsipfs:ping:error') + function getPacket (msg) { // Default msg const basePacket = {Success: false, Time: 0, Text: ''} @@ -21,9 +24,7 @@ module.exports = function ping (self) { return cb(new Error(OFFLINE_ERROR)) } - const source = Pushable(function (err) { - console.log('stream closed!', err) - }) + const source = Pushable() const response = pull( source, @@ -42,30 +43,35 @@ module.exports = function ping (self) { self._libp2pNode.ping(peer, (err, p) => { if (err) { - console.log('ERROR', err) - return source.abort(err) + log.error(err) + source.push(getPacket({Text: err.toString()})) + return source.end(err) } + let packetCount = 0 let totalTime = 0 source.push(getPacket({Success: true, Text: `PING ${peerId}`})) + p.on('ping', (time) => { - console.log('ON PING') source.push(getPacket({ Success: true, Time: time })) totalTime += time packetCount++ - console.log(packetCount, count) if (packetCount >= count) { const average = totalTime/count p.stop() - source.push(getPacket({ Success: false, Text: `Average latency: ${average}ms`})) + source.push(getPacket({ Success: true, Text: `Average latency: ${average}ms` })) source.end() } }) - console.log('Setup handler') + p.on('error', (err) => { - console.log('ERROR BATATA', err) - source.abort(err) + log.error(err) + p.stop() + source.push(getPacket({Text: err.toString()})) + source.end(err) }) + + p.start() }) cb(null, response) diff --git a/src/http/api/resources/ping.js b/src/http/api/resources/ping.js index ef38af0e1e..b26588de66 100644 --- a/src/http/api/resources/ping.js +++ b/src/http/api/resources/ping.js @@ -37,7 +37,7 @@ exports.get = { const responseStream = toStream.source(pullStream) const stream2 = new PassThrough() pump(responseStream, stream2) - return reply(stream2).type('application/json').header('x-chunked-output', '1') + return reply(stream2).type('application/json').header('X-Chunked-Output', '1') }) } } diff --git a/src/http/api/routes/ping.js b/src/http/api/routes/ping.js index 858e0bdca8..92b081862f 100644 --- a/src/http/api/routes/ping.js +++ b/src/http/api/routes/ping.js @@ -9,10 +9,6 @@ module.exports = (server) => { method: '*', path: '/api/v0/ping', config: { - payload: { - parse: false, - output: 'stream' - }, handler: resources.ping.get.handler, validate: resources.ping.get.validate } From bf608db5cda3568f2838d0d3fac6ba8e96a53a8b Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Wed, 4 Apr 2018 00:47:30 +0100 Subject: [PATCH 05/12] chore: ping component linting --- src/cli/commands/ping.js | 2 +- src/core/components/ping.js | 6 +++--- src/http/api/resources/ping.js | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/cli/commands/ping.js b/src/cli/commands/ping.js index 53638a0423..7513c7c314 100644 --- a/src/cli/commands/ping.js +++ b/src/cli/commands/ping.js @@ -26,7 +26,7 @@ module.exports = { pull( argv.ipfs.pingPullStream(peerId, { count }), pullCatch(err => { - throw err + throw err }), drain(({ Time, Text }) => { // Check if it's a pong diff --git a/src/core/components/ping.js b/src/core/components/ping.js index 7e5f1a32e0..c18def3ea9 100644 --- a/src/core/components/ping.js +++ b/src/core/components/ping.js @@ -48,7 +48,7 @@ module.exports = function ping (self) { return source.end(err) } - let packetCount = 0 + let packetCount = 0 let totalTime = 0 source.push(getPacket({Success: true, Text: `PING ${peerId}`})) @@ -57,7 +57,7 @@ module.exports = function ping (self) { totalTime += time packetCount++ if (packetCount >= count) { - const average = totalTime/count + const average = totalTime / count p.stop() source.push(getPacket({ Success: true, Text: `Average latency: ${average}ms` })) source.end() @@ -73,7 +73,7 @@ module.exports = function ping (self) { p.start() }) - + cb(null, response) }) } diff --git a/src/http/api/resources/ping.js b/src/http/api/resources/ping.js index b26588de66..762c3ebe03 100644 --- a/src/http/api/resources/ping.js +++ b/src/http/api/resources/ping.js @@ -13,7 +13,8 @@ exports.get = { query: Joi.object().keys({ n: Joi.alternatives() .when('count', { - is: true, then: Joi.any().forbidden(), + is: true, + then: Joi.any().forbidden(), otherwise: Joi.number().greater(0) }), count: Joi.number().greater(0), From ecb1a1e3f6ae10f5e717198ad2edd32efe972339 Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Thu, 5 Apr 2018 02:43:09 +0100 Subject: [PATCH 06/12] chore: bump js-ipfs-api and fix http ping validation --- src/cli/commands/ping.js | 1 - src/http/api/resources/ping.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cli/commands/ping.js b/src/cli/commands/ping.js index 7513c7c314..26416b34ca 100644 --- a/src/cli/commands/ping.js +++ b/src/cli/commands/ping.js @@ -22,7 +22,6 @@ module.exports = { handler (argv) { const peerId = argv.peerId const count = argv.count || 10 - pull( argv.ipfs.pingPullStream(peerId, { count }), pullCatch(err => { diff --git a/src/http/api/resources/ping.js b/src/http/api/resources/ping.js index 762c3ebe03..77a75003f4 100644 --- a/src/http/api/resources/ping.js +++ b/src/http/api/resources/ping.js @@ -19,7 +19,7 @@ exports.get = { }), count: Joi.number().greater(0), arg: Joi.string() - }) + }).unknown() }, handler: (request, reply) => { const ipfs = request.server.app.ipfs From 30536af37d2db67059fdf2dafb025d5426c91645 Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Fri, 6 Apr 2018 03:36:32 +0100 Subject: [PATCH 07/12] chore: add test to ping cli command --- test/cli/ping.js | 99 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 test/cli/ping.js diff --git a/test/cli/ping.js b/test/cli/ping.js new file mode 100644 index 0000000000..c5c53e2b22 --- /dev/null +++ b/test/cli/ping.js @@ -0,0 +1,99 @@ +/* eslint max-nested-callbacks: ["error", 8] */ +/* eslint-env mocha */ +'use strict' + +const chai = require('chai') +const dirtyChai = require('dirty-chai') +const expect = chai.expect +chai.use(dirtyChai) +const delay = require('delay') +const series = require('async/series') +const ipfsExec = require('../utils/ipfs-exec') + +const DaemonFactory = require('ipfsd-ctl') +const df = DaemonFactory.create({ type: 'js' }) + +const config = { + Bootstrap: [], + Discovery: { + MDNS: { + Enabled: + false + } + } +} + +describe.only('ping', function () { + this.timeout(80 * 1000) + + let ipfsdA + let ipfsdB + let ipfsdBId + let cli + + before(function (done) { + this.timeout(60 * 1000) + + df.spawn({ + exec: './src/cli/bin.js', + config, + initoptions: { bits: 512 } + }, (err, _ipfsd) => { + expect(err).to.not.exist() + ipfsdA = _ipfsd + done() + }) + }) + + before((done) => { + this.timeout(60 * 1000) + series([ + (cb) => { + df.spawn({ + exec: `./src/cli/bin.js`, + config, + initOptions: { bits: 512 } + }, (err, _ipfsd) => { + expect(err).to.not.exist() + ipfsdB = _ipfsd + cb() + }) + }, + (cb) => { + ipfsdB.api.id((err, peerInfo) => { + expect(err).to.not.exist() + console.log(peerInfo) + ipfsdBId = peerInfo.id + cb() + }) + } + ], done) + }) + + after((done) => ipfsdA.stop(done)) + after((done) => ipfsdB.stop(done)) + + before((done) => { + cli = ipfsExec(ipfsdA.repoPath) + done() + }) + + it('ping host', (done) => { + const ping = cli(`ping ${ipfsdBId}`) + const result = [] + ping.stdout.on('data', (packet) => { + console.log('ON DATA') + result.push(packet.toString()) + }) + + ping.stdout.on('end', (c) => { + console.log('END', result) + done() + }) + + ping.catch((err) => { + expect(err).to.not.exist() + done() + }) + }) +}) From ca0438a392988fcf54d32c6c59f522bef2e98c2a Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Sun, 8 Apr 2018 21:28:08 +0100 Subject: [PATCH 08/12] chore: add ping cli test --- test/cli/commands.js | 2 +- test/cli/ping.js | 98 +++++++++++++++++++++++++++++++++----------- 2 files changed, 75 insertions(+), 25 deletions(-) diff --git a/test/cli/commands.js b/test/cli/commands.js index 6886dac40a..1e194eb143 100644 --- a/test/cli/commands.js +++ b/test/cli/commands.js @@ -14,7 +14,7 @@ describe('commands', () => runOnAndOff((thing) => { ipfs = thing.ipfs }) - it.only('list the commands', () => { + it('list the commands', () => { return ipfs('commands').then((out) => { expect(out.split('\n')).to.have.length(commandCount + 1) }) diff --git a/test/cli/ping.js b/test/cli/ping.js index c5c53e2b22..d991b6a3b3 100644 --- a/test/cli/ping.js +++ b/test/cli/ping.js @@ -24,27 +24,13 @@ const config = { } describe.only('ping', function () { - this.timeout(80 * 1000) - + this.timeout(60 * 1000) let ipfsdA let ipfsdB + let bMultiaddr let ipfsdBId let cli - before(function (done) { - this.timeout(60 * 1000) - - df.spawn({ - exec: './src/cli/bin.js', - config, - initoptions: { bits: 512 } - }, (err, _ipfsd) => { - expect(err).to.not.exist() - ipfsdA = _ipfsd - done() - }) - }) - before((done) => { this.timeout(60 * 1000) series([ @@ -62,38 +48,102 @@ describe.only('ping', function () { (cb) => { ipfsdB.api.id((err, peerInfo) => { expect(err).to.not.exist() - console.log(peerInfo) ipfsdBId = peerInfo.id + bMultiaddr = peerInfo.addresses[0] cb() }) } ], done) }) - after((done) => ipfsdA.stop(done)) - after((done) => ipfsdB.stop(done)) + before(function (done) { + this.timeout(60 * 1000) + + df.spawn({ + exec: './src/cli/bin.js', + config, + initoptions: { bits: 512 } + }, (err, _ipfsd) => { + expect(err).to.not.exist() + ipfsdA = _ipfsd + ipfsdA.api.swarm.connect(bMultiaddr, done) + }) + }) before((done) => { + this.timeout(60 * 1000) cli = ipfsExec(ipfsdA.repoPath) done() }) + after((done) => ipfsdA.stop(done)) + after((done) => ipfsdB.stop(done)) + it('ping host', (done) => { + this.timeout(60 * 1000) const ping = cli(`ping ${ipfsdBId}`) const result = [] - ping.stdout.on('data', (packet) => { - console.log('ON DATA') - result.push(packet.toString()) + ping.stdout.on('data', (output) => { + const packets = output.toString().split('\n').slice(0, -1) + result.push(...packets) }) - ping.stdout.on('end', (c) => { - console.log('END', result) + ping.stdout.on('end', () => { + expect(result).to.have.lengthOf(12) + expect(result[0]).to.equal(`PING ${ipfsdBId}`) + for(let i = 1; i < 11; i++) { + expect(result[i]).to.match(/^Pong received: time=\d+ ms$/) + } + expect(result[11]).to.match(/^Average latency: \d+(.\d+)?ms$/) done() }) ping.catch((err) => { expect(err).to.not.exist() + }) + }) + + it('ping host with --n option', (done) => { + this.timeout(60 * 1000) + const ping = cli(`ping --n 1 ${ipfsdBId}`) + const result = [] + ping.stdout.on('data', (output) => { + const packets = output.toString().split('\n').slice(0, -1) + result.push(...packets) + }) + + ping.stdout.on('end', () => { + expect(result).to.have.lengthOf(3) + expect(result[0]).to.equal(`PING ${ipfsdBId}`) + expect(result[1]).to.match(/^Pong received: time=\d+ ms$/) + expect(result[2]).to.match(/^Average latency: \d+(.\d+)?ms$/) done() }) + + ping.catch((err) => { + expect(err).to.not.exist() + }) + }) + + it('ping host with --count option', (done) => { + this.timeout(60 * 1000) + const ping = cli(`ping --count 1 ${ipfsdBId}`) + const result = [] + ping.stdout.on('data', (output) => { + const packets = output.toString().split('\n').slice(0, -1) + result.push(...packets) + }) + + ping.stdout.on('end', () => { + expect(result).to.have.lengthOf(3) + expect(result[0]).to.equal(`PING ${ipfsdBId}`) + expect(result[1]).to.match(/^Pong received: time=\d+ ms$/) + expect(result[2]).to.match(/^Average latency: \d+(.\d+)?ms$/) + done() + }) + + ping.catch((err) => { + expect(err).to.not.exist() + }) }) }) From 89c61e33cad8f6b6e4915b856bb13886a9dd0ed7 Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Sun, 6 May 2018 18:20:17 +0100 Subject: [PATCH 09/12] chore: refactor ping component and some cleanup --- src/cli/commands/ping.js | 2 +- src/core/components/ping.js | 105 +++++++++++++++++++++--------------- test/cli/ping.js | 2 +- 3 files changed, 63 insertions(+), 46 deletions(-) diff --git a/src/cli/commands/ping.js b/src/cli/commands/ping.js index 26416b34ca..ab9d73666e 100644 --- a/src/cli/commands/ping.js +++ b/src/cli/commands/ping.js @@ -9,7 +9,7 @@ const print = require('../utils').print module.exports = { command: 'ping ', - describe: 'Measure the latency of a connection', + description: 'Measure the latency of a connection', builder: { count: { diff --git a/src/core/components/ping.js b/src/core/components/ping.js index c18def3ea9..11a3d60cb2 100644 --- a/src/core/components/ping.js +++ b/src/core/components/ping.js @@ -3,21 +3,14 @@ const promisify = require('promisify-es6') const debug = require('debug') const OFFLINE_ERROR = require('../utils').OFFLINE_ERROR -const PeerId = require('peer-id') -const PeerInfo = require('peer-info') const pull = require('pull-stream/pull') const Pushable = require('pull-pushable') const ndjson = require('pull-ndjson') +const waterfall = require('async/waterfall') const log = debug('jsipfs:ping') log.error = debug('jsipfs:ping:error') -function getPacket (msg) { - // Default msg - const basePacket = {Success: false, Time: 0, Text: ''} - return Object.assign({}, basePacket, msg) -} - module.exports = function ping (self) { return promisify((peerId, count, cb) => { if (!self.isOnline()) { @@ -31,49 +24,73 @@ module.exports = function ping (self) { ndjson.serialize() ) - let peer - try { - peer = self._libp2pNode.peerBook.get(peerId) - } catch (err) { - // Conforming with go implemmentation, not sure if makes sense to log this - // since we perform no `findPeer` - source.push(getPacket({Success: true, Text: `Looking up peer ${peerId}`})) - peer = new PeerInfo(PeerId.createFromB58String(peerId)) - } + waterfall([ + getPeer.bind(null, self._libp2pNode, source, peerId), + runPing.bind(null, self._libp2pNode, source, count) + ], (err) => { + log.error(err) + source.push(getPacket({Text: err.toString()})) + return source.end(err) + }) - self._libp2pNode.ping(peer, (err, p) => { - if (err) { - log.error(err) - source.push(getPacket({Text: err.toString()})) - return source.end(err) - } + cb(null, response) + }) +} + +function getPacket (msg) { + // Default msg + const basePacket = {Success: false, Time: 0, Text: ''} + return Object.assign({}, basePacket, msg) +} - let packetCount = 0 - let totalTime = 0 - source.push(getPacket({Success: true, Text: `PING ${peerId}`})) +function getPeer (libp2pNode, statusStream, peerId, cb) { + let peer + try { + peer = libp2pNode.peerBook.get(peerId) + console.log(peer) + return cb(null, peer) + } catch (err) { + // Check if we have support for peerRouting + if (!libp2pNode.peerRouting) { + return cb(new Error('Peer not found in peer book and no peer routing mechanism enabled')) + } + // Share lookup status just as in the go implemmentation + statusStream.push(getPacket({Success: true, Text: `Looking up peer ${peerId}`})) + libp2pNode.peerRouting.findPeer(peerId, cb) + } +} - p.on('ping', (time) => { - source.push(getPacket({ Success: true, Time: time })) - totalTime += time - packetCount++ - if (packetCount >= count) { - const average = totalTime / count - p.stop() - source.push(getPacket({ Success: true, Text: `Average latency: ${average}ms` })) - source.end() - } - }) +function runPing (libp2pNode, statusStream, count, peer, cb) { + libp2pNode.ping(peer, (err, p) => { + if (err) { + return cb(err) + } + + let packetCount = 0 + let totalTime = 0 + statusStream.push(getPacket({Success: true, Text: `PING ${peer.id.toB58String()}`})) - p.on('error', (err) => { - log.error(err) + p.on('ping', (time) => { + statusStream.push(getPacket({ Success: true, Time: time })) + totalTime += time + packetCount++ + if (packetCount >= count) { + const average = totalTime / count p.stop() - source.push(getPacket({Text: err.toString()})) - source.end(err) - }) + statusStream.push(getPacket({ Success: true, Text: `Average latency: ${average}ms` })) + statusStream.end() + } + }) - p.start() + p.on('error', (err) => { + log.error(err) + p.stop() + statusStream.push(getPacket({Text: err.toString()})) + statusStream.end(err) }) - cb(null, response) + p.start() + + return cb() }) } diff --git a/test/cli/ping.js b/test/cli/ping.js index d991b6a3b3..f52c3c29c1 100644 --- a/test/cli/ping.js +++ b/test/cli/ping.js @@ -23,7 +23,7 @@ const config = { } } -describe.only('ping', function () { +describe('ping', function () { this.timeout(60 * 1000) let ipfsdA let ipfsdB From fc06d599e5581fbf37c26e4933cac5ac99296b5e Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Sun, 6 May 2018 20:06:36 +0100 Subject: [PATCH 10/12] chore: add tests to the ping http API --- src/http/api/resources/ping.js | 5 ++-- test/cli/ping.js | 10 +++---- test/http-api/inject/ping.js | 52 ++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 test/http-api/inject/ping.js diff --git a/src/http/api/resources/ping.js b/src/http/api/resources/ping.js index 77a75003f4..35c65ad818 100644 --- a/src/http/api/resources/ping.js +++ b/src/http/api/resources/ping.js @@ -13,12 +13,12 @@ exports.get = { query: Joi.object().keys({ n: Joi.alternatives() .when('count', { - is: true, + is: Joi.any().exist(), then: Joi.any().forbidden(), otherwise: Joi.number().greater(0) }), count: Joi.number().greater(0), - arg: Joi.string() + arg: Joi.string().required() }).unknown() }, handler: (request, reply) => { @@ -26,7 +26,6 @@ exports.get = { const peerId = request.query.arg // Default count to 10 const count = request.query.n || request.query.count || 10 - ipfs.ping(peerId, count, (err, pullStream) => { if (err) { return reply(boom.badRequest(err)) diff --git a/test/cli/ping.js b/test/cli/ping.js index f52c3c29c1..a571b2e380 100644 --- a/test/cli/ping.js +++ b/test/cli/ping.js @@ -4,14 +4,13 @@ const chai = require('chai') const dirtyChai = require('dirty-chai') -const expect = chai.expect -chai.use(dirtyChai) -const delay = require('delay') const series = require('async/series') +const DaemonFactory = require('ipfsd-ctl') const ipfsExec = require('../utils/ipfs-exec') -const DaemonFactory = require('ipfsd-ctl') const df = DaemonFactory.create({ type: 'js' }) +const expect = chai.expect +chai.use(dirtyChai) const config = { Bootstrap: [], @@ -66,6 +65,7 @@ describe('ping', function () { }, (err, _ipfsd) => { expect(err).to.not.exist() ipfsdA = _ipfsd + // Without DHT we need to have an already established connection ipfsdA.api.swarm.connect(bMultiaddr, done) }) }) @@ -91,7 +91,7 @@ describe('ping', function () { ping.stdout.on('end', () => { expect(result).to.have.lengthOf(12) expect(result[0]).to.equal(`PING ${ipfsdBId}`) - for(let i = 1; i < 11; i++) { + for (let i = 1; i < 11; i++) { expect(result[i]).to.match(/^Pong received: time=\d+ ms$/) } expect(result[11]).to.match(/^Average latency: \d+(.\d+)?ms$/) diff --git a/test/http-api/inject/ping.js b/test/http-api/inject/ping.js new file mode 100644 index 0000000000..8cda3836c3 --- /dev/null +++ b/test/http-api/inject/ping.js @@ -0,0 +1,52 @@ +/* eslint max-nested-callbacks: ["error", 8] */ +/* eslint-env mocha */ +'use strict' + +const chai = require('chai') +const dirtyChai = require('dirty-chai') + +const expect = chai.expect +chai.use(dirtyChai) + +module.exports = (http) => { + describe('/ping', function () { + let api + + before(() => { + api = http.api.server.select('API') + }) + + it('returns 400 if both n and count are provided', (done) => { + api.inject({ + method: 'GET', + url: `/api/v0/ping?arg=someRandomId&n=1&count=1` + }, (res) => { + expect(res.statusCode).to.equal(400) + done() + }) + }) + + it('returns 400 if arg is not provided', (done) => { + api.inject({ + method: 'GET', + url: `/api/v0/ping?count=1` + }, (res) => { + expect(res.statusCode).to.equal(400) + done() + }) + }) + + it('returns 200 and the response stream with the ping result', (done) => { + api.inject({ + method: 'GET', + url: `/api/v0/ping?arg=someRandomId` + }, (res) => { + expect(res.statusCode).to.equal(200) + expect(res.headers['x-chunked-output']).to.equal('1') + expect(res.headers['transfer-encoding']).to.equal('chunked') + expect(res.headers['content-type']).to.include('application/json') + done() + }) + }) + }) +} From ffe32180b006193154d862d89f51febdd877dfb5 Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Sun, 6 May 2018 20:33:26 +0100 Subject: [PATCH 11/12] fix: no need to check for peerRouting method in ping --- src/core/components/ping.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/core/components/ping.js b/src/core/components/ping.js index 11a3d60cb2..ac7506379b 100644 --- a/src/core/components/ping.js +++ b/src/core/components/ping.js @@ -50,12 +50,10 @@ function getPeer (libp2pNode, statusStream, peerId, cb) { console.log(peer) return cb(null, peer) } catch (err) { - // Check if we have support for peerRouting - if (!libp2pNode.peerRouting) { - return cb(new Error('Peer not found in peer book and no peer routing mechanism enabled')) - } + log('Peer not found in peer book, trying peer routing') // Share lookup status just as in the go implemmentation statusStream.push(getPacket({Success: true, Text: `Looking up peer ${peerId}`})) + // Try to use peerRouting libp2pNode.peerRouting.findPeer(peerId, cb) } } From a4c7fb816de5e84a276f0cb752aa4cf720704ccd Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Tue, 8 May 2018 04:25:49 +0100 Subject: [PATCH 12/12] chore: add tests for ping core functionality --- src/core/components/ping.js | 14 ++- src/http/api/resources/ping.js | 9 +- test/core/ping.spec.js | 220 +++++++++++++++++++++++++++++++++ 3 files changed, 230 insertions(+), 13 deletions(-) create mode 100644 test/core/ping.spec.js diff --git a/src/core/components/ping.js b/src/core/components/ping.js index ac7506379b..6804acf83d 100644 --- a/src/core/components/ping.js +++ b/src/core/components/ping.js @@ -3,6 +3,7 @@ const promisify = require('promisify-es6') const debug = require('debug') const OFFLINE_ERROR = require('../utils').OFFLINE_ERROR +const PeerId = require('peer-id') const pull = require('pull-stream/pull') const Pushable = require('pull-pushable') const ndjson = require('pull-ndjson') @@ -23,14 +24,15 @@ module.exports = function ping (self) { source, ndjson.serialize() ) - waterfall([ getPeer.bind(null, self._libp2pNode, source, peerId), runPing.bind(null, self._libp2pNode, source, count) ], (err) => { - log.error(err) - source.push(getPacket({Text: err.toString()})) - return source.end(err) + if (err) { + log.error(err) + source.push(getPacket({Text: err.toString()})) + source.end(err) + } }) cb(null, response) @@ -47,19 +49,19 @@ function getPeer (libp2pNode, statusStream, peerId, cb) { let peer try { peer = libp2pNode.peerBook.get(peerId) - console.log(peer) return cb(null, peer) } catch (err) { log('Peer not found in peer book, trying peer routing') // Share lookup status just as in the go implemmentation statusStream.push(getPacket({Success: true, Text: `Looking up peer ${peerId}`})) // Try to use peerRouting - libp2pNode.peerRouting.findPeer(peerId, cb) + libp2pNode.peerRouting.findPeer(PeerId.createFromB58String(peerId), cb) } } function runPing (libp2pNode, statusStream, count, peer, cb) { libp2pNode.ping(peer, (err, p) => { + log('Got peer', peer) if (err) { return cb(err) } diff --git a/src/http/api/resources/ping.js b/src/http/api/resources/ping.js index 35c65ad818..3bc0b74b48 100644 --- a/src/http/api/resources/ping.js +++ b/src/http/api/resources/ping.js @@ -11,15 +11,10 @@ exports = module.exports exports.get = { validate: { query: Joi.object().keys({ - n: Joi.alternatives() - .when('count', { - is: Joi.any().exist(), - then: Joi.any().forbidden(), - otherwise: Joi.number().greater(0) - }), + n: Joi.number().greater(0), count: Joi.number().greater(0), arg: Joi.string().required() - }).unknown() + }).xor('n', 'count').unknown() }, handler: (request, reply) => { const ipfs = request.server.app.ipfs diff --git a/test/core/ping.spec.js b/test/core/ping.spec.js new file mode 100644 index 0000000000..b94299ba80 --- /dev/null +++ b/test/core/ping.spec.js @@ -0,0 +1,220 @@ +/* eslint-env mocha */ +'use strict' + +const chai = require('chai') +const dirtyChai = require('dirty-chai') +const pull = require('pull-stream/pull') +const drain = require('pull-stream/sinks/drain') +const pullCatch = require('pull-catch') +const parallel = require('async/parallel') +const DaemonFactory = require('ipfsd-ctl') +const isNode = require('detect-node') + +const expect = chai.expect +chai.use(dirtyChai) +const df = DaemonFactory.create({ exec: 'src/cli/bin.js' }) + +const config = { + Bootstrap: [], + Discovery: { + MDNS: { + Enabled: + false + } + } +} + +function spawnNode ({ dht = false }, cb) { + const args = dht ? ['--enable-dht-experiment'] : [] + df.spawn({ + args, + config, + initOptions: { bits: 512 } + }, cb) +} + +describe('ping', function () { + this.timeout(60 * 1000) + + if (!isNode) return + + describe('DHT disabled', function () { + // Without DHT nodes need to be previously connected + let ipfsdA + let ipfsdB + let bMultiaddr + let ipfsdBId + + // Spawn nodes + before(function (done) { + this.timeout(60 * 1000) + + parallel([ + spawnNode.bind(null, { dht: false }), + spawnNode.bind(null, { dht: false }) + ], (err, ipfsd) => { + expect(err).to.not.exist() + ipfsdA = ipfsd[0] + ipfsdB = ipfsd[1] + done() + }) + }) + + // Get the peer info object + before(function (done) { + this.timeout(60 * 1000) + + ipfsdB.api.id((err, peerInfo) => { + expect(err).to.not.exist() + ipfsdBId = peerInfo.id + bMultiaddr = peerInfo.addresses[0] + done() + }) + }) + + // Connect the nodes + before(function (done) { + this.timeout(60 * 1000) + ipfsdA.api.swarm.connect(bMultiaddr, done) + }) + + after((done) => ipfsdA.stop(done)) + after((done) => ipfsdB.stop(done)) + + it('sends the specified number of packets', (done) => { + let packetNum = 0 + const count = 3 + pull( + ipfsdA.api.pingPullStream(ipfsdBId, { count }), + pullCatch(err => { + expect(err).to.not.exist() + }), + drain(({ Success, Time, Text }) => { + expect(Success).to.be.true() + // It's a pong + if (Time) { + packetNum++ + } + }, () => { + expect(packetNum).to.equal(count) + done() + }) + ) + }) + + it('pinging an unknown peer will fail accordingly', (done) => { + let messageNum = 0 + const count = 2 + pull( + ipfsdA.api.pingPullStream('unknown', { count }), + pullCatch(err => { + expect(err).to.not.exist() + }), + drain(({ Success, Time, Text }) => { + messageNum++ + // Assert that the ping command falls back to the peerRouting + if (messageNum === 1) { + expect(Text).to.include('Looking up') + } + + // Fails accordingly while trying to use peerRouting + if (messageNum === 2) { + expect(Success).to.be.false() + } + }, () => { + expect(messageNum).to.equal(count) + done() + }) + ) + }) + }) + + describe('DHT enabled', function () { + // Our bootstrap process will run 3 IPFS daemons where + // A ----> B ----> C + // Allowing us to test the ping command using the DHT peer routing + let ipfsdA + let ipfsdB + let ipfsdC + let bMultiaddr + let cMultiaddr + let ipfsdCId + + // Spawn nodes + before(function (done) { + this.timeout(60 * 1000) + + parallel([ + spawnNode.bind(null, { dht: true }), + spawnNode.bind(null, { dht: true }), + spawnNode.bind(null, { dht: true }) + ], (err, ipfsd) => { + expect(err).to.not.exist() + ipfsdA = ipfsd[0] + ipfsdB = ipfsd[1] + ipfsdC = ipfsd[2] + done() + }) + }) + + // Get the peer info objects + before(function (done) { + this.timeout(60 * 1000) + + parallel([ + ipfsdB.api.id.bind(ipfsdB.api), + ipfsdC.api.id.bind(ipfsdC.api) + ], (err, peerInfo) => { + expect(err).to.not.exist() + bMultiaddr = peerInfo[0].addresses[0] + ipfsdCId = peerInfo[1].id + cMultiaddr = peerInfo[1].addresses[0] + done() + }) + }) + + // Connect the nodes + before(function (done) { + this.timeout(60 * 1000) + + parallel([ + ipfsdA.api.swarm.connect.bind(ipfsdA.api, bMultiaddr), + ipfsdB.api.swarm.connect.bind(ipfsdB.api, cMultiaddr) + ], done) + }) + + // FIXME timeout needed for connections to succeed + before((done) => setTimeout(done, 100)) + + after((done) => ipfsdA.stop(done)) + after((done) => ipfsdB.stop(done)) + after((done) => ipfsdC.stop(done)) + + it('if enabled uses the DHT peer routing to find peer', (done) => { + let messageNum = 0 + let packetNum = 0 + const count = 3 + pull( + ipfsdA.api.pingPullStream(ipfsdCId, { count }), + pullCatch(err => { + expect(err).to.not.exist() + }), + drain(({ Success, Time, Text }) => { + messageNum++ + expect(Success).to.be.true() + // Assert that the ping command falls back to the peerRouting + if (messageNum === 1) { + expect(Text).to.include('Looking up') + } + // It's a pong + if (Time) { + packetNum++ + } + }, () => { + expect(packetNum).to.equal(count) + done() + }) + ) + }) + }) +})