Skip to content

Commit

Permalink
fix: do not add abort signals to useless addresses (#913)
Browse files Browse the repository at this point in the history
  • Loading branch information
vasco-santos committed Apr 20, 2021
1 parent 3f7dde3 commit 06e8f3d
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 7 deletions.
8 changes: 6 additions & 2 deletions src/dialer/index.js
Expand Up @@ -110,7 +110,7 @@ class Dialer {
const dialTarget = await this._createDialTarget(peer)

if (!dialTarget.addrs.length) {
throw errCode(new Error('The dial request has no addresses'), codes.ERR_NO_VALID_ADDRESSES)
throw errCode(new Error('The dial request has no valid addresses'), codes.ERR_NO_VALID_ADDRESSES)
}
const pendingDial = this._pendingDials.get(dialTarget.id) || this._createPendingDial(dialTarget, options)

Expand All @@ -134,6 +134,7 @@ class Dialer {
* Creates a DialTarget. The DialTarget is used to create and track
* the DialRequest to a given peer.
* If a multiaddr is received it should be the first address attempted.
* Multiaddrs not supported by the available transports will be filtered out.
*
* @private
* @param {PeerId|Multiaddr|string} peer - A PeerId or Multiaddr
Expand Down Expand Up @@ -162,9 +163,12 @@ class Dialer {
resolvedAddrs.forEach(ra => addrs.push(ra))
}

// Multiaddrs not supported by the available transports will be filtered out.
const supportedAddrs = addrs.filter(a => this.transportManager.transportForMultiaddr(a))

return {
id: id.toB58String(),
addrs
addrs: supportedAddrs
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/core/listening.node.js
Expand Up @@ -45,7 +45,7 @@ describe('Listening', () => {
expect(addrs.length).to.be.at.least(2)
for (const addr of addrs) {
const opts = addr.toOptions()
expect(opts.family).to.equal('ipv4')
expect(opts.family).to.equal(4)
expect(opts.transport).to.equal('tcp')
expect(opts.host).to.match(/[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+/)
expect(opts.port).to.be.gt(0)
Expand Down
28 changes: 24 additions & 4 deletions test/dialing/direct.node.js
Expand Up @@ -98,8 +98,8 @@ describe('Dialing (direct, TCP)', () => {
const dialer = new Dialer({ transportManager: localTM, peerStore })

await expect(dialer.connectToPeer(unsupportedAddr))
.to.eventually.be.rejectedWith(AggregateError)
.and.to.have.nested.property('._errors[0].code', ErrorCodes.ERR_TRANSPORT_UNAVAILABLE)
.to.eventually.be.rejectedWith(Error)
.and.to.have.nested.property('.code', ErrorCodes.ERR_NO_VALID_ADDRESSES)
})

it('should fail to connect if peer has no known addresses', async () => {
Expand Down Expand Up @@ -139,8 +139,28 @@ describe('Dialing (direct, TCP)', () => {
const peerId = await PeerId.createFromJSON(Peers[0])

await expect(dialer.connectToPeer(peerId))
.to.eventually.be.rejectedWith(AggregateError)
.and.to.have.nested.property('._errors[0].code', ErrorCodes.ERR_TRANSPORT_UNAVAILABLE)
.to.eventually.be.rejectedWith(Error)
.and.to.have.nested.property('.code', ErrorCodes.ERR_NO_VALID_ADDRESSES)
})

it('should only try to connect to addresses supported by the transports configured', async () => {
const remoteAddrs = remoteTM.getAddrs()
const dialer = new Dialer({
transportManager: localTM,
peerStore: {
addressBook: {
add: () => { },
getMultiaddrsForPeer: () => [...remoteAddrs, unsupportedAddr]
}
}
})
const peerId = await PeerId.createFromJSON(Peers[0])

sinon.spy(localTM, 'dial')
const connection = await dialer.connectToPeer(peerId)
expect(localTM.dial.callCount).to.equal(remoteAddrs.length)
expect(connection).to.exist()
await connection.close()
})

it('should abort dials on queue task timeout', async () => {
Expand Down

0 comments on commit 06e8f3d

Please sign in to comment.