refactor: update network to use async libp2p #206
Conversation
jacobheun
force-pushed
the
refactor/libp2p-async
branch
from
00ff648 to
c8593bb
Compare
jacobheun
changed the title
|
This is still pending the release of libp2p 0.27, but should otherwise be good to review. I refactored some things that were related to the libp2p change, but tried to avoid touching too much. I wouldn't mind helping do some more refactoring as a followup PR (after the new year). |
package.json
Outdated
| "just-debounce-it": "^1.1.0", | ||
| "moving-average": "^1.0.0", | ||
| "multicodec": "~0.5.0", | ||
| "multihashing-async": "^0.8.0", | ||
| "protons": "^1.0.1", | ||
| "pull-length-prefixed": "^1.3.1", | ||
| "pull-stream": "^3.6.9", |
There was a problem hiding this comment.
Can we remove this and callbackify?
src/network.js
Outdated
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
try/catch around this pipeline? I think the remote can send a stream reset message that will throw.
There was a problem hiding this comment.
Yes, good catch, added.
| this._log('sendMessage to %s', stringId, msg) | ||
|
|
||
| const { conn, protocol } = await this._dialPeer(peer) | ||
| const { stream, protocol } = await this._dialPeer(peer) |
There was a problem hiding this comment.
It shouldn't need one because sendMessage already throws. There are catches in both places sendMessage is used.
src/network.js
Outdated
| const message = await Message.deserialize(data.slice()) | ||
| this.bitswap._receiveMessage(connection.remotePeer, message) | ||
| } catch (err) { | ||
| this.bitswap._receiveError(err) |
There was a problem hiding this comment.
Do we want to continue receiving or return?
There was a problem hiding this comment.
To be the same as the existing code it should end iteration, you're right. I added a break to exit iteration to keep it consistent with the existing functionality.
There was a problem hiding this comment.
Awesome! Thanks @jacobheun, and congrats on the libp2p changes, it makes the calling code so much nicer. Thanks for refactoring some of the gnarlier code in here, I'd love to have further refactors where you can see improvements in future.
|
@jacobheun please let me know when libp2p is released and I will update the dependency and merge |
|
@dirkmc will do, we just have two outstanding docs PRs, libp2p/js-libp2p#514 and libp2p/js-libp2p#508. Once those are merged I will do an RC and follow up shortly after that with a release (potentially tomorrow, pending reviews). |
|
Cool thank you, no rush ❤️ |
jacobheun
force-pushed
the
refactor/libp2p-async
branch
from
3af509a to
51209da
Compare
|
@dirkmc I've updated libp2p, this should be all set. |
Migrating to async libp2p.
Note: This is currently using the prerelease