WebSockets represented as a BidiFlow not a Flow

[fommil]

This describes WebSockets far better than Flow:

http://doc.akka.io/docs/akka-stream-and-http-experimental/1.0/java/stream-graphs.html#Bidirectional_Flows

Can we please have a change in the WebSockets API to be a BidiFlow instead of a Flow? @jrudolph

Incidentally, this would really help with the marshalling because Out1/In2 are both in the user's domain model whereas Out2/In1 are both Message.

[rkuhn]

How could a WebSocket be a BidiFlow? It represents a single TCP connection, which means that it has exactly one input and exactly one output—hence it is topologically a Flow. There must be a misunderstanding at work here, could you please elaborate?

[fommil]

@rkuhn there is one input and one output on the client side, but there is also one input and one output (potentially independent) on the server side. WebSockets are actually more like TWO flows, not one. It is not typically used as a request/response protocol, which is what Flow is modelling.

Classic example of Websockets: a stock market ticker push, with client requests to add/remove subscriptions. This is also similar to how ENSIME behaves, and I can say with experience that hacking Flow to behave as independent source/sink on the server side was a real pain.

[fommil]

@rkuhn FYI, this is how we hacked it to work https://github.com/ensime/ensime-server/blob/master/server/src/main/scala/org/ensime/server/WebSocketBoilerplate.scala

[rkuhn]

Please take a step back and consider that the client needs its own representation of the end of the bidirectional WebSocket as well as the server does—this is in no way representable as a BidiFlow, though, because two openings are at the client machine and two of them are on the server machine. For each of these participants it looks like only one input and one output, nothing more. So, this ticket is clearly invalid.

What you show in your (needlessly emotionally commented) code example is just that you may of course want to represent the JSON (un)marshalling stage as a BidiFlow to be plugged between the real WebSocket ws (which indeed is a Flow) and the application code app (which again is a Flow—even if the reading and writing ports do not communicate, that is not mandated for Flows at all). Code:

val json = BidiFlow.wrap(
    Flow[Outgoing].map(o => TextMessage.Strict(o.toJson.toString(printer))),
    Flow[Message].collect { case TextMessage.Strict(m) => m.parseJson.convertTo[Incoming] }
  )
...
app.join(json).join(ws).run()

(no crazy boilerplate needed)

[fommil]

Yes, the separation of the marshalling is the main benefit of exposing this as a BidiFlow. Surely that is more fitting with the routing approach taken elsewhere such that an implicit marshaller takes care of user
domain <=> TextMessage mappings in much the same way as the ToMarshallable stuff works for HttpRequest / HttpResponse.

The comments are addressed to the members of the ensime team, to whom this file is a crazy amount of indecipherable boilerplate. Feel free to skip those words and move on. Also, pull requests welcome.

You can find a corresponding test for the file in https://github.com/ensime/ensime-server/blob/master/server/src/test/scala/org/ensime/server/WebSocketBoilerplateSpec.scala which also shows that if the endpoint were a BidiFlow then the testing would be significantly easier.

[rkuhn]

I showed you how to properly use a BidiFlow to factor out the marshalling stage, beyond that there is nothing about WebSockets that lends itself to this treatment. If you still are convinced that I am missing something then please be very explicit and write down the API method signatures that you want to see changed to BidiFlow (if you do this then you’ll really have to write them down here—saying “just change all” does not achieve the same effect, the onus is on you to prove that your proposal makes sense).

One more thing: if “the endpoint were a BidiFlow” then which four streams should be wired into it and by whom? An endpoint is at the end of something, not in the middle.

[fommil]

OK, fair enough from the exposed API it could remain as Flow, in which case you could use a BidiFlow internally to do the marshalling on the In1 / Out2 side as suggested... but this would be best in the framework (much as ToMarshallable is in the REST routes) rather than something the user has to do.

The BidiFlow wrapper you've given is useful for testing, thanks.

I'm happy to close as a dupe of #17969

[rkuhn]

Right, allowing nice usage of Marshallers in the routing directives for WebSockets is a good point. Thanks for the discussion!

[viktorklang]

@fommil It seems like this is not only the incorrect place for this discussion, but also a conflation of topics. Let me know if you want to continue in private. Thanks, Sam.

[viktorklang]

@fommil I haven't seen this issue prior to my very recent mention so unlikely :)

[fommil]

(Insert picture of man scratching head)