Skip to content

Commit

Permalink
fix: stop stream after first pong received (#545)
Browse files Browse the repository at this point in the history
When connecting to go-IPFS from a webworker, the stream opened by
the ping protocol is never closed.

The change here uses `take` to only receive one buffer from the
remote node before closing the stream.
  • Loading branch information
achingbrain authored Feb 3, 2020
1 parent f39e8f0 commit be8fc9d
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/ping/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const errCode = require('err-code')
const crypto = require('libp2p-crypto')
const pipe = require('it-pipe')
const { toBuffer } = require('it-buffer')
const { collect } = require('streaming-iterables')
const { collect, take } = require('streaming-iterables')

const { PROTOCOL, PING_LENGTH } = require('./constants')

Expand All @@ -29,6 +29,7 @@ async function ping (node, peer) {
const [result] = await pipe(
[data],
stream,
stream => take(1, stream),
toBuffer,
collect
)
Expand Down
38 changes: 38 additions & 0 deletions test/core/ping.node.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ chai.use(require('dirty-chai'))
const { expect } = chai

const pTimes = require('p-times')
const pipe = require('it-pipe')

const peerUtils = require('../utils/creators/peer')
const baseOptions = require('../utils/base-options')
const { PROTOCOL } = require('../../src/ping/constants')

describe('ping', () => {
let nodes
Expand All @@ -32,4 +34,40 @@ describe('ping', () => {
const averageLatency = latencies.reduce((p, c) => p + c, 0) / latencies.length
expect(averageLatency).to.be.a('Number')
})

it('only waits for the first response to arrive', async () => {
nodes[1].handle(PROTOCOL, async ({ connection, stream }) => {
let firstInvocation = true

await pipe(
stream,
function (stream) {
const output = {
[Symbol.asyncIterator]: () => output,
next: async () => {
if (firstInvocation) {
firstInvocation = false

for await (const data of stream) {
return {
value: data,
done: false
}
}
} else {
return new Promise() // never resolve
}
}
}

return output
},
stream
)
})

const latency = await nodes[0].ping(nodes[1].peerInfo)

expect(latency).to.be.a('Number')
})
})

0 comments on commit be8fc9d

Please sign in to comment.