Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add client support for websockets #330

Closed
jedesah opened this issue Jul 2, 2015 · 32 comments
Closed

Add client support for websockets #330

jedesah opened this issue Jul 2, 2015 · 32 comments
Labels
enhancement Feature requests and improvements module:client
Milestone

Comments

@jedesah
Copy link
Contributor

jedesah commented Jul 2, 2015

It is currently possible to implement a server that exposes a websocket endpoint. However we currently do not provide support for consuming websockets through the client.

@bryce-anderson bryce-anderson added the enhancement Feature requests and improvements label Jul 2, 2015
@bryce-anderson
Copy link
Member

As a start to getting this discussion rolling, is there a reasonable situation where a websocket client won't behave almost completely different than a regular client? Besides giving it a Url, and of course that fact that websocket is bootstrapped on a http request, it seems to me like they would be very different concepts.

@jedesah
Copy link
Contributor Author

jedesah commented Jul 2, 2015

Should the API look something like this?

val greeting = "greeting"
val resp = client(uri("ws://echo.websocket.org")).as[Exchange[WebSocketFrame, WebSocketFrame]].run

val msgStream: Process[Task, WebSocketFrame] = Process(Text(greeting), Close())

msgStream.to(resp.write).run.run
resp.read.runLog.run mustEqual List(greeting)

@bryce-anderson
Copy link
Member

As with the server, I think we want to give the client an Exchange so that the client can drive the execution. What you've posted looks to me like (no pun intended) a typical http POST request.

@jedesah
Copy link
Contributor Author

jedesah commented Jul 2, 2015

But isn't the initial connection a typical http request that gets upgraded?

I am not sure I understand in what way the above example code does not give the client an Exchange. It seems to me that resp is an Exchange with which the client can drive the execution of the websocket connection.

@bryce-anderson
Copy link
Member

In your example you've clearly transformed the body into an Exchange that will be stored in resp. That means its your job to hook it up and drive it as you've done with all the .run business below.

Under the covers a websocket request starts as a http request, but your example above will fail due to not including an Connection: upgrade header. That means we need to include special semantics for making a websocket request to begin with, and after dealing with their differences, at some point their similarities are superficial. Therefor, I'm not (yet) tying myself to the requirement that we make a websocket request in the exact same manner as a http request.

@jedesah
Copy link
Contributor Author

jedesah commented Jul 2, 2015

Okay, I understand your point better now.

Do you think it would be acceptable to add the headers Connection: upgrade and Upgrade: websocket if the URI being used contains "ws:" or "wss:"? Or is it better to have this happen more explicitly by providing an extra parameter to a call for a request? A WebSocketClient could also be an entirely separate class, but then some common logic might be duplicated such as timeout and such.

As for the Exchange, could you provide an example of it being "hooked up for you" (as a client). I don't see what there is to do more then provide a sink to write values too and a stream that represents values coming from the server.

@jedesah
Copy link
Contributor Author

jedesah commented Jul 2, 2015

Given that http4s-servlet does not appear to be it's own repo anymore, is there still a reason to have http4s-websocket as a separate repo?

For instance, isWebSocketRequest(headers: Traversable[(String, String)]) defined in http4s-websocket takes a Traversable[(String, String)] instead of Headers, presumably to avoid a circular dependency.

I am also concerned about potentially needing to modify http4s-websocket to add support for the client and having that be a bad workflow.

@rossabaker
Copy link
Member

blaze, not to be confused with http4s-blaze-core, uses http4s-websocket. It was spun off from blaze so http4s-servlet can eventually use it.

@jedesah
Copy link
Contributor Author

jedesah commented Jul 2, 2015

Okay, I am a little confused, but if it's necessary. It's unfortunate though that we need to dumb down the APIs.

@rossabaker
Copy link
Member

