ssbc / ssb-gossip

Schedule connections randomly with a peerlist constructed from config, multicast UDP announcements, feed announcements, and API-calls
MIT License
10 stars 3 forks source link

peer.close(true) ? #15

Closed staltz closed 5 years ago

staltz commented 5 years ago

When the network interfaces change, gossip.reconnect is called, which in turn calls peer.close(true):

https://github.com/ssbc/ssb-gossip/blob/db2892c5b805812a67fb3878ec4d93988b427e86/index.js#L298

This will end up calling PacketStream.prototype.destroy(end = true), which ends up throwing the error as "unexpected end of parent stream", and this shows up in the logs as a problem.

Do we really want gossip.reconnect to call peer.close(true) or could it call peer.close()? I suppose we don't want gossip.reconnect to tell the logs and the user that there is a problem.

dominictarr commented 5 years ago

context: when I first wrote this stuff, I made muxrpc.close() gracefully close i.e. let all the streams close, then close... but I later realized that added complexity and wasn't that useful. It was a little useful for tests... but not really.

I think it's important that the apis get that error, because they were trying to do something, and it didn't happen.

I think the fix is just don't log it. that just makes people post issues asking if there is an error. ;)

staltz commented 5 years ago

Yes, and now in further tests I noticed that it's better to have that specific error showing, e.g. ssb-replicate filters for that specific "unexpected end of parent stream".