[request-response] Permit sending of responses directly on a `ResponseChannel`?

[romanb]

It has been brought up in #1639, that it would be nice if the API of libp2p-request-response would offer ResponseChannel::send(response) instead of RequestResponse::send_response(channel, response). The advantage is obviously that one does not need to have access to the RequestResponse behaviour in order to send responses, i.e. one can distribute the ResponseChannels received from inbound requests in the application and send responses "from anywhere". Since RequestResponse::send_response currently needs no access to any state of the RequestResponse behaviour (contrary to RequestResponse::send_request), this would be easy to do. The main concern is one of future-proofness, i.e. if RequestResponse::send_response should later need access to RequestResponse state, the change may potentially be not backward-compatible or at least require more effort (i.e. it may be possible to do such a change without breaking the public API if the ResponseChannel that the behaviour exposes contains another oneshot channel whose receiving end is kept by the RequestResponse behaviour; once resolved (i.e. the response is send) it would forward the response to the oneshot channel whose receiving end is in the InboundUpgrade). Thus, if we change the API to offer ResponseChannel::send(response) and later need access to RequestResponse state, that would require some more indirection or a breaking change to the API. This issue is about weighing the pros/cons of providing ResponseChannel::send(response) vs the loss of flexibility in how sending of responses is implemented internally and gathering opinions on this matter.

[thomaseizinger]

My 2 cents on this topic are that the current API is fine as it is.

• It encourages an application design in which accessing the network is a conscious act.
• Contrary to the arguments provided in RequestResponse updates #1639, I think it actually improves testability because - if designed correctly - the application can be completely unaware of the channel. Instead of forwarding it to other parts of the application for processing of the request, one can just process the request and hand the return value back into the channel, i.e.:

- incoming request (request, channel)
	- response = process (request)
	- respond(response, channel)

Admittedly, designs like these are not so easy to see and it also took me a while to wrap my head around that. I gets especially tricky if process is an async function because you can't await inside NetworkBehaviourEventProcess and spawning into a runtime will require you to Arc<Mutex> things. At least for me, a part of the confusion comes from the NetworkBehaviourEventProcess design. It encourages designs where parts of the application "live" inside the top-level NetworkBehaviour so that they can be accessed. This is kind of odd especially if the application has other APIs as well.

I think the hard part here is the knowledge about how to build concurrent systems in Rust that don't trick around &mut self with Arc<Mutex> too much but rather embrace it. Rust-libp2p itself has a really good design here and so do the underlying Futures of async/await. I feel like what we actually need is documentation and examples of how to integrate rust-libp2p with async/await properly.

This might be a bit of a long-shot from the original discussion topic but I feel like it is actually connected. In the end, the desire behind exposing the channel was to integrate with the remaining part of the application built around async/await.

[tomaka]

I do also believe that the code as it is encourages good programming practices.

I'm not against escape hatches, as sometimes they're extremely useful, but in my opinion the idiomatic way should remain to pass back the response to the behaviour.