Think of blaze and http4s-websocket as completely unaware of http4s. (Which makes the "http4s-websocket" name pretty unfortunate. Hmm.) Things that make sense for websockets as a wire protocol belong in http4s-websocket. Things that make sense for websockets in conjuncton with the http4s model belong in http4s-core. API for the client and server belong in http4s-client and http4s-server, respectively.

So:

  1. I wouldn't be surprised if some things move from http4s-server into http4s-core.
  2. If http4s-websocket has limitations, we can fix it. But that's not the place for scalaz-stream, our header model, etc.

@jedesah
Copy link
Contributor Author

jedesah commented Jul 3, 2015

@rossabaker That makes sense. I would argue though that if websockets deserves it's own library that defines websocket concepts in a very abstract way, that it might make sense for HTTP to get the same treatment, a library that defines things like headers, response codes, media formats, etc in an abstract way, with nothing to do with scalaz-stream and such. And if that were the case, it might make sense to have websockets depend on http.

@jedesah
Copy link
Contributor Author

jedesah commented Jul 6, 2015

@bryce-anderson I re-read your comment this morning (way above) about the API "running" the Exchange for the client vs manual hookup. I get what you mean now, no need to provide an example, sorry for being slow.

Maybe that is the right approach, but I am not convinced. Such an approach would suffer from the same problem that the server side API for websocket suffers, that is, the necessity for the user to instantiate a queue to accomplish simple things (which I continue to believe is undesirable).

Let's say we have the following:

val mgsToSend: Process[Task, WebSocketFrame] = ??? // messages that need to be sent to the server
def doSomething(receivedMsgs: Process[Task, WebSocketFrame]): Task[_] = ??? // going to accomplish something with a stream of responses from the server

My belief is the above scenario should be fairly common.

This is what it looks like with an API that requires supplying an Exchange[WebSocketFrame, WebSocketFrame]:

val q = async.unboundedQueue[WebSocketFrame]
val exchange = Exchange(mgsToSend, q.enqueue)
val receivedMsgs = q.dequeue
val conn: Task[Unit] = websocket.connect(uri,exchange)
val handle: Task[_] = doSomething(receiveMsgs)
val result = conn *> handle
result.run

This is what it looks like if the API provides an Exchange[WebSocketFrame] instead:

val resp:Task[Exchange[WebSocketFrame, WebSocketFrame]]]  = websocket.connect(uri)

val result = resp.flatMap { ex =>
    val send = mgsToSend.to(ex.write).run
    val receive = doSomething(ex.read)
    Applicative[Task].apply2(Task.fork(send), receive)(???)
}
result.run

Does this seem reasonable? This was partly an exercise for myself but I hope it is useful to the discussion.

@jedesah
Copy link
Contributor Author

jedesah commented Jul 6, 2015

On another note, to answer your original question @bryce-anderson, it is very true that a websocket client does not appear to share all that many things with a regular HTTP client. That being said, I think it would be nice to share the functionality around connection pooling if at all possible.

@mericano1
Copy link

Just reading up on this, I was wondering what is the best way to consume a webSocket from an http4s client right now?

@rossabaker
Copy link
Member

There isn't currently a way to do so. I started something in #618, but haven't made time to finish it. Perhaps someone would like to pick that up and run with it?

@loicdescotte
Copy link

It could be related to #1051
For example, one could consume a stream in http4s, from a websocket connection using Monix, like we can do with Play and Akka Stream.

@AnirudhVyas
Copy link

is this nearing completion by any chance?

@rkrzewski
Copy link

@AnirudhVyas today I've learned that in fact there is a WebSocket client in http4s project: https://jdk-http-client.http4s.org/latest/index.html#websocket-client

@joan38
Copy link
Contributor

joan38 commented Jul 25, 2020

Yes, we are using it in kubernetes-client.
Please feel free to have a look at how @novakov-alexey moved us out of akka-http: joan38/kubernetes-client#20
There is 1 caveat tho to this: You raise the min JVM requirement to 11.

@rossabaker
Copy link
Member

