Review the design of the OneShotHandler disconnecting in case of protocol error

[tomaka]

When an error happens when upgrading an outbound request, the OneShotHandler straight up closes the connection.

Errors can happen because of a protocol error (e.g. multistream-select received bad data), in which case this behaviour makes sense.
But errors can also for a variety of reasons, such as if the requested protocol isn't supported by the remote, or the remote wants to send us a response that is too large, in which case closing the connection is too aggressive.

We should in my opinion instead propagate any error to the behaviour.

[tomaka]

Ok I'm stupid. This is actually how it's been designed, as it makes it possible for the protocol to force-close the connection. You are expected to pass a Result<Result<...>, _>. Otherwise there would also no way to know that an error has happened during a request.

[tomaka]

if the requested protocol isn't supported by the remote

This, however, is a legitimate case where you might not want to force-close the connection.

[romanb]

At the moment I'd lean towards deprecating the OneShotHandler and referring to request-response for implementing these use-cases. I'd be interested in second opinions about that.

[thomaseizinger]

At the moment I'd lean towards deprecating the OneShotHandler and referring to request-response for implementing these use-cases. I'd be interested in second opinions about that.

In my experience, rust-libp2p is the most ergonomic if things are composed on the NetworkBehaviour level. As such, the lack of a OneShotBehaviour on top of a OneShotHandler makes it not very attractive to use.

The problem with composing several ProtocolsHandler within a single NetworkBehaviour is that the type-signatures become quite clunky. When composing on the NetworkBehaviour level, this is hidden from the user through the NetworkBehaviour trait.

The separation into ProtocolsHandler and NetworkBehaviour still makes sense for stateful protocols, i.e. protocols that need to exchange multiple messages before they finish and emit an event to the NetworkBehaviour. However, for "oneshot" protocols, I've always wanted to go straight to the NetworkBehaviour level. The request-response abstraction makes this a lot easier now, so I guess the conclusion is that it is not really needed?

[wngr]

OneShotHandler is a handy building block for writing a custom NetworkBehaviour (which can be composed nicely as @thomaseizinger described above). I don't see the request-response protocol as a replacement for it -- it's another abstraction level in the usage I've experienced so far.

[thomaseizinger]

Closing in favor of #3591.