Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dont emit peer:disconnect when a connection is still open #301

Closed
jacobheun opened this issue Dec 19, 2018 · 1 comment
Closed

Dont emit peer:disconnect when a connection is still open #301

jacobheun opened this issue Dec 19, 2018 · 1 comment
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up

Comments

@jacobheun
Copy link
Contributor

jacobheun commented Dec 19, 2018

Currently the peer:disconnect event will be fired anytime a muxed connection to a peer is closed, https://github.com/libp2p/js-libp2p/blob/v0.24.3/src/index.js#L85. However, since it's possible to have an outbound and inbound connection to a peer, closing one of those still means we are connected to a that peer. Only when we no longer have a connection to a peer, should we emit the disconnect event.

Performing a check of the total connections for a given peer before emitting the disconnect event when we receive a muxed connection closed event, would enable us to only emit the event when all connections are closed.

Reference #299 (comment)

@jacobheun jacobheun added kind/bug A bug in existing code (including security flaws) help wanted Seeking public contribution on this issue exp/novice Someone with a little familiarity can pick up status/ready Ready to be worked P2 Medium: Good to have, but can wait until someone steps up labels Dec 19, 2018
@jacobheun
Copy link
Contributor Author

The latest switch handles this in it's updated connection tracking logic, https://github.com/libp2p/js-libp2p-switch/blob/v0.42.11/src/connection/manager.js#L86.

@ghost ghost removed the status/ready Ready to be worked label May 17, 2019
maschad pushed a commit to maschad/js-libp2p that referenced this issue Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant