nim-libp2p icon indicating copy to clipboard operation
nim-libp2p copied to clipboard

Failed transport's `accept` are not `re-spawned` or handled in any way.

Open dryajov opened this issue 2 years ago • 3 comments

Currently, the switch will spawn a transport's accept which will run unattended. If the accept fails for any reason there is no way of notifying the calling application, nor does the switch attempt to re-spawn it. It would be nice if we could have more granular control over this flow as there might be cases where a switch ends up without any active accepts and is effectively unreachable.

dryajov avatar Dec 01 '21 16:12 dryajov

https://github.com/status-im/nim-libp2p/pull/658 for instance caused the WS accept loop to crash and thus ws was unreachable

There are multiple ways to look at it, but if you think of it like an asyncSpawn the loop should never fail, and if it does, it should be a defect basically

Now, is there some cases where the accept may legitimately fail and not be restarted, but the program continues? Probably, and we may need to find how to handle it. It kinda goes back to the runtime mounting/unmouting of transports you talked about

Menduist avatar Dec 01 '21 17:12 Menduist

There are multiple ways to look at it, but if you think of it like an asyncSpawn the loop should never fail, and if it does, it should be a defect basically

Yeah, I agree with this, what I'm saying is that we don't address this in any way. One way might be that we always try to re-spawn the accept after it exited, this sounds like an easy quick fix to mend things for now, but this does introduce it's own issues, for example if accept keeps failing what then, do we keep re-spawning indefinitely?

All in all, this sort of things are a bit hard to reason about in a "framework/library", how much do you expose to the user and how much do you try to handle internally? This are hard questions to the tune of the discussion in #662.

dryajov avatar Dec 01 '21 17:12 dryajov

It kinda goes back to the runtime mounting/unmouting of transports you talked about

Yeah, this to definitely. The case of dynamic addresses for mobile applications is yet another complexity that we'll need to address at some point.

dryajov avatar Dec 01 '21 17:12 dryajov