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

Fix/handshake error #101

Merged
merged 3 commits into from
Mar 28, 2018
Merged

Fix/handshake error #101

merged 3 commits into from
Mar 28, 2018

Conversation

jacobheun
Copy link
Contributor

Callback naming was inconsistent in some of the files so I cleaned those up to help make them easier to read and avoid any potential hoisting issues.

Errors in the handshake weren't being passed back to finish, so if I handshake failed early enough that a peer id was not attained, it would cause an Assertion error attempting to set the peer info of the encrypted connection.

fixes ipfs/js-ipfs#1221

Failed handshakes abort the connection but dont inform users of the failure properly. If a remote was not successfully negotiated this could cause a peerId missing issue
Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

Excellent debugging! Thanks so much for fixing this @jacobheun :)

Did you manage to verify that indeed the js-ipfs daemon doesn't crash with this daemon anymore?

@jacobheun
Copy link
Contributor Author

@diasdavid yes, I was able to run a daemon for the past two hours. It did end up crashing but it was due to ipfs/js-ipfs#1156.

I leveraged @victorbjelkholm 's repo mentioned in the ipfs/js-ipfs#1221 description to reproduce and verify stability.

I believe the root error occurs here, https://github.com/libp2p/js-libp2p-secio/blob/master/src/handshake/crypto.js#L74, as the remote id would not be set for the connection at this point, causing the ultimate peerId missing thrown error.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Node crashes with "Missing peerId" after a few minutes
2 participants