Improve StreamingHandlers contract

[ineiti]

The current RegisterStreamingHandlers is not ideal when it comes to implement a streaming service:

• every new client message calls the endpoint again
• the endpoint cannot return an error once the streaming started
• the contract between onet and the endpoint is not clear

A first, simplified method signature for a blocking endpoint: Endpoint(<-input, ->output) error

• arguments:
  • input-channel of messages from client
  • output-channel of messages from endpoint
• return:
  • error, which can be nil to indicate the endpoint finished its work and is done

Onet would do the following:

• setup the input- and output-channel
• call the endpoint for each new connection from a client using Client.Stream
  • the endpoint is blocking and listening on the input channel for new messages, and sending messages on the output channel
  • if the endpoint is done, it can return with a nil error to indicate successful termination
  • any non-nil error indicates an error by the endpoint
• send every message from the client through the input channel to the endpoint
• send every message from the output-channel of the endpoint to the client
• close the input-channel if the websocket is closed by the client
  • all further messages in the output-channel are ignored
  • any returned error from the endpoint is ignored
  • who should close the output-channel?
• close the connection if the endpoint returns
  • if the error is nil, the connection is closed without an error
  • if the error is not nil, the error is sent as the reason for closing the connection

Comments, @tharvik @cgrigis @nkcr ?

[tharvik]

Great idea, it's a bit hard to use currently. I'm adding some comments below.

A first, simplified change could be:

smth like Handler(inputs <-chan, outputs chan->, errors chan->)? I think that using smth like Handler(inputs <-chan, outputs chan->) error would be more idiomatic, so that one can use the classic if err != nil of golang without having to send something on the channel and then returning. Also, as zero-or-one message is expected on the error channel, no need to use a channel, nil and != nil has the same meaning.

• call the endpoint only once for each connection from a client

The client uses Client.Stream to call a streaming handler, is that what you mean by "each connection"?

• one of the following actions triggers onet to close the input-channel:

What happens if the handler finishes and the output channel isn't closed? (discard if you're thinking of Handler(<-chan) (chan->, chan->, error), which I personally don't like)

Some other remarks:

• onet needs to take care of all channel creation/closing, as to avoid missuses
• send/recv only channels are cool, and safer
• StreamingConn.ReadMessage is quite cumbersome to use, using a classic channel on the client might be easier

[ineiti]

• endpoint behaviour: when onet calls the endpoint, should it directly return (my first idea), or use an endless loop and only return if it's done (what I gather from your comment)? Probably the second one is better, so your comments about the error-channel make more sense.
• Error-channel: if the endpoint is blocking, your comment absolutely makes sense, and we don't need this channel
• endpoint signature: if the endpoint is blocking, this discussion is moot, and the only signature that makes sense is Handler(<-input, ->output) error

I'm updating the description

[ineiti]

To understand @tharvik's comments, here the original description:

The current RegisterStreamingHandlers is not ideal when it comes to implement a streaming service:

• every new client message calls the endpoint again
• the endpoint cannot return an error once the streaming started
• the contract between onet and the endpoint is not clear

A first, simplified change could be:

• input:
  • input-channel of messages from client
• output:
  • output-channel of messages from endpoint
  • error-channel from endpoint

Onet would do the following:

• call the endpoint only once for each connection from a client
• send every message from a client through the input channel
• send every message from the output-channel to the client
• send the first error from the endpoint to the client
• one of the following actions triggers onet to close the input-channel:
  • the websocket is closed by the client
  • the output-channel closes, plus onet will close the websocket
  • an error is received through the error-channel, plus onet will close the websocket with the given error