I haven't heard objections to the existing client interface. It would be great to get that moved into core and a second implementation, if someone is looking for a medium-effort contribution.

@nelusnegur
Copy link
Contributor

@rossabaker I would like to take this, if it's not urgent.

@rossabaker
Copy link
Member

@nelusnegur Do you have a backend in mind that you'd like to try to implement it on? AHC, okhttp, and Jetty all have their own models for it. Ember would be pure FP, but have more from-scratch coding. Blaze has the same drawbacks as Ember, and also gets you into Futureland.

@nelusnegur
Copy link
Contributor

nelusnegur commented Aug 11, 2020

@rossabaker I haven't thought about it yet, but from your short summary, I would prefer Ember (I'll have a longer journey, but a more interesting one).
Also, should we preserve the same public interface as it is now, or something could be changed? And where the implementation should be placed, in ember-server and client respectively or somewhere else?

@rossabaker
Copy link
Member

@amesgen has done most of the work on web sockets and @ChristopherDavenport on Ember, so I suggest we invite their wisdom.

I would think any client interface belongs in org.http4s.client, and any model we share generically between client and server could go in org.http4s.websocket in http4s-core. And then implementations would go in ember-server and ember-client.

You might want to focus on client support first, since that's been in high demand for a while, and there's currently no option before JDK11. Then server support could be added to ember in a followup.

@ChristopherDavenport
Copy link
Member

Websocket is sorta an upgrade protocol where its just never released right? In which case you have the socket, so if we know its a websocket, then should be pretty operable.

The client is definitely simple, as you can almost immediately pull out a connection and then turn that into the protocol fairly independently of the rest of the flow, steal the internals as helpers as necessary. My knowledge of websockets is the part thats not super helpful here. Buts once you have a socket everything is fairly straightforward byte protocols.

@rossabaker
Copy link
Member

I can't remember without looking whether a web socket connection can be returned to the pool for HTTP after the close handshake, or whether a termination of the connection is part of the protocol. But implementing as the latter would at least get things off the ground, I think.

@ChristopherDavenport
Copy link
Member

https://github.com/http4s/http4s/blob/master/ember-client/src/main/scala/org/http4s/ember/client/internal/ClientHelpers.scala#L52 - This is presently how we extract out a request socket.

It forms the basis of its extraction from the pool inside the builder.
https://github.com/http4s/http4s/blob/master/ember-client/src/main/scala/org/http4s/ember/client/EmberClientBuilder.scala#L136

Then its actually just the client operating with that taken resource which is a RequestKeySocket, which is just a socket with a request key so we know how to reuse.

https://github.com/http4s/http4s/blob/master/ember-client/src/main/scala/org/http4s/ember/client/EmberClientBuilder.scala#L161

So if we need to change creation that would be that first helper, but otherwise you then have a socket and can implement a protocol fully independently. The client is straight forward. Get connection from pool, log state, run the request through the request method which does all the work, so however the discriminator works could just operate fully seperately right there.

@nelusnegur
Copy link
Contributor

Thank you all very much for the explanations! That was really helpful. I'll try to implement the client first, and then we can move forward to server side part.

@nelusnegur
Copy link
Contributor

Hi. I wanted to let you know that I'm not able to work on this issue anymore. I thought I'll have time during the weekends, but due to some personal circumstances, currently I cannot find the necessary time for it. I'm really sorry about this. Thank you so much for the opportunity to contribute!

@rossabaker
Copy link
Member

Thanks for the update. I hope things turn out well, and if you have time again in the future, I'm sure we'll have more work!

@rossabaker
Copy link
Member

Let's at least assure ourselves we can do this in a non-breaking way before going 1.0.

This is one of the highest value tickets on the board.

@rossabaker rossabaker added this to the 1.0 milestone Apr 5, 2021
@armanbilge
Copy link
Member

armanbilge commented May 1, 2022

Done in #5520.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests and improvements module:client
Projects
None yet
Development

No branches or pull requests