fix: prevent double dialing same peer

[dirkmc]

Fixes #62

[vasco-santos]

The implementation using a Set looks good to me!

However, I feel that we should not add the events for dialing, unless we have a concrete use case from users of this module.

[vasco-santos]

Left just a new note! It looks like we decreased some coverage, as well.

Can you add a test for the following scenario:

// stopped, so we should just bail out
if (!this._dials.has(idB58Str)) {
 return callback()
}

[dirkmc]

@vasco-santos it seems like maybe we're missing an error handler somewhere in the transport stack:
https://ci.ipfs.team/blue/organizations/jenkins/libp2p%2Fjs-libp2p-floodsub/detail/PR-63/5/pipeline/256
Do you have any ideas on how to fix?

[vasco-santos]

So, it seems it is failing on spdy-transport. @jacobheun can you have a look?

[jacobheun]

@vasco-santos I'll take a look, floodsub is using some pretty old deps. I'll get a PR together with any necessary changes. The error itself isn't a problem, but obviously we don't want bubbling unhandled exceptions.

[vasco-santos]

thanks @jacobheun

[vasco-santos]

@jacobheun already fixed the spdy issue ❤️

@dirkmc can you rebase the PR now?

[vasco-santos]

Thanks @dirkmc

There are a few lines added that are not covered by the tests, for instance, https://codecov.io/gh/libp2p/js-libp2p-floodsub/src/8f1e36f8ad0488cf50efaa8aba981c8153cf54f9/src/base.js#L112

Can you have a look?

[dirkmc]

Oh hmm that definitely should be covered, I'll take a look

Also thanks @jacobheun for upgrading everything 👍

[vasco-santos]

Great work @dirkmc !
LGTM

[dirkmc]

I have to say I'm impressed with the tooling you guys have put in place, it caught some very subtle cases where after Jacob's upgrade some of the test cases I had written were no longer testing what they appeared, even though they were still passing. Kudos 👍