Http client websocket support #1895

benzwreck · 2023-07-12T07:27:14Z

Before submitting pull request:

Check if the project compiles by running sbt compile
Verify docs compilation by running sbt compileDocs
Check if tests pass by running sbt test
Format code by running sbt scalafmt

adamw · 2023-07-18T16:00:59Z

core/src/main/scalajvm/sttp/client4/DefaultSyncBackend.scala


 object DefaultSyncBackend {

 /** Creates a default synchronous backend with the given `options`, which is currently based on
 * [[HttpClientSyncBackend]].
 */
- def apply(options: BackendOptions = BackendOptions.Default): SyncBackend =
+ def apply(options: BackendOptions = BackendOptions.Default): WebSocketBackend[Identity] =


This should return a dedicated subtype, WebSocketSyncBackend; that's what #1766 is about

(the goal is to expose to the user "clean" signatures, without the Identity wrapper)

adamw · 2023-07-18T16:05:57Z

docs/testing.md

@@ -230,7 +230,7 @@ It is also possible to create a stub backend which delegates calls to another (p

 ```scala mdoc:compile-only
 val testingBackend =
- SyncBackendStub.withFallback(DefaultSyncBackend())
+ WebSocketBackendStub.withFallback(DefaultSyncBackend())


I think we can keep SyncBackendStub here

adamw · 2023-07-18T16:14:07Z

core/src/main/scalajvm/sttp/client4/internal/httpclient/WebSocketImpl.scala

+}
+
+private[client4] object WebSocketImpl {
+ def sync[F[_]](


I think we still need a sequencer for the sync version - a sync websocket can still be used from multiple threads. I think using a semaphore as in FutureSequencer should work here (but in a sync version, of course)

adamw · 2023-07-18T16:14:22Z

This is going in the right direction :)

adamw · 2023-07-21T08:16:18Z

core/src/main/scalajvm/sttp/client4/internal/httpclient/IdSequencer.scala

+ semaphore.acquire()
+ }
+ val result = t
+ semaphore.release()


the release should be in a finally block

adamw · 2023-07-21T08:22:24Z

core/src/main/scala/sttp/client4/backend.scala

@@ -52,6 +52,11 @@ trait SyncBackend extends Backend[Identity] {
 override def monad: MonadError[Identity] = IdMonad
 }

+/** A [[GenericBackend]] which is synchronous (side effects are run directly), and supports web sockets. */
+trait WebSocketSyncBackend extends SyncBackend with WebSocketBackend[Identity] {


I think we should have an override for send in WebSocketRequest with the simplified signature:

def send(backend: WebSocketSyncBackend): Response[T] = backend.send(this)

adamw · 2023-07-21T08:23:40Z

examples/src/main/scala/sttp/client4/examples/GetRawResponseBodySynchronous.scala

@@ -13,7 +13,7 @@ object GetRawResponseBodySynchronous extends App {
 .get(uri"https://httpbin.org/get")
 .response(asBoth(asJson[HttpBinResponse], asStringAlways))

- val backend: SyncBackend = HttpClientSyncBackend()
+ val backend: WebSocketSyncBackend = HttpClientSyncBackend()


I think in the non-websocket examples we could leave SyncBackend, since it's a super-trait? We don't need the ws capability here, so maybe it's simpler when it's typed this way?

adamw · 2023-07-21T08:37:31Z

core/src/main/scalajvm/sttp/client4/internal/httpclient/WebSocketImpl.scala

+ new WebSocketImpl[F](ws, queue, _isOpen, sequencer) {
+ override implicit def monad: MonadError[F] = _monad
+
+ override protected[client4] def fromCompletableFuture(cf: CompletableFuture[JWebSocket]): F[Unit] =


[minor] maybe we should simply make fromCompletableFuture a parameter (of type CF => F[Unit]), instead of a method?

adamw · 2023-07-21T08:39:33Z

core/src/main/scalajvm/sttp/client4/httpclient/HttpClientSyncBackend.scala

+ sequencer: Sequencer[Identity]
+ ): Identity[Response[T]] = {
+ val isOpen: AtomicBoolean = new AtomicBoolean(false)
+ val responseCell = new ArrayBlockingQueue[Either[Throwable, Future[Response[T]]]](5)


I wonder why this is 5 ... wouldn't 1 work as well?

adamw · 2023-07-24T08:44:59Z

docs/conf/redirects.md

@@ -36,8 +36,8 @@ You can disable the stripping of all sensitive headers using the following code:
 import sttp.client4._
 import sttp.client4.wrappers.{FollowRedirectsBackend, FollowRedirectsConfig}

-val myBackend: SyncBackend = DefaultSyncBackend()
-val backend: SyncBackend = FollowRedirectsBackend(
+val myBackend: WebSocketSyncBackend = DefaultSyncBackend()


let's use SyncBackend here, since we don't need web socket functionality

adamw · 2023-07-24T08:45:05Z

docs/json.md

@@ -46,7 +46,7 @@ Response can be parsed into json using `asJson[T]`, provided there's an implicit
 import sttp.client4._
 import sttp.client4.circe._

-val backend: SyncBackend = DefaultSyncBackend()
+val backend: WebSocketSyncBackend = DefaultSyncBackend()


adamw · 2023-07-24T08:45:41Z

docs/requests/authentication.md

@@ -32,7 +32,7 @@ In order to add digest authentication support just wrap other backend as follows
 ```scala mdoc:compile-only
 import sttp.client4.wrappers.DigestAuthenticationBackend

-val myBackend: SyncBackend = DefaultSyncBackend()
+val myBackend: WebSocketSyncBackend = DefaultSyncBackend()


and here :)

adamw · 2023-07-24T08:45:54Z

core/src/main/scalajvm/sttp/client4/internal/httpclient/IdSequencer.scala

+ blocking {
+ semaphore.acquire()
+ }
+ val result = t


this can be simplified: the result is not needed

Hmm, but t is a by-name parameter so I guess we need to run it before returning. Otherwise how would that method look like?

blocking { semaphore.acquire() } semaphore.release() t

? Cause we need to return T here.

t will be evaluted each time it's referenced in code

so:

blocking { semaphore.acquire() } try t finally semaphore.release()

Okay, I get it :) Thank you!

adamw · 2023-07-24T08:46:13Z

core/src/main/scalajvm/sttp/client4/internal/httpclient/IdSequencer.scala

+private[client4] class IdSequencer extends Sequencer[Identity] {
+ private val semaphore = new Semaphore(1)
+
+ def apply[T](t: => Identity[T]): Identity[T] =


do we need the Identity in types, wouldn't def apply[T](t: => T): T work?

adamw · 2023-07-24T08:48:46Z

core/src/main/scalajvm/sttp/client4/httpclient/HttpClientBackend.scala

@@ -103,8 +127,46 @@ abstract class HttpClientBackend[F[_], S, P, B](
 monad.map(body)(Response(_, code, "", headers, Nil, request.onlyMetadata))
 }

+ protected def createSimpleQueue[T]: F[SimpleQueue[F, T]]


do these methods need to be in the super-class? are they used here at all?

this is needed in WebSocket implementation and used by all classes in httpclient package

yes, but does it need to be declared in HttpClientBackend? Is there any common code that uses this method?

It was declared in HttpClientAsyncBackend before because it was only needed in async implementation (HttpClientFutureBackend). I thought it would be a good idea to move it to HttpClientBackend since it is used in HttpClientSyncBackend which implements HttpClientBackend and HttpClientFutureBackend which implements HttpClientAsyncBackend which implements HttpClientBackend.
So from my point of view it is not a bad idea to have it here. WDYT?

from the point of view of the HttpCLientBackend class, this method is not used, and hence an implement detail of child classes - so it shouldn't be declared there

I can see your point and it makes sense to me right now. Thanks for the explanation :)

adamw · 2023-07-24T08:50:00Z

core/src/main/scalajvm/sttp/client4/httpclient/HttpClientBackend.scala

 protected def standardEncoding: (B, String) => B

+ private[client4] def prepareWebSocketBuilder[T](


maybe this can be protected, as it's only used from subclasses?

adamw · 2023-07-24T08:52:17Z

core/src/main/scalajvm/sttp/client4/internal/httpclient/WebSocketImpl.scala

 f match {
 case WebSocketFrame.Text(payload, finalFragment, _) =>
- fromCompletableFuture(ws.sendText(payload, finalFragment))
+ handleWS(ws.sendText(payload, finalFragment))


I don't think the name is right here, this method doesn't "handle the websocket" - it just converts the future to the effect, ignoring the result?

I changed the name to fromCompletableFutureToEffect. Couldn't figured out a better one :/

adamw · 2023-07-24T08:56:06Z

core/src/main/scalajvm/sttp/client4/httpclient/HttpClientSyncBackend.scala

+ }
+ )
+ val baseResponse = Response((), StatusCode.SwitchingProtocols, "", Nil, Nil, request.onlyMetadata)
+ val body = Future(blocking(bodyFromHttpClient(Right(webSocket), request.response, baseResponse)))


since we're in a blocking world I think we can simply do Future.successfull(bodyFrom...)

benzwreck added 4 commits July 11, 2023 20:08

wip

a696eae

wip

69af980

POC implementation

ec1a42a

Fix ExecutionContext explicit type definition

0a61173

benzwreck requested review from adamw and kciesielski July 12, 2023 07:27

This was linked to issues Jul 12, 2023

Add WebSocket support to HttpClientSyncBackend #1767

Closed

Add a WebSocketSyncBackend type #1766

Closed

benzwreck added 7 commits July 13, 2023 15:39

Unify common interface for sync and async

8e68e47

Fix tests file structure

0bbe1dc

Refactor WebSocketImpl

9b3ae6b

Refactor WebSocket Builder preparation

9af5780

Fix docs

5060120

Fix docs

343295d

Fix docs v3

542a0f3

benzwreck marked this pull request as ready for review July 17, 2023 09:47

Fix docs v4

ab397b2

adamw reviewed Jul 18, 2023

View reviewed changes

benzwreck added 3 commits July 21, 2023 07:49

Add WebSocketSyncBackend type and use sequencer in synchronous client

e9be420

Formatting

7682885

Change backend signature in tests

fa01001

benzwreck requested a review from adamw July 21, 2023 07:23

adamw reviewed Jul 21, 2023

View reviewed changes

benzwreck added 3 commits July 23, 2023 14:06

More fixes

087f18e

Update docs

268341b

Fix tests

a54a6ae

benzwreck requested a review from adamw July 24, 2023 07:54

adamw reviewed Jul 24, 2023

View reviewed changes

benzwreck and others added 8 commits July 24, 2023 17:09

Even more fixes

0e86ae1

Rever to future blocking

fd55305

Code cleanup

d835ffc

Reading the ws body is not blocking

b2f38ee

Simplify by removing Future usage

db17655

Synchronous WS example

f049874

Docs

cecf5ef

Docs

5bc017e

adamw merged commit d5a42e8 into master Jul 27, 2023
12 of 15 checks passed

mergify bot deleted the http-client-websocket-support branch July 27, 2023 10:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Http client websocket support #1895

Http client websocket support #1895

benzwreck commented Jul 12, 2023 •

edited

Loading

adamw Jul 18, 2023

adamw Jul 18, 2023

adamw Jul 18, 2023

adamw Jul 18, 2023

adamw commented Jul 18, 2023

adamw Jul 21, 2023

adamw Jul 21, 2023

adamw Jul 21, 2023

adamw Jul 21, 2023

adamw Jul 21, 2023

adamw Jul 24, 2023

adamw Jul 24, 2023

adamw Jul 24, 2023

adamw Jul 24, 2023

benzwreck Jul 24, 2023

adamw Jul 24, 2023

benzwreck Jul 24, 2023

adamw Jul 24, 2023

adamw Jul 24, 2023

benzwreck Jul 24, 2023

adamw Jul 24, 2023

benzwreck Jul 24, 2023

adamw Jul 24, 2023

benzwreck Jul 24, 2023

adamw Jul 24, 2023

adamw Jul 24, 2023

benzwreck Jul 24, 2023

adamw Jul 24, 2023

		protected def standardEncoding: (B, String) => B

		private[client4] def prepareWebSocketBuilder[T](

Http client websocket support #1895

Http client websocket support #1895

Conversation

benzwreck commented Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamw commented Jul 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benzwreck commented Jul 12, 2023 •

edited

Loading