fix: improve connection tracking #318
Conversation
ghost
assigned
| @@ -68,9 +68,6 @@ class IncomingConnectionFSM extends BaseConnection { | |||
| this.emit('muxed', this.conn) | |||
| }) | |||
| this._state.on('DISCONNECTING', () => { | |||
| if (this.theirPeerInfo) { | |||
| this.theirPeerInfo.disconnect() | |||
| } | |||
There was a problem hiding this comment.
This was unnecessary. Incoming connections don't cause the connect to happen until after the are muxed, which will now be handled by the switch.connection.add() logic.
| @@ -325,13 +319,13 @@ class ConnectionFSM extends BaseConnection { | |||
| return this.close(maybeUnexpectedEnd(err)) | |||
| } | |||
|
|
|||
| const conn = observeConnection(null, this.switch.crypto.tag, _conn, this.switch.observer) | |||
|
|
|||
| this.conn = this.switch.crypto.encrypt(this.ourPeerInfo.id, conn, this.theirPeerInfo.id, (err) => { | |||
There was a problem hiding this comment.
I found an edge case here, where if the connection was terminated while encryption was being negotiated, this.conn would get resolved and would later throw an error when secio went to set the inner connection. This avoids that.
| proxyConnection.setInnerConn(conn) | ||
| callback(null, proxyConnection) | ||
| conn.setPeerInfo(connection.theirPeerInfo) | ||
| callback(null, conn) |
There was a problem hiding this comment.
The proxyConnection was superfluous here, so I got rid of it.
jacobheun
changed the title
| connection.theirPeerInfo.disconnect() | ||
| this.switch.emit('peer-mux-closed', connection.theirPeerInfo) | ||
| return | ||
| } | ||
|
|
||
| for (let i = 0; i < this.connections[connection.theirB58Id].length; i++) { |
There was a problem hiding this comment.
I feel like the code in this class could be simpler with Set and Map - suggestion for a future PR
There was a problem hiding this comment.
I agree, a Map would allow for handling the two dimensional array pretty cleanly.
|
I left a couple of questions in the code but in general it seems like a much more reliable way of doing things to me 👍 |
|
LGTM 👍 |
Previously,
peer-mux-establishedandpeer-muxed-closedevents were being fired anytime a connection was created or closed. The problem with this is that if you happened to have 2 connections to a peer and only 1 of them closed, the closed event would fire, causing the connection manager to believe that peer no longer had any connections. This would result in the connection manager believing there were way fewer connections than actually existed.This PR moves the emit events and
PeerInfo.disconnectcalls into theswitch.connectionclass. When connections are added and removed it will check to see if there are any additional connections to that peer. This way, we only fire events when changes actually occur.