Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

feat: implement ipfs ping flags #928 #1202

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions src/cli/commands/ping.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
'use strict'

const print = require('../utils').print

module.exports = {
command: 'ping <peerId>',

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)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A ping with count 10 is a ping with 10 packets, not 10 ping calls. You need to count the number of pings that come in p.

Copy link
Author

@melvin0008 melvin0008 Feb 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that 10 ping calls is wrong. I started refactoring the code and was able to use the ping object and stream the times to the api.

But I have run into a problem. In js-ipfs-api the response assumes there will be one ping request.

So even if I stream multiple ping responses only the first one is returned.

My understanding of the flow when someone types jsipfs ping is

  1. cli/commands/ping.js -> this calls the ipfs-api ping
  2. node_modules/ipfs-api/src/ping.js -> this calls the http api ping
  3. http/api/resources/ping.js -> this calls the component ping
  4. core/components/ping.js which would then return the ping object
  5. http/api/resources/ping.js would then stream the times to the ipfs-api and end the stream when number of pings are complete.

The problem is js-ipfs-api is expecting only one response ping object. Is there a workaround I'm missing or should I create an issue inn js-ipfs-api ping and fix that. I just want to confirm whether you its an issue or not. :)

Also, irrespective of the api I was able to get the curl outputs for both the go-ipfs http api and js-ipfs http api to be the same . I have attached images for the same. But again I find one inconsistency . Since we are using new Date() in js-libp2p-ping we are not getting the same unit as go-ipfs. go-ipfs returns in nanosecond and js-ipfs returns the time in milliseconds. Let me know whether I should create a issue and fix that. I would be glad to do it. I just want to confirm whether its an issue or not.

Curl response to js-ipfs
screen shot 2018-02-17 at 6 37 38 pm

Curl response to go-ipfs
screen shot 2018-02-17 at 6 36 28 pm

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a workaround I'm missing or should I create an issue inn js-ipfs-api ping and fix that.

Sounds like it needs to be fixed in js-ipfs-api then. Just go for it too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that @ngotchac noticed a misbehavior a while ago too ipfs-inactive/js-ipfs-http-client#556

}
}
30 changes: 28 additions & 2 deletions src/core/components/ping.js
Original file line number Diff line number Diff line change
@@ -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)
})
})
})
}
1 change: 1 addition & 0 deletions src/http/api/resources/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

exports.version = require('./version')
exports.id = require('./id')
exports.ping = require('./ping')
exports.bootstrap = require('./bootstrap')
exports.repo = require('./repo')
exports.object = require('./object')
Expand Down
18 changes: 18 additions & 0 deletions src/http/api/resources/ping.js
Original file line number Diff line number Diff line change
@@ -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')
})
}
1 change: 1 addition & 0 deletions src/http/api/routes/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,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)
Expand Down
19 changes: 19 additions & 0 deletions src/http/api/routes/ping.js
Original file line number Diff line number Diff line change
@@ -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
}
})
}
2 changes: 1 addition & 1 deletion test/cli/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
const expect = require('chai').expect
const runOnAndOff = require('../utils/on-and-off')

const commandCount = 60
const commandCount = 61

describe('commands', () => runOnAndOff((thing) => {
let ipfs
Expand Down