From a181932ee0358e17d072584b4aa1cb27f537d03f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Antunes?= Date: Tue, 8 May 2018 07:59:01 +0100 Subject: [PATCH 01/18] feat: ping (#1299) * feat: implement `ipfs ping` flags #928 * feat: first shot at ping implementaion * fix: ETOOMANYSTREAMS :cry: * chore: cleanup on the ping component * chore: ping component linting * chore: bump js-ipfs-api and fix http ping validation * chore: add test to ping cli command * chore: add ping cli test * chore: refactor ping component and some cleanup * chore: add tests to the ping http API * fix: no need to check for peerRouting method in ping * chore: add tests for ping core functionality --- package.json | 1 + src/cli/commands/ping.js | 41 ++++++ src/core/components/ping.js | 91 ++++++++++++- src/http/api/resources/index.js | 1 + src/http/api/resources/ping.js | 38 ++++++ src/http/api/routes/index.js | 1 + src/http/api/routes/ping.js | 16 +++ test/cli/commands.js | 3 +- test/cli/ping.js | 149 +++++++++++++++++++++ test/core/ping.spec.js | 220 ++++++++++++++++++++++++++++++++ test/http-api/inject/ping.js | 52 ++++++++ 11 files changed, 610 insertions(+), 3 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 create mode 100644 test/cli/ping.js create mode 100644 test/core/ping.spec.js create mode 100644 test/http-api/inject/ping.js diff --git a/package.json b/package.json index 90b39928b7..898324a87d 100644 --- a/package.json +++ b/package.json @@ -154,6 +154,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 new file mode 100644 index 0000000000..ab9d73666e --- /dev/null +++ b/src/cli/commands/ping.js @@ -0,0 +1,41 @@ +'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 = { + command: 'ping ', + + description: '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 + 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 eb7a45b34f..6804acf83d 100644 --- a/src/core/components/ping.js +++ b/src/core/components/ping.js @@ -1,9 +1,96 @@ 'use strict' 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') +const waterfall = require('async/waterfall') + +const log = debug('jsipfs:ping') +log.error = debug('jsipfs:ping:error') module.exports = function ping (self) { - return promisify((callback) => { - callback(new Error('Not implemented')) + return promisify((peerId, count, cb) => { + if (!self.isOnline()) { + return cb(new Error(OFFLINE_ERROR)) + } + + const source = Pushable() + + const response = pull( + source, + ndjson.serialize() + ) + waterfall([ + getPeer.bind(null, self._libp2pNode, source, peerId), + runPing.bind(null, self._libp2pNode, source, count) + ], (err) => { + if (err) { + log.error(err) + source.push(getPacket({Text: err.toString()})) + source.end(err) + } + }) + + cb(null, response) + }) +} + +function getPacket (msg) { + // Default msg + const basePacket = {Success: false, Time: 0, Text: ''} + return Object.assign({}, basePacket, msg) +} + +function getPeer (libp2pNode, statusStream, peerId, cb) { + let peer + try { + peer = libp2pNode.peerBook.get(peerId) + 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.createFromB58String(peerId), cb) + } +} + +function runPing (libp2pNode, statusStream, count, peer, cb) { + libp2pNode.ping(peer, (err, p) => { + log('Got peer', peer) + if (err) { + return cb(err) + } + + let packetCount = 0 + let totalTime = 0 + statusStream.push(getPacket({Success: true, Text: `PING ${peer.id.toB58String()}`})) + + p.on('ping', (time) => { + statusStream.push(getPacket({ Success: true, Time: time })) + totalTime += time + packetCount++ + if (packetCount >= count) { + const average = totalTime / count + p.stop() + statusStream.push(getPacket({ Success: true, Text: `Average latency: ${average}ms` })) + statusStream.end() + } + }) + + p.on('error', (err) => { + log.error(err) + p.stop() + statusStream.push(getPacket({Text: err.toString()})) + statusStream.end(err) + }) + + p.start() + + return cb() }) } 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..3bc0b74b48 --- /dev/null +++ b/src/http/api/resources/ping.js @@ -0,0 +1,38 @@ +'use strict' + +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 + +exports.get = { + validate: { + query: Joi.object().keys({ + n: Joi.number().greater(0), + count: Joi.number().greater(0), + arg: Joi.string().required() + }).xor('n', 'count').unknown() + }, + 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, count, (err, pullStream) => { + if (err) { + return reply(boom.badRequest(err)) + } + // 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') + }) + } +} 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..92b081862f --- /dev/null +++ b/src/http/api/routes/ping.js @@ -0,0 +1,16 @@ +'use strict' + +const resources = require('./../resources') + +module.exports = (server) => { + const api = server.select('API') + + api.route({ + method: '*', + path: '/api/v0/ping', + config: { + handler: resources.ping.get.handler, + validate: resources.ping.get.validate + } + }) +} diff --git a/test/cli/commands.js b/test/cli/commands.js index 06154cfac6..1e194eb143 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 diff --git a/test/cli/ping.js b/test/cli/ping.js new file mode 100644 index 0000000000..a571b2e380 --- /dev/null +++ b/test/cli/ping.js @@ -0,0 +1,149 @@ +/* eslint max-nested-callbacks: ["error", 8] */ +/* eslint-env mocha */ +'use strict' + +const chai = require('chai') +const dirtyChai = require('dirty-chai') +const series = require('async/series') +const DaemonFactory = require('ipfsd-ctl') +const ipfsExec = require('../utils/ipfs-exec') + +const df = DaemonFactory.create({ type: 'js' }) +const expect = chai.expect +chai.use(dirtyChai) + +const config = { + Bootstrap: [], + Discovery: { + MDNS: { + Enabled: + false + } + } +} + +describe('ping', function () { + this.timeout(60 * 1000) + let ipfsdA + let ipfsdB + let bMultiaddr + let ipfsdBId + let cli + + 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() + ipfsdBId = peerInfo.id + bMultiaddr = peerInfo.addresses[0] + cb() + }) + } + ], 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 + // Without DHT we need to have an already established connection + 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', (output) => { + const packets = output.toString().split('\n').slice(0, -1) + result.push(...packets) + }) + + 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() + }) + }) +}) 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() + }) + ) + }) + }) +}) 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 94c3db661271c7b8ca8298523b9d564130466724 Mon Sep 17 00:00:00 2001 From: David Dias Date: Tue, 8 May 2018 09:00:54 +0200 Subject: [PATCH 02/18] chore: update deps --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 898324a87d..0d8f520024 100644 --- a/package.json +++ b/package.json @@ -78,13 +78,13 @@ "lodash": "^4.17.10", "mocha": "^5.1.1", "ncp": "^2.0.0", - "nexpect": "^0.5.0", + "nexpect": "~0.5.0", "pretty-bytes": "^4.0.2", "qs": "^6.5.2", "random-fs": "^1.0.3", "rimraf": "^2.6.2", "stream-to-promise": "^2.2.0", - "transform-loader": "^0.2.4" + "transform-loader": "~0.2.4" }, "dependencies": { "async": "^2.6.0", From b052b7d8f3d3dadddfcc571bedce2c55f4a3fa44 Mon Sep 17 00:00:00 2001 From: David Dias Date: Tue, 8 May 2018 09:20:23 +0200 Subject: [PATCH 03/18] fix: simplify circle --- circle.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/circle.yml b/circle.yml index 55886db205..0c02b8f7d6 100644 --- a/circle.yml +++ b/circle.yml @@ -1,13 +1,10 @@ # Warning: This file is automatically synced from https://github.com/ipfs/ci-sync so if you want to change it, please change it there and ask someone to sync all repositories. machine: node: - version: 8 + version: 8.11.1 test: - pre: - - npm run lint post: - - make test - npm run coverage -- --upload --providers coveralls dependencies: From e6a98780a08d33bcd40b2bd7965df1de1745d213 Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Tue, 8 May 2018 10:18:55 +0100 Subject: [PATCH 04/18] chore: move ping http validation back to joi alternatives --- src/http/api/resources/ping.js | 9 +++++++-- test/http-api/inject/ping.js | 4 ++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/http/api/resources/ping.js b/src/http/api/resources/ping.js index 3bc0b74b48..35c65ad818 100644 --- a/src/http/api/resources/ping.js +++ b/src/http/api/resources/ping.js @@ -11,10 +11,15 @@ exports = module.exports exports.get = { validate: { query: Joi.object().keys({ - n: Joi.number().greater(0), + n: Joi.alternatives() + .when('count', { + is: Joi.any().exist(), + then: Joi.any().forbidden(), + otherwise: Joi.number().greater(0) + }), count: Joi.number().greater(0), arg: Joi.string().required() - }).xor('n', 'count').unknown() + }).unknown() }, handler: (request, reply) => { const ipfs = request.server.app.ipfs diff --git a/test/http-api/inject/ping.js b/test/http-api/inject/ping.js index 8cda3836c3..7ccf747816 100644 --- a/test/http-api/inject/ping.js +++ b/test/http-api/inject/ping.js @@ -19,7 +19,7 @@ module.exports = (http) => { 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` + url: `/api/v0/ping?arg=peerid&n=1&count=1` }, (res) => { expect(res.statusCode).to.equal(400) done() @@ -39,7 +39,7 @@ module.exports = (http) => { it('returns 200 and the response stream with the ping result', (done) => { api.inject({ method: 'GET', - url: `/api/v0/ping?arg=someRandomId` + url: `/api/v0/ping?arg=peerid` }, (res) => { expect(res.statusCode).to.equal(200) expect(res.headers['x-chunked-output']).to.equal('1') From e8508a4020a56a5aec75eec5cd6fd07e558b154d Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Tue, 8 May 2018 17:10:51 +0100 Subject: [PATCH 05/18] chore: remove useless pull-catch dep --- package.json | 1 - src/cli/commands/ping.js | 4 ---- test/core/ping.spec.js | 19 ++++++------------- 3 files changed, 6 insertions(+), 18 deletions(-) diff --git a/package.json b/package.json index 0d8f520024..c7309dc402 100644 --- a/package.json +++ b/package.json @@ -154,7 +154,6 @@ "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 ab9d73666e..4163d11df1 100644 --- a/src/cli/commands/ping.js +++ b/src/cli/commands/ping.js @@ -2,7 +2,6 @@ const pull = require('pull-stream/pull') const drain = require('pull-stream/sinks/drain') -const pullCatch = require('pull-catch') const print = require('../utils').print @@ -24,9 +23,6 @@ module.exports = { const count = argv.count || 10 pull( argv.ipfs.pingPullStream(peerId, { count }), - pullCatch(err => { - throw err - }), drain(({ Time, Text }) => { // Check if it's a pong if (Time) { diff --git a/test/core/ping.spec.js b/test/core/ping.spec.js index b94299ba80..5e1dd95289 100644 --- a/test/core/ping.spec.js +++ b/test/core/ping.spec.js @@ -5,7 +5,6 @@ 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') @@ -86,16 +85,14 @@ describe('ping', function () { 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++ } - }, () => { + }, (err) => { + expect(err).to.not.exist() expect(packetNum).to.equal(count) done() }) @@ -107,9 +104,6 @@ describe('ping', function () { 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 @@ -121,7 +115,8 @@ describe('ping', function () { if (messageNum === 2) { expect(Success).to.be.false() } - }, () => { + }, (err) => { + expect(err).to.not.exist() expect(messageNum).to.equal(count) done() }) @@ -196,9 +191,6 @@ describe('ping', function () { 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() @@ -210,7 +202,8 @@ describe('ping', function () { if (Time) { packetNum++ } - }, () => { + }, (err) => { + expect(err).to.not.exist() expect(packetNum).to.equal(count) done() }) From 63362567c2e1fde56899a11926ef5946c1069ad4 Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Tue, 8 May 2018 17:43:10 +0100 Subject: [PATCH 06/18] chore: no need to require the pull module twice --- src/cli/commands/ping.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cli/commands/ping.js b/src/cli/commands/ping.js index 4163d11df1..7d48bc4af2 100644 --- a/src/cli/commands/ping.js +++ b/src/cli/commands/ping.js @@ -1,7 +1,6 @@ 'use strict' const pull = require('pull-stream/pull') -const drain = require('pull-stream/sinks/drain') const print = require('../utils').print @@ -23,7 +22,7 @@ module.exports = { const count = argv.count || 10 pull( argv.ipfs.pingPullStream(peerId, { count }), - drain(({ Time, Text }) => { + pull.drain(({ Time, Text }) => { // Check if it's a pong if (Time) { print(`Pong received: time=${Time} ms`) From bbfa94bd1c2c9939261e9facc4663adfc5fd312e Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Tue, 8 May 2018 18:20:36 +0100 Subject: [PATCH 07/18] chore: minor cleanup --- src/core/components/ping.js | 9 +++++---- src/http/api/resources/ping.js | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/core/components/ping.js b/src/core/components/ping.js index 6804acf83d..a6be305d25 100644 --- a/src/core/components/ping.js +++ b/src/core/components/ping.js @@ -42,30 +42,31 @@ module.exports = function ping (self) { function getPacket (msg) { // Default msg const basePacket = {Success: false, Time: 0, Text: ''} - return Object.assign({}, basePacket, msg) + return Object.assign(basePacket, msg) } function getPeer (libp2pNode, statusStream, peerId, cb) { let peer try { peer = libp2pNode.peerBook.get(peerId) - 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.createFromB58String(peerId), cb) + return libp2pNode.peerRouting.findPeer(PeerId.createFromB58String(peerId), cb) } + return cb(null, peer) } function runPing (libp2pNode, statusStream, count, peer, cb) { libp2pNode.ping(peer, (err, p) => { - log('Got peer', peer) if (err) { return cb(err) } + log('Got peer', peer) + let packetCount = 0 let totalTime = 0 statusStream.push(getPacket({Success: true, Text: `PING ${peer.id.toB58String()}`})) diff --git a/src/http/api/resources/ping.js b/src/http/api/resources/ping.js index 35c65ad818..c51ff5c008 100644 --- a/src/http/api/resources/ping.js +++ b/src/http/api/resources/ping.js @@ -15,9 +15,9 @@ exports.get = { .when('count', { is: Joi.any().exist(), then: Joi.any().forbidden(), - otherwise: Joi.number().greater(0) + otherwise: Joi.number().integer().greater(0) }), - count: Joi.number().greater(0), + count: Joi.number().integer().greater(0), arg: Joi.string().required() }).unknown() }, From e5cee1182d4cbd81aae490aee6b450d559604dd4 Mon Sep 17 00:00:00 2001 From: David Dias Date: Wed, 9 May 2018 08:27:53 +0200 Subject: [PATCH 08/18] test: update timeout to make sure it happens after swarm connect --- test/core/ping.spec.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/core/ping.spec.js b/test/core/ping.spec.js index 5e1dd95289..dc9af14dc2 100644 --- a/test/core/ping.spec.js +++ b/test/core/ping.spec.js @@ -175,11 +175,10 @@ describe('ping', function () { parallel([ ipfsdA.api.swarm.connect.bind(ipfsdA.api, bMultiaddr), ipfsdB.api.swarm.connect.bind(ipfsdB.api, cMultiaddr) - ], done) + ], (err) => setTimeout(() => done(err), 500)) // FIXME timeout needed for connections to succeed }) - // FIXME timeout needed for connections to succeed - before((done) => setTimeout(done, 100)) + after((done) => ipfsdA.stop(done)) after((done) => ipfsdB.stop(done)) From 541dc73cc4af7d0fbff2f442229930d6c8efb6a2 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Mon, 14 May 2018 14:34:14 +0100 Subject: [PATCH 09/18] feat: add pingReadableStream and pingPullStream License: MIT Signed-off-by: Alan Shaw --- src/cli/commands/ping.js | 8 +- src/core/components/index.js | 2 + src/core/components/ping-pull-stream.js | 93 +++++++++++++++++++++ src/core/components/ping-readable-stream.js | 7 ++ src/core/components/ping.js | 92 +------------------- src/core/index.js | 2 + src/http/api/resources/ping.js | 37 ++++---- test/core/ping.spec.js | 26 +++--- 8 files changed, 145 insertions(+), 122 deletions(-) create mode 100644 src/core/components/ping-pull-stream.js create mode 100644 src/core/components/ping-readable-stream.js diff --git a/src/cli/commands/ping.js b/src/cli/commands/ping.js index 7d48bc4af2..31843bb448 100644 --- a/src/cli/commands/ping.js +++ b/src/cli/commands/ping.js @@ -22,13 +22,13 @@ module.exports = { const count = argv.count || 10 pull( argv.ipfs.pingPullStream(peerId, { count }), - pull.drain(({ Time, Text }) => { + pull.drain(({ time, text }) => { // Check if it's a pong - if (Time) { - print(`Pong received: time=${Time} ms`) + if (time) { + print(`Pong received: time=${time} ms`) // Status response } else { - print(Text) + print(text) } }) ) diff --git a/src/core/components/index.js b/src/core/components/index.js index 4f4e410e43..ce95b27a53 100644 --- a/src/core/components/index.js +++ b/src/core/components/index.js @@ -16,6 +16,8 @@ exports.dag = require('./dag') exports.libp2p = require('./libp2p') exports.swarm = require('./swarm') exports.ping = require('./ping') +exports.pingPullStream = require('./ping-pull-stream') +exports.pingReadableStream = require('./ping-readable-stream') exports.files = require('./files') exports.bitswap = require('./bitswap') exports.pubsub = require('./pubsub') diff --git a/src/core/components/ping-pull-stream.js b/src/core/components/ping-pull-stream.js new file mode 100644 index 0000000000..6d4d28b71a --- /dev/null +++ b/src/core/components/ping-pull-stream.js @@ -0,0 +1,93 @@ +'use strict' + +const debug = require('debug') +const OFFLINE_ERROR = require('../utils').OFFLINE_ERROR +const PeerId = require('peer-id') +const pull = require('pull-stream') +const Pushable = require('pull-pushable') +const waterfall = require('async/waterfall') + +const log = debug('jsipfs:pingPullStream') +log.error = debug('jsipfs:pingPullStream:error') + +module.exports = function pingPullStream (self) { + return (peerId, opts) => { + if (!self.isOnline()) { + return pull.error(new Error(OFFLINE_ERROR)) + } + + opts = Object.assign({ count: 10 }, opts) + + const source = Pushable() + + waterfall([ + (cb) => getPeer(self._libp2pNode, source, peerId, cb), + (peer, cb) => runPing(self._libp2pNode, source, opts.count, peer, cb) + ], (err) => { + if (err) { + log.error(err) + source.push(getPacket({ text: err.toString() })) + source.end(err) + } + }) + + return source + } +} + +function getPacket (msg) { + // Default msg + const basePacket = { success: true, time: 0, text: '' } + return Object.assign(basePacket, msg) +} + +function getPeer (libp2pNode, statusStream, peerId, cb) { + let peer + try { + peer = libp2pNode.peerBook.get(peerId) + } catch (err) { + log('Peer not found in peer book, trying peer routing') + // Share lookup status just as in the go implemmentation + statusStream.push(getPacket({ text: `Looking up peer ${peerId}` })) + // Try to use peerRouting + return libp2pNode.peerRouting.findPeer(PeerId.createFromB58String(peerId), cb) + } + return cb(null, peer) +} + +function runPing (libp2pNode, statusStream, count, peer, cb) { + libp2pNode.ping(peer, (err, p) => { + if (err) { + return cb(err) + } + + log('Got peer', peer) + + let packetCount = 0 + let totalTime = 0 + statusStream.push(getPacket({ text: `PING ${peer.id.toB58String()}` })) + + p.on('ping', (time) => { + statusStream.push(getPacket({ time: time })) + totalTime += time + packetCount++ + if (packetCount >= count) { + const average = totalTime / count + p.stop() + statusStream.push(getPacket({ text: `Average latency: ${average}ms` })) + statusStream.end() + } + }) + + p.on('error', (err) => { + log.error(err) + p.stop() + statusStream.push(getPacket({ success: false, text: err.toString() })) + statusStream.end(err) + }) + + p.start() + + return cb() + }) +} diff --git a/src/core/components/ping-readable-stream.js b/src/core/components/ping-readable-stream.js new file mode 100644 index 0000000000..b6809ffb48 --- /dev/null +++ b/src/core/components/ping-readable-stream.js @@ -0,0 +1,7 @@ +'use strict' + +const toStream = require('pull-stream-to-stream') + +module.exports = function pingReadableStream (self) { + return (peerId, opts) => toStream.source(self.pingPullStream(peerId, opts)) +} diff --git a/src/core/components/ping.js b/src/core/components/ping.js index a6be305d25..c5bfba9134 100644 --- a/src/core/components/ping.js +++ b/src/core/components/ping.js @@ -1,97 +1,13 @@ 'use strict' 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') -const waterfall = require('async/waterfall') - -const log = debug('jsipfs:ping') -log.error = debug('jsipfs:ping:error') module.exports = function ping (self) { - return promisify((peerId, count, cb) => { - if (!self.isOnline()) { - return cb(new Error(OFFLINE_ERROR)) - } - - const source = Pushable() - - const response = pull( - source, - ndjson.serialize() + return promisify((peerId, opts, cb) => { + pull( + self.pingPullStream(peerId, opts), + pull.collect(cb) ) - waterfall([ - getPeer.bind(null, self._libp2pNode, source, peerId), - runPing.bind(null, self._libp2pNode, source, count) - ], (err) => { - if (err) { - log.error(err) - source.push(getPacket({Text: err.toString()})) - source.end(err) - } - }) - - cb(null, response) - }) -} - -function getPacket (msg) { - // Default msg - const basePacket = {Success: false, Time: 0, Text: ''} - return Object.assign(basePacket, msg) -} - -function getPeer (libp2pNode, statusStream, peerId, cb) { - let peer - try { - peer = libp2pNode.peerBook.get(peerId) - } 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 - return libp2pNode.peerRouting.findPeer(PeerId.createFromB58String(peerId), cb) - } - return cb(null, peer) -} - -function runPing (libp2pNode, statusStream, count, peer, cb) { - libp2pNode.ping(peer, (err, p) => { - if (err) { - return cb(err) - } - - log('Got peer', peer) - - let packetCount = 0 - let totalTime = 0 - statusStream.push(getPacket({Success: true, Text: `PING ${peer.id.toB58String()}`})) - - p.on('ping', (time) => { - statusStream.push(getPacket({ Success: true, Time: time })) - totalTime += time - packetCount++ - if (packetCount >= count) { - const average = totalTime / count - p.stop() - statusStream.push(getPacket({ Success: true, Text: `Average latency: ${average}ms` })) - statusStream.end() - } - }) - - p.on('error', (err) => { - log.error(err) - p.stop() - statusStream.push(getPacket({Text: err.toString()})) - statusStream.end(err) - }) - - p.start() - - return cb() }) } diff --git a/src/core/index.js b/src/core/index.js index 46f8c9bb2c..0b19160429 100644 --- a/src/core/index.js +++ b/src/core/index.js @@ -102,6 +102,8 @@ class IPFS extends EventEmitter { this.files = components.files(this) this.bitswap = components.bitswap(this) this.ping = components.ping(this) + this.pingPullStream = components.pingPullStream(this) + this.pingReadableStream = components.pingReadableStream(this) this.pubsub = components.pubsub(this) this.dht = components.dht(this) this.dns = components.dns(this) diff --git a/src/http/api/resources/ping.js b/src/http/api/resources/ping.js index c51ff5c008..56824db629 100644 --- a/src/http/api/resources/ping.js +++ b/src/http/api/resources/ping.js @@ -1,13 +1,12 @@ 'use strict' const Joi = require('joi') -const boom = require('boom') +const pull = require('pull-stream') const toStream = require('pull-stream-to-stream') +const ndjson = require('pull-ndjson') const PassThrough = require('readable-stream').PassThrough const pump = require('pump') -exports = module.exports - exports.get = { validate: { query: Joi.object().keys({ @@ -26,18 +25,24 @@ 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)) - } - // 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') - }) + + const source = pull( + ipfs.pingPullStream(peerId, { count: count }), + pull.map((chunk) => ({ + Success: chunk.success, + Time: chunk.time, + Text: chunk.text + })), + ndjson.serialize() + ) + + // 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(source) + const stream2 = new PassThrough() + pump(responseStream, stream2) + return reply(stream2).type('application/json').header('X-Chunked-Output', '1') } } diff --git a/test/core/ping.spec.js b/test/core/ping.spec.js index dc9af14dc2..d94689981b 100644 --- a/test/core/ping.spec.js +++ b/test/core/ping.spec.js @@ -32,7 +32,7 @@ function spawnNode ({ dht = false }, cb) { }, cb) } -describe('ping', function () { +describe.only('ping', function () { this.timeout(60 * 1000) if (!isNode) return @@ -85,10 +85,10 @@ describe('ping', function () { const count = 3 pull( ipfsdA.api.pingPullStream(ipfsdBId, { count }), - drain(({ Success, Time, Text }) => { - expect(Success).to.be.true() + drain(({ success, time, text }) => { + expect(success).to.be.true() // It's a pong - if (Time) { + if (time) { packetNum++ } }, (err) => { @@ -104,16 +104,16 @@ describe('ping', function () { const count = 2 pull( ipfsdA.api.pingPullStream('unknown', { count }), - drain(({ Success, Time, Text }) => { + drain(({ success, time, text }) => { messageNum++ // Assert that the ping command falls back to the peerRouting if (messageNum === 1) { - expect(Text).to.include('Looking up') + expect(text).to.include('Looking up') } // Fails accordingly while trying to use peerRouting if (messageNum === 2) { - expect(Success).to.be.false() + expect(success).to.be.false() } }, (err) => { expect(err).to.not.exist() @@ -175,11 +175,9 @@ describe('ping', function () { parallel([ ipfsdA.api.swarm.connect.bind(ipfsdA.api, bMultiaddr), ipfsdB.api.swarm.connect.bind(ipfsdB.api, cMultiaddr) - ], (err) => setTimeout(() => done(err), 500)) // FIXME timeout needed for connections to succeed + ], (err) => setTimeout(() => done(err), 500)) // FIXME timeout needed for connections to succeed }) - - after((done) => ipfsdA.stop(done)) after((done) => ipfsdB.stop(done)) after((done) => ipfsdC.stop(done)) @@ -190,15 +188,15 @@ describe('ping', function () { const count = 3 pull( ipfsdA.api.pingPullStream(ipfsdCId, { count }), - drain(({ Success, Time, Text }) => { + drain(({ success, time, text }) => { messageNum++ - expect(Success).to.be.true() + expect(success).to.be.true() // Assert that the ping command falls back to the peerRouting if (messageNum === 1) { - expect(Text).to.include('Looking up') + expect(text).to.include('Looking up') } // It's a pong - if (Time) { + if (time) { packetNum++ } }, (err) => { From d9789dc72ed5c2de785ce5f3a7200cda76615412 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Mon, 14 May 2018 14:35:42 +0100 Subject: [PATCH 10/18] fix: remove .only License: MIT Signed-off-by: Alan Shaw --- test/core/ping.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/ping.spec.js b/test/core/ping.spec.js index d94689981b..c6d1334e86 100644 --- a/test/core/ping.spec.js +++ b/test/core/ping.spec.js @@ -32,7 +32,7 @@ function spawnNode ({ dht = false }, cb) { }, cb) } -describe.only('ping', function () { +describe('ping', function () { this.timeout(60 * 1000) if (!isNode) return From b71c633c6f58a3a2a88afc0c6b37eb74929b978b Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Mon, 14 May 2018 15:52:03 +0100 Subject: [PATCH 11/18] fix: ping test fixes License: MIT Signed-off-by: Alan Shaw --- src/cli/commands/ping.js | 3 +-- src/core/components/ping-pull-stream.js | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/cli/commands/ping.js b/src/cli/commands/ping.js index 31843bb448..77e2fdbeb4 100644 --- a/src/cli/commands/ping.js +++ b/src/cli/commands/ping.js @@ -1,7 +1,6 @@ 'use strict' -const pull = require('pull-stream/pull') - +const pull = require('pull-stream') const print = require('../utils').print module.exports = { diff --git a/src/core/components/ping-pull-stream.js b/src/core/components/ping-pull-stream.js index 6d4d28b71a..72531b8d0b 100644 --- a/src/core/components/ping-pull-stream.js +++ b/src/core/components/ping-pull-stream.js @@ -26,7 +26,7 @@ module.exports = function pingPullStream (self) { ], (err) => { if (err) { log.error(err) - source.push(getPacket({ text: err.toString() })) + source.push(getPacket({ success: false, text: err.toString() })) source.end(err) } }) @@ -43,16 +43,25 @@ function getPacket (msg) { function getPeer (libp2pNode, statusStream, peerId, cb) { let peer + try { peer = libp2pNode.peerBook.get(peerId) } catch (err) { log('Peer not found in peer book, trying peer routing') // Share lookup status just as in the go implemmentation statusStream.push(getPacket({ text: `Looking up peer ${peerId}` })) + // Try to use peerRouting - return libp2pNode.peerRouting.findPeer(PeerId.createFromB58String(peerId), cb) + try { + peerId = PeerId.createFromB58String(peerId) + } catch (err) { + return cb(err) + } + + return libp2pNode.peerRouting.findPeer(peerId, cb) } - return cb(null, peer) + + cb(null, peer) } function runPing (libp2pNode, statusStream, count, peer, cb) { From 0eca618c6a534e17a744d95142f7c2928828e895 Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Tue, 15 May 2018 22:36:08 +0100 Subject: [PATCH 12/18] fix(ping): peers setup is not dependent on an artificial delay --- test/core/ping.spec.js | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/test/core/ping.spec.js b/test/core/ping.spec.js index c6d1334e86..aed1f62712 100644 --- a/test/core/ping.spec.js +++ b/test/core/ping.spec.js @@ -170,12 +170,28 @@ describe('ping', function () { // Connect the nodes before(function (done) { - this.timeout(60 * 1000) + this.timeout(30 * 1000) + let interval + + // Check to see if peers are already connected + const checkConnections = () => { + ipfsdB.api.swarm.peers((err, peerInfos) => { + if (err) return done(err) + + if (peerInfos.length > 1) { + clearInterval(interval) + return done() + } + }) + } parallel([ ipfsdA.api.swarm.connect.bind(ipfsdA.api, bMultiaddr), ipfsdB.api.swarm.connect.bind(ipfsdB.api, cMultiaddr) - ], (err) => setTimeout(() => done(err), 500)) // FIXME timeout needed for connections to succeed + ], (err) => { + if (err) return done(err) + interval = setInterval(checkConnections, 300) + }) }) after((done) => ipfsdA.stop(done)) From 3571450e8b3356844928ac91f9713cf5172b7a57 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Wed, 16 May 2018 14:12:28 +0100 Subject: [PATCH 13/18] test: add interface-ipfs-core ping tests License: MIT Signed-off-by: Alan Shaw --- src/core/components/ping-pull-stream.js | 4 ++- test/core/interface/interface.spec.js | 1 + test/core/interface/ping.js | 35 +++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 test/core/interface/ping.js diff --git a/src/core/components/ping-pull-stream.js b/src/core/components/ping-pull-stream.js index 72531b8d0b..c4a2acfeae 100644 --- a/src/core/components/ping-pull-stream.js +++ b/src/core/components/ping-pull-stream.js @@ -55,7 +55,9 @@ function getPeer (libp2pNode, statusStream, peerId, cb) { try { peerId = PeerId.createFromB58String(peerId) } catch (err) { - return cb(err) + return cb(Object.assign(err, { + message: `failed to parse peer address '${peerId}': input isn't valid multihash` + })) } return libp2pNode.peerRouting.findPeer(peerId, cb) diff --git a/test/core/interface/interface.spec.js b/test/core/interface/interface.spec.js index 810b7f2198..356036adeb 100644 --- a/test/core/interface/interface.spec.js +++ b/test/core/interface/interface.spec.js @@ -16,6 +16,7 @@ describe('interface-ipfs-core tests', () => { require('./key') if (isNode) { require('./swarm') + require('./ping') require('./pubsub') require('./dht') } diff --git a/test/core/interface/ping.js b/test/core/interface/ping.js new file mode 100644 index 0000000000..cc0f85b9c1 --- /dev/null +++ b/test/core/interface/ping.js @@ -0,0 +1,35 @@ +/* eslint-env mocha */ +'use strict' + +const test = require('interface-ipfs-core') +const parallel = require('async/parallel') + +const IPFS = require('../../../src') + +const DaemonFactory = require('ipfsd-ctl') +const df = DaemonFactory.create({ type: 'proc', exec: IPFS }) + +const nodes = [] +const common = { + setup: function (callback) { + callback(null, { + spawnNode: (cb) => { + df.spawn({ + initOptions: { bits: 512 } + }, (err, _ipfsd) => { + if (err) { + return cb(err) + } + + nodes.push(_ipfsd) + cb(null, _ipfsd.api) + }) + } + }) + }, + teardown: function (callback) { + parallel(nodes.map((node) => (cb) => node.stop(cb)), callback) + } +} + +test.ping(common) From f9adf54b35dca845ad8ad83e446b1553083dc3e2 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Fri, 18 May 2018 16:14:10 +0100 Subject: [PATCH 14/18] chore: upgrade interface-ipfs-core dependency 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 c7309dc402..a95f4d1f0c 100644 --- a/package.json +++ b/package.json @@ -73,7 +73,7 @@ "expose-loader": "^0.7.5", "form-data": "^2.3.2", "hat": "0.0.3", - "interface-ipfs-core": "~0.65.7", + "interface-ipfs-core": "~0.66.2", "ipfsd-ctl": "~0.34.0", "lodash": "^4.17.10", "mocha": "^5.1.1", From 29959fb6a88ecd27d816f3d08f54547d3d4fe177 Mon Sep 17 00:00:00 2001 From: David Dias Date: Sun, 20 May 2018 19:09:45 +0100 Subject: [PATCH 15/18] chore: update deps --- package.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index a95f4d1f0c..4fec680150 100644 --- a/package.json +++ b/package.json @@ -74,7 +74,7 @@ "form-data": "^2.3.2", "hat": "0.0.3", "interface-ipfs-core": "~0.66.2", - "ipfsd-ctl": "~0.34.0", + "ipfsd-ctl": "~0.36.0", "lodash": "^4.17.10", "mocha": "^5.1.1", "ncp": "^2.0.0", @@ -105,7 +105,7 @@ "hapi-set-header": "^1.0.2", "hoek": "^5.0.3", "human-to-milliseconds": "^1.0.0", - "ipfs-api": "^21.0.0", + "ipfs-api": "^22.0.0", "ipfs-bitswap": "~0.20.0", "ipfs-block": "~0.7.1", "ipfs-block-service": "~0.14.0", @@ -148,7 +148,7 @@ "multihashes": "~0.4.13", "once": "^1.4.0", "path-exists": "^3.0.0", - "peer-book": "~0.7.0", + "peer-book": "~0.8.0", "peer-id": "~0.10.7", "peer-info": "~0.14.1", "progress": "^2.0.0", From 8c98145eb61a44da7a7a060fde1b48469e61b4c0 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Mon, 21 May 2018 14:51:52 +0100 Subject: [PATCH 16/18] fix: more robust ping tests License: MIT Signed-off-by: Alan Shaw --- test/cli/ping.js | 11 +++++++++-- test/core/ping.spec.js | 41 +++++++++++++++++++++++++++++++---------- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/test/cli/ping.js b/test/cli/ping.js index a571b2e380..170a1fc3b9 100644 --- a/test/cli/ping.js +++ b/test/cli/ping.js @@ -76,8 +76,15 @@ describe('ping', function () { done() }) - after((done) => ipfsdA.stop(done)) - after((done) => ipfsdB.stop(done)) + after((done) => { + if (!ipfsdA) return done() + ipfsdA.stop(done) + }) + + after((done) => { + if (!ipfsdB) return done() + ipfsdB.stop(done) + }) it('ping host', (done) => { this.timeout(60 * 1000) diff --git a/test/core/ping.spec.js b/test/core/ping.spec.js index aed1f62712..ab19b3d2cc 100644 --- a/test/core/ping.spec.js +++ b/test/core/ping.spec.js @@ -6,6 +6,7 @@ const dirtyChai = require('dirty-chai') const pull = require('pull-stream/pull') const drain = require('pull-stream/sinks/drain') const parallel = require('async/parallel') +const series = require('async/series') const DaemonFactory = require('ipfsd-ctl') const isNode = require('detect-node') @@ -48,7 +49,7 @@ describe('ping', function () { before(function (done) { this.timeout(60 * 1000) - parallel([ + series([ spawnNode.bind(null, { dht: false }), spawnNode.bind(null, { dht: false }) ], (err, ipfsd) => { @@ -77,15 +78,22 @@ describe('ping', function () { ipfsdA.api.swarm.connect(bMultiaddr, done) }) - after((done) => ipfsdA.stop(done)) - after((done) => ipfsdB.stop(done)) + after((done) => { + if (!ipfsdA) return done() + ipfsdA.stop(done) + }) + + after((done) => { + if (!ipfsdB) return done() + ipfsdB.stop(done) + }) it('sends the specified number of packets', (done) => { let packetNum = 0 const count = 3 pull( ipfsdA.api.pingPullStream(ipfsdBId, { count }), - drain(({ success, time, text }) => { + drain(({ success, time }) => { expect(success).to.be.true() // It's a pong if (time) { @@ -100,10 +108,11 @@ describe('ping', function () { }) it('pinging an unknown peer will fail accordingly', (done) => { + const unknownPeerId = 'QmUmaEnH1uMmvckMZbh3yShaasvELPW4ZLPWnB4entMTEn' let messageNum = 0 const count = 2 pull( - ipfsdA.api.pingPullStream('unknown', { count }), + ipfsdA.api.pingPullStream(unknownPeerId, { count }), drain(({ success, time, text }) => { messageNum++ // Assert that the ping command falls back to the peerRouting @@ -116,7 +125,8 @@ describe('ping', function () { expect(success).to.be.false() } }, (err) => { - expect(err).to.not.exist() + expect(err).to.exist() + expect(err.message).to.include('DHT is not available') expect(messageNum).to.equal(count) done() }) @@ -139,7 +149,7 @@ describe('ping', function () { before(function (done) { this.timeout(60 * 1000) - parallel([ + series([ spawnNode.bind(null, { dht: true }), spawnNode.bind(null, { dht: true }), spawnNode.bind(null, { dht: true }) @@ -194,9 +204,20 @@ describe('ping', function () { }) }) - after((done) => ipfsdA.stop(done)) - after((done) => ipfsdB.stop(done)) - after((done) => ipfsdC.stop(done)) + after((done) => { + if (!ipfsdA) return done() + ipfsdA.stop(done) + }) + + after((done) => { + if (!ipfsdB) return done() + ipfsdB.stop(done) + }) + + after((done) => { + if (!ipfsdC) return done() + ipfsdC.stop(done) + }) it('if enabled uses the DHT peer routing to find peer', (done) => { let messageNum = 0 From d7b4fe11cebeddea1fc11382bf2b0c3ff98a3cf4 Mon Sep 17 00:00:00 2001 From: David Dias Date: Wed, 23 May 2018 10:57:42 +0100 Subject: [PATCH 17/18] chore: update ipfsd-ctl --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 4fec680150..73c2ed61f2 100644 --- a/package.json +++ b/package.json @@ -74,7 +74,7 @@ "form-data": "^2.3.2", "hat": "0.0.3", "interface-ipfs-core": "~0.66.2", - "ipfsd-ctl": "~0.36.0", + "ipfsd-ctl": "~0.37.0", "lodash": "^4.17.10", "mocha": "^5.1.1", "ncp": "^2.0.0", From 1c8eccf6e068301e30d02c2d4a58bbe0f901b75f Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Thu, 24 May 2018 21:05:15 +0100 Subject: [PATCH 18/18] fix: correctly differentiate pong responses License: MIT Signed-off-by: Alan Shaw --- src/cli/commands/ping.js | 4 ++-- test/core/ping.spec.js | 19 ++++++++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/cli/commands/ping.js b/src/cli/commands/ping.js index 77e2fdbeb4..3760b33b75 100644 --- a/src/cli/commands/ping.js +++ b/src/cli/commands/ping.js @@ -21,9 +21,9 @@ module.exports = { const count = argv.count || 10 pull( argv.ipfs.pingPullStream(peerId, { count }), - pull.drain(({ time, text }) => { + pull.drain(({ success, time, text }) => { // Check if it's a pong - if (time) { + if (success && !text) { print(`Pong received: time=${time} ms`) // Status response } else { diff --git a/test/core/ping.spec.js b/test/core/ping.spec.js index ab19b3d2cc..583ff6c964 100644 --- a/test/core/ping.spec.js +++ b/test/core/ping.spec.js @@ -33,6 +33,11 @@ function spawnNode ({ dht = false }, cb) { }, cb) } +// Determine if a ping response object is a pong, or something else, like a status message +function isPong (pingResponse) { + return Boolean(pingResponse && pingResponse.success && !pingResponse.text) +} + describe('ping', function () { this.timeout(60 * 1000) @@ -93,10 +98,10 @@ describe('ping', function () { const count = 3 pull( ipfsdA.api.pingPullStream(ipfsdBId, { count }), - drain(({ success, time }) => { - expect(success).to.be.true() + drain((res) => { + expect(res.success).to.be.true() // It's a pong - if (time) { + if (isPong(res)) { packetNum++ } }, (err) => { @@ -225,15 +230,15 @@ describe('ping', function () { const count = 3 pull( ipfsdA.api.pingPullStream(ipfsdCId, { count }), - drain(({ success, time, text }) => { + drain((res) => { messageNum++ - expect(success).to.be.true() + expect(res.success).to.be.true() // Assert that the ping command falls back to the peerRouting if (messageNum === 1) { - expect(text).to.include('Looking up') + expect(res.text).to.include('Looking up') } // It's a pong - if (time) { + if (isPong(res)) { packetNum++ } }, (err) => {