[Proposal] Add type hierarchy of requests and backends

[adpi2]

Following the discussion in #1607, I started experimenting with sttp, to see if it was possible to get a more user-friendly API while preserving the full power and flexibility. I did not try to maintain the binary-compatibility.

I pushed the experiment into a fully working implementation of sttp, which is, in my opinion, a decent proposal of its next client4 API. I submit it to you to get your feedback and to decide if we should continue working in this direction.

As you may know, we are currently building the first version of the Scala Toolkit (as announced by @odersky in the Simply Scala talk and by @romanowski in the Scala with Batteries-included talk). I think it would make sense to include this new API of sttp in the Toolkit. It could be a milestone version, if not a stable one, but we first need to decide if we want to go forward with this proposal.

Description

Requests

The single type RequestT[U[_], T, -R] is replaced by a hierarchy of classes:

To avoid duplication of the common builder methods, I also introduced the PartialRequestBuilder and RequestBuilder traits. They should only be used internally and never appear in user code.

This hierarchy of types is defined in core in this commit and applied downstream in this commit. It adds a total of 356 lines of code, mostly scaladoc comments, to document the different types of requests.

Backends

Similarly, the single type SttpBackend[F[_], +R] is replaced by a hierarchy of traits:

Each trait defines a concrete send method, implemented by calling the same internalSend method which is defined in AbstractBackend:

trait Backend[F[_]] extends AbstractBackend[F, Any] {
  def send[T](request: Request[T]): F[Response[T]] = internalSend(request)
}

trait SyncBackend extends Backend[Identity] {
  override def send[T](request: Request[T]): Response[T] = internalSend(request)
}

trait WebSocketBackend[F[_]] extends Backend[F] with AbstractBackend[F, WebSockets] {
  def send[T](request: WebSocketRequest[F, T]): F[Response[T]] = internalSend(request)
}

trait StreamBackend[F[_], +S] extends Backend[F] with AbstractBackend[F, S] {
  def send[T](request: StreamRequest[T, S with Effect[F]]): F[Response[T]] = internalSend(request)
}

trait WebSocketStreamBackend[F[_], S]
    extends WebSocketBackend[F]
    with StreamBackend[F, S]
    with AbstractBackend[F, S with WebSockets] {
  def send[T](request: WebSocketStreamRequest[T, S]): F[Response[T]] = internalSend(request)
}

To implement a concrete backend one must implement the internalSend method.

Example:

class HttpClientFutureBackend() extends WebSocketBackend[Future] {
  override def internalSend[T](request: AbstractRequest[T, WebSockets with Effect[Future]]): Future[Response[T]] = ???
}

This new hierarchy of backends is defined in core in this commit and applied downstream in this commit. It adds a total of 639 lines of code, mostly made of overload constructors of delegate backends (see section in Drawbacks and limitations below about delegate backends).

Motivation

Less abstraction in inferred types

The level of abstraction of the inferred types is closer to the expectation of the user. There is no RequestT[Identity, T, Any] or SttpBackend[Identity, Any] exposed to the user anymore. It leads to less confusion for beginners, but also more conciseness and better readability for all users.

Some examples on the types of requests as inferred by the compiler (the comments contain the former inferred types):

// val request: RequestT[Identity, String, Any]
val  request: Request[String] =
  basicRequest.get(uri"...").response(asStringAlways)

// val request: RequestT[Identity, String, Any with Effect[Future] with WebSockets]
val wsRequest: WebSocketRequest[Future, String] =
  basicRequest
    .get(uri"")
    .response(asWebSocketAlways(_ => Future.successful("Hello World")))

Another example, on the response of a synchronous backend:

// val syncBackend: SttpBackend[Identity,Any]
val syncBackend: SyncBackend =
  HttpClientSyncBackend()

// val response: Identity[Response[String]] =
val response: Response[String] =
  syncBackend.send(request)

Discoverability

One can use autocompletion, to discover the capababilities of a backend or a request.

Using the proposed API

// autocompleting HttpClientFutureBackend().send returns
send[T](request: Request[T]): Future[Response[T]]
send[T](request: WebSocketRequest[Future, T]): Future[Response[T]]

// autocompleting basicRequest.get(uri"...").send returns
def send(backend: SyncBackend): Response[T]
def send[F[_]](backend: Backend[F]): F[Response[T]]

Using the former API:

// autocompleting HttpClientFutureBackend().send returns
send[T, R >: WebSockets with Effect[Future]](request: Request[T, R]): Future[Response[T]]

// autocompleting basicRequest.get(uri"...").send returns
def send[F[_], P](backend: SttpBackend[F, P])(implicit isIdInRequest: IsIdInRequest[U], pEffectFIsR: P with Effect[F] <:< R): F[Response[T]]

Error Messages

We generally get better error messages:

basicRequest.send(syncBackend)
//^^^^^^^^^^^^^^^
//  value send is not a member of sttp.client3.PartialRequest[Either[String,String]]
//  did you mean head?

syncBackend.send(basicRequest)
//               ^^^^^^^^^^^^
// type mismatch;
// found   : sttp.client3.PartialRequest[Either[String,String]]
// required: sttp.client3.Request[?]

wsRequest.send(syncBackend)
//             ^^^^^^^^^^^
// type mismatch;
// found   : sttp.client3.SyncBackend
// required: sttp.client3.WebSocketBackend[scala.concurrent.Future]

syncBackend.send(wsRequest)
//               ^^^^^^^^^
// type mismatch;
// found   : sttp.client3.WebSocketRequest[scala.concurrent.Future,String]
// required: sttp.client3.Request[?]

Formerly it was:

// this error message comes from the @implicitNotFound of type IsIdInRequest[U[_]]
basicRequest.send(syncBackend)
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^
//  This is a partial request, the method & url are not specified. Use .get(...), .post(...) etc. to obtain a non-partial request.

syncBackend.send(basicRequest)
// type mismatch;
// found   : sttp.client3.RequestT[sttp.client3.Empty,Either[String,String],Any]
// required: sttp.client3.Request[?,?]
//    (which expands to)  sttp.client3.RequestT[sttp.client3.Identity,?,?]

wsRequest.send(syncBackend)
//^^^^^^^^^^^^^^^^^^^^^^^^^
// Cannot prove that Any with sttp.capabilities.Effect[[X]sttp.client3.Identity[X]] <:< sttp.capabilities.Effect[scala.concurrent.Future] with sttp.capabilities.WebSockets

syncBackend.send(wsRequest)
//^^^^^^^^^^^^^^ ^^^^^^^^^
// inferred type arguments [String,sttp.capabilities.Effect[scala.concurrent.Future] with sttp.capabilities.WebSockets] do not conform to method send's type parameter bounds [T,R >: Any with sttp.capabilities.Effect[sttp.client3.Identity]]
//
// type mismatch;
// found   : sttp.client3.RequestT[sttp.client3.Identity,String,sttp.capabilities.Effect[scala.concurrent.Future] with sttp.capabilities.WebSockets]
// required: sttp.client3.Request[T,R]
//    (which expands to)  sttp.client3.RequestT[sttp.client3.Identity,T,R]

Specialized Documentation

On each types of requests or backends we can specialize the scaladoc documentation to clearly state which concrete backend can send which request.

For instance on the scaladoc of WebSocketRequest, we can list all the backends that support websockets:

To send a WebSocketRequest, on the JVM you can use the HttpClientFutureBackend, or on Scala.js you can use the FetchBackend.
See the full list of websocket backends on https://sttp.softwaremill.com/en/latest/backends/summary.html

No separate synchronous-only API

The proposed API contains a SyncBackend that can send any request of type Request[T].

We don't need a separate synchronous-only API and the current SimpleHttpClient may not be useful anymore.

Drawback and limitations

Overload constructors of delegate backend

Since we now have 5 types of backends, we now need 5 overloads of each delegate backend. (A delegate backend is invariant on the type of backend that it encapsulates)

This is an example of how I implemented it:

abstract class FollowRedirectsBackend[F[_], P] private (
    delegate: AbstractBackend[F, P],
    config: FollowRedirectsConfig
) extends DelegateSttpBackend(delegate) {
  override def internalSend[T](request: AbstractRequest[T, P with Effect[F]]): F[Response[T]] =
    ??? // same implementation as before
}

object FollowRedirectsBackend {
  // one overload for each types of backends
  def apply(delegate: SyncBackend): SyncBackend =
    new FollowRedirectsBackend(delegate) with SyncBackend {}

  def apply[F[_]](delegate: Backend[F]): Backend[F] =
    new FollowRedirectsBackend(delegate) with Backend[F] {}

  def apply[F[_]](delegate: WebSocketBackend[F]): WebSocketBackend[F] =
    new FollowRedirectsBackend(delegate) with WebSocketBackend[F] {}

  def apply[F[_], S](delegate: StreamBackend[F, S]): StreamBackend[F, S] =
    new FollowRedirectsBackend(delegate) with StreamBackend[F, S] {}

  def apply[F[_], S](delegate: WebSocketStreamBackend[F, S]): WebSocketStreamBackend[F, S] =
    new FollowRedirectsBackend(delegate) with WebSocketStreamBackend[F, S] {}
}

I tried to reduce the amount of added code as much as possible, but this is still kind of tedious work. It becomes even more cumbersome when we have to deal with default arguments in the constructor of a delegate backend, because overloaded methods cannot have any default argument. To solve this problem I added a few Config case class: the FollowRedirectsConfig, the LogConfig, the PrometheusConfig...

This is how it is done for FollowRedirectsBackend:

// added a new case class with default values
case class FollowRedirectsConfig(
    contentHeaders: Set[String] = HeaderNames.ContentHeaders,
    sensitiveHeaders: Set[String] = HeaderNames.SensitiveHeaders,
    transformUri: Uri => Uri = FollowRedirectsBackend.DefaultUriTransform
)

// added a default config
object FollowRedirectsConfig {
  val Default = FollowRedirectsConfig()
}

object FollowRedirectsBackend {
  // most users don't need to customize the config
  def apply(delegate: SyncBackend): SyncBackend =
    apply(delegate, FollowRedirectsConfig.Default)

  def apply[F[_]](delegate: Backend[F]): Backend[F] =
    apply(delegate, FollowRedirectsConfig.Default)

  def apply[F[_]](delegate: WebSocketBackend[F]): WebSocketBackend[F] =
    apply(delegate, FollowRedirectsConfig.Default)

  def apply[F[_], S](delegate: StreamBackend[F, S]): StreamBackend[F, S] =
    apply(delegate, FollowRedirectsConfig.Default)

  def apply[F[_], S](delegate: WebSocketStreamBackend[F, S]): WebSocketStreamBackend[F, S] =
    apply(delegate, FollowRedirectsConfig.Default)

  // some users have their own config
  def apply(delegate: SyncBackend, config: FollowRedirectsConfig): SyncBackend =
    new FollowRedirectsBackend(delegate, config) with SyncBackend {}

  def apply[F[_]](delegate: Backend[F], config: FollowRedirectsConfig): Backend[F] =
    new FollowRedirectsBackend(delegate, config) with Backend[F] {}

  def apply[F[_]](delegate: WebSocketBackend[F], config: FollowRedirectsConfig): WebSocketBackend[F] =
    new FollowRedirectsBackend(delegate, config) with WebSocketBackend[F] {}

  def apply[F[_], S](delegate: StreamBackend[F, S], config: FollowRedirectsConfig): StreamBackend[F, S] =
    new FollowRedirectsBackend(delegate, config) with StreamBackend[F, S] {}
}

Some users will probably need to use the same technique to define their own delegate backends.
Hopefully, a lot of users don't have to define any overloaded constructor, since they use a single type of concrete backend.

Less flexibility in PartialRequest

I assumed that there is no need to introduce PartialWebSocketRequest, PartialStreamRequest and PartialWebSocketStreamRequest types. But that means the user must specify the method and uri before they can specify the websocket response-as, or the stream response-as or the stream body.

For instance, this does not compile anymore:

basicRequest
  .response(asWebSocketAlways(_ => Future.successful("Hello World")))
//          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
// type mismatch;
// found   : sttp.client3.WebSocketResponseAs[scala.concurrent.Future,String]
// required: sttp.client3.ResponseAs[?]
  .get(uri"")

One must write this instead:

basicRequest
  .get(uri"") // specify the method and uri before
  .response(asWebSocketAlways(_ => Future.successful("Hello World")))

Should we add the PartialWebSocketRequest, PartialStreamRequest and PartialWebSocketStreamRequest types?

Adding more capabilities

If, in the future, we need to add more capabilities, it will be a lot more tedious. We will need to define many new types of requests and backends, for each meaningful combination of capababilities.

As a solution of this problem, we can use a more flexible type hierarchy, made of two requests, and three backends:

• The Request[+T], the SyncBackend and the Backend[F[_]] for simple requests and backends with no capabilities.
• The SttpRequest[+T, -R] and the SttpBackend[F[_], +R] for any request and backend that needs one or more capabilities.

This would be still better for simple use cases, such as scripting, and super flexible for the more advanced use cases: websocket app or streaming.

If we plan on more capabilities we should use the more flexible type hierarchy, otherwise I think the proposed type hierarchy is better.

StreamRequest

The proposed stream request is defined as StreamRequest[T, -R] where R is the capability. R must contain a stream of type Streams, but it can also contain an Effect[F]. I could make this more explicit by defining two types of stream request, the StreamRequest[T, S <: Streams], and the StreamRequestF[F[_], T, S <: Streams].

Correspondingly the stream backend would define two send methods:

trait StreamBackend[F[_], S <: Streams] extends AbstractBackend[F[_], S] {
  def send[T](request: StreamRequest[T, S]): F[T] = internalSend(request)
  def send[T](request: StreamRequestF[F, T, S]): F[T] = internalSend(request)
}

I decided not to, but I am still not 100% convinced.

Going further

To go further on this new API, I wonder if it would be possible to split the SttpApi in parts so that we can decide to import a subset of methods: the simple response-as, the websocket response-as, the streaming response-as and bodies, the streaming and websocket response-as.

Next Steps

• Reviewing the proposal and deciding if this is going in the right direction
• Updating the website documentation. I updated the code examples but not the explanations.
• Rename package to client4 and claim group-id com.softwaremill.sttp.client4
• Releasing an early version and testing it on real projects.
• Implementing other desired changes: there probably are other changes that you want to add in the next version of sttp
• Making it more easy to maintain binary compatibility: review the visibility of some classes and methods, make some data classes easier to evolve.

[adamw]

@adpi2 Thank you! That's a really well-prepared (& most importantly - working!) proposal.

It would be of course quite a big revolution in the sttp space. But definitely worth considering. Since you kick-started discussions around sttp4, I created a forum thread to attempt to gather feedback from existing sttp users, to understand what are the biggest pain points both for beginners & long-time users, so that we fix the right problems, and don't create new ones.

Some quite comments on the proposal itself:

• I don't think we'll have any more capabilities in the future; we could have sync backends which support streaming, though (I'd expect some form of streaming to emerge in the land of Loom-java)
• one additional downside is that I think it would no longer be possible to write a method, which customises arbitrary requests? you'd need separate methods for customising partial & full requests (as these are separate types)

It boils down to a tradeoff: simpler & more user-friendly types of requests/backends, vs the flexibility to be able to customise any request and create any backend wrapper without code duplication

Some things that are missing:

• migration - what would it take for existing users to migrate to the new type hierarchy. I think on the API side there's relatively little changes, it would mostly be changing the types (which is good), but I might be missing something

Questions:

• what would be the benefit of splitting SttpApi into multiple traits? wouldn't it make discoverability harder? Right now you've got a single import, and this enables all of the as* methods to be visible by auto-complete

And also to consider

• do we want to make any changes to the model & the shared data structures? these are even harder to change, as they are used by tapir as well
• maintenance costs; we'll need a really good motivation to make such breaking changes, as it will mean that for some time (probably quite long) both sttp3 & sttp4 will need to be maintained. Maintenance costs are already quite high (due to sttp supporting 2.11, 2.12, 2.13, 3, JVM, JS, Native - esp. JS & Native builds break really often). So maybe that would be a good occasion to drop some older scala versions? Though we would probably drop the non-problematic 2.11+JVM, keeping the problematic 3+Native, so maybe that's not really relevant.

[adpi2]

@adamw Thanks for the good feedback.

I created a forum thread to attempt to gather feedback from existing sttp users, to understand what are the biggest pain points both for beginners & long-time users, so that we fix the right problems, and don't create new ones.

That's a great! It would be good to have some feedback from complete Scala beginners, but that's harder to get.

One additional downside is that I think it would no longer be possible to write a method, which customises arbitrary requests? you'd need separate methods for customising partial & full requests (as these are separate types)

It is still possible, but slightly more constrained. This is how:

def customize[R <: RequestBuilder[R]](request: R): R =
  request.body("foo" -> "bar")
    .auth.basic("user", "password")
    .cookies("foo" -> "bar")
    .maxRedirects(2)

val request: Request[String] = 
  customize(basicRequest.get(uri"...").response(asStringAlways))

val wsRequest: WebSocketRequest[Future,String] = 
  customize(
    basicRequest.get(uri"").response(asWebSocketAlways(???))
  )

RequestBuilder contains all the common builder methods of requests: headers, body, tags, options, but not the response and streamBody methods.
In the example, the customize method accepts any of the request types: it can take a Request, a WebSocketRequest, a StreamRequest and a WebSocketStreamRequest and it returns the exact same type.

migration - what would it take for existing users to migrate to the new type hierarchy. I think on the API side there's relatively little changes, it would mostly be changing the types (which is good), but I might be missing something

It would change:

• The types of requests and backends
• Less frequently, the construction of delegate backends: we now need to use config case classes to customize the delegate backends: FollowRedirectsBackend, LoggingBackends...
• Even less frequently, the definition of custom backends

I wonder if it would be possible to automate the migration. I think it is partially possible: we can define Scalafix rules to automate some of the changes, but definitely not all of them. A good migration guide is still very much needed.

what would be the benefit of splitting SttpApi into multiple traits? wouldn't it make discoverability harder? Right now you've got a single import, and this enables all of the as* methods to be visible by auto-complete.

As a user of the toolkit, I would prefer not to have access to the asStream methods if there is no stream backend available.

In other words, the asStream methods should only be available if I import for instance sttp.client3.httpclient.fs2._, and it would be defined as:

def asStream[F[_], T](f: fs2.Stream[F, Byte] =>  F[T])

instead of:

def asStream[F[_], T, S](s: S)(f: s.BinaryStream => F[T])

This is only an idea, and I am already quite happy with the current default import.

do we want to make any changes to the model & the shared data structures? these are even harder to change, as they are used by tapir as well

As far as I am concerned, I don't think it is needed.

If we release an experimental version of sttp4, we can later decide to break other parts of the API, even the shared model, and coordinate with a new version of tapir. Or rollback some of the changes, if the migration is too hard. That would be the safest strategy IMO.

maintenance costs; we'll need a really good motivation to make such breaking changes, as it will mean that for some time (probably quite long) both sttp3 & sttp4 will need to be maintained.

To be honest, I don't have much experience in maintaining such a library with so many different implementations, depending on many different libraries and ecosystems. I imagine we can maintain sttp3 and sttp4 in two branches. As long as sttp4 is experimental, sttp3 is the default branch, it receives all bug-fixes, and added features. Once in a while we port all the changes to sttp4. Once sttp4 is stable, it becomes the default and we backport the bugfixes to sttp3.

So maybe that would be a good occasion to drop some older scala versions? Though we would probably drop the non-problematic 2.11+JVM, keeping the problematic 3+Native, so maybe that's not really relevant.

👍 for dropping Scala 2.11

[jodersky]

Thanks for making this proposal @adpi2, and thank you for writing all the detailed descriptions.

I think that the proposed changes indeed make the signatures of the user API simpler and more discoverable, and this is a step in the right direction.

Looking at the big picture however, I think that the sttp API is in general still more advanced than what I would imagine when thinking of a "basic" HTTP client. Personally, my ideal simple client would be a bunch of static methods which wrap HTTP verbs eagerly, and where everything would be accessible with only one non-wildcard import. My favorite examples of such an API are Python's requests library and the requests-scala library, and, if you stretch the definition a bit, the curl command line tool. Such a basic API would of course also be much more limited than what sttp has to offer, but I think that it would still be useful for most use-cases, especially in scripting and one-off program contexts, which IMO is where the Scala Toolkit effort is most helpful.

Now, I don't think that sttp's interface should change to accomodate this, because otherwise it wouldn't be sttp anymore. Hence I wonder if it would make sense to split the offering in two: one library for a high-level interface, similar to scala-requests, and one more powerful and flexible interface -- yet still very discoverable -- like this one proposed here, but where all still use sttp's backends. I haven't looked too much into what would be required for this, but I can try to spend some time next week on a proof-of-concept, porting scala-requests (JVM to start) to use sttp under the hood.

[adamw]

@adpi2 sorry for the long silence - I was trying to gather some feedback both regarding the proposition and what users would like to see changed in sttp. Unfortunately, this mostly failed - despite quite large engagement on various posts in social media - I got only a couple of replies, and about areas other than discussed here (though these will be probably need to be addressed in sttp4 anyway, so good to know about them and thanks for the people that did respond :) ).

But also, there was not a word of negative feedback, so I guess that's a good sign ;). I'll do a more thorough review next (though maybe I won't have any comments) and we'll take it from there.

Another thing to consider is the cost of migrating. A number of people are already using sttp, and a new major version means that eventually, they should migrate. So we should be careful to impose this additional cost only when this really pays off.

One idea regarding the various backends and their wrappers - maybe it would be possible to have sth like a GenericBackend, where all the logic would reside; on top of this, SyncBackend etc would be simple delegates, which fix the send signature to a specific effect. A backend wrapper would be essentially a function GenericBackend[F, ...] => GenericBackend[F, ...]. SyncBackend.wrap would re-wrap a generic backend with the end-user interface.

RequestBuilder contains all the common builder methods of requests: headers, body, tags, options, but not the response and streamBody methods.

This is good enough - as long as it's possible to write a generic request-modifying function, should be fine. However, this would mean that RequestBuilder isn't internal, but part of the public user interface (which is fine I guess)

As a user of the toolkit, I would prefer not to have access to the asStream methods if there is no stream backend available.

I understand that, but on the other hand this limits discoverability as it's harder to get to know about the streaming capabilities in the first place, plus having to lookup the specific import (if an IDE doesn't help us). So I think the cons outweight the pros here.

To be honest, I don't have much experience in maintaining such a library with so many different implementations, depending on many different libraries and ecosystems. I imagine we can maintain sttp3 and sttp4 in two branches. As long as sttp4 is experimental, sttp3 is the default branch, it receives all bug-fixes, and added features. Once in a while we port all the changes to sttp4. Once sttp4 is stable, it becomes the default and we backport the bugfixes to sttp3.

Yeah, it's the backporting that is the whole problem. Each PR after being merges needs to be rebased and a PR needs to be created for the other branch (this should be possible to automate). Then you need to merge, release, etc. It's additional boring work that needs to be done ;)

[adamw]

@jodersky I might be wrong here, but I don't have the impression that the main audience for the Toolkit is one-off programs and scripts: an area where Scala is currently almost non-existent (I haven't heard about anybody doing scripting in Scala, to be honest, comparing to a whole lot of other use-cases). But of course, I'm not the one defining what's in the Toolkit or not :).

An alternative API is of course possible, but that would be a different library. As above - I'm not the one picking what goes into the Toolkit, I can just try to do my best if sttp is offered the opportunity. So I'm not sure if that's the best place for discussing whether sttp, or another more Python-like library should be the Toolkit's HTTP client. It might be that another library is a better choice - but they I'm afraid I or other people from the sttp team won't be of much help :).

One thing I'll definitely not want to do, is include an alternative API in sttp itself. Firstly because of maintenance reasons, secondly because I don't think having two libraries glued into one is a good idea. If another approach is better, fine, but let's not overcomplicate and try to call it a single thing. sttp's power lies in the fact that it scales from direct, synchronous backends, making simple GET requests, all the way to integrating with effect systems and streaming libraries. You only have to learn a single API. I don't think that HTTP is complicated enough so that you should have to decide when to use the "simple" and when the "advanced" library.

Not the fact that there are multiple approaches to effect handling in Scala, ranging from direct, through Future to IO might be both seen as a blessing and a curse - but again, that's a different discussion :)

[odersky]

Have we thought about including both sstp and a simple requests-like library that wraps it in the toolkit? The upside would be that simple use cases would be handled simply but that full sttp is available as a fallback for more advanced stuff.

[SethTisue]

I don't have the impression that the main audience for the Toolkit is one-off programs and scripts: an area where Scala is currently almost non-existent (I haven't heard about anybody doing scripting in Scala, to be honest, comparing to a whole lot of other use-cases)

I think that Scala has for many years now been in a vicious cycle where library authors don't cater to this use case because they think the users aren't out there, but the users aren't there in part because they aren't usually well serviced by library authors (except Haoyi). Breaking out of this cycle is one of the aims of the Toolkit.

[szymon-rd]

Toolkit is the motivation for this PR, but is not really the main topic here. I feel like motivating this change based on Toolkit can shift the perspective from the usability argumentation, and I feel there's a significant improvement. Mainly I don't really think that it makes the sttp more basic, it definitely simplifies the type signatures, making them less scary, but, per the discussion, does not make it more limited. And, in my opinion, that makes it a great change, regardless of the existence of Toolkit. In this case it's not really shifting any balance, I feel that both newcomers and more experienced Scala users can benefit from this in some way. Nevertheless, it's not an easy task to make change this big in a big library. But I would really like to use sttp API like this in the future :)

But of course, I'm not the one defining what's in the Toolkit or not :).

Toolkit is an attempt to address a community need and every perspective has to be heard and addressed, without it we miss the point. Everyone willing to get engaged in creating the Toolkit will be the part of process of defining what's in the Toolkit. Of course we need to make a decision at some point and have processes for this purpose, otherwise we could discuss every step indefinitely :) We will be opening a discussion about it soon, I hope that all the parties that have something to say will speak up. Even when it's purely negative feedback, given of course that it is constructive.

[adamw]

@SethTisue I think that Scala has for many years now been in a vicious cycle where library authors don't cater to this use case because they think the users aren't out there, but the users aren't there in part because they aren't usually well serviced by library authors (except Haoyi). Breaking out of this cycle is one of the aims of the Toolkit.

You're right, though I'd say that scripts are the main audience of sttp; so I wouldn't prioritise this over all the other use-cases. So this would be a "nice to have" in terms of priorities.

Though I'd argue that we have sth that isn't far off. It's a single wildcard import (which is a difference from what @jodersky is describing, but we need to bring in both a default backend instance & the API to describe requests):

import sttp.client3.quick._
simpleHttpClient.send(quickRequest.get(uri"http://httpbin.org/ip"))

If we replace simpleHttpClient with the changes by @adpi2, we could have an extension method on requests having this syntax instead:

import sttp.client3.quick._
quickRequest.get(uri"http://httpbin.org/ip").send()

[DaniRey]

Huge thanks to @adamw for sttp and @adpi2 for that promising PR.

As an sttp user I would love to get an experimental release, to try the proposed API changes. In my experience APIs are always hard to judge without using them.

In my opinion it would be great to have a scalable API for a scalable language. The quickRequest example from @adamw looks promising to me. Scalable in my opinion means, to avoid having two APIs, but sensible defaults and convenience methods to keep simple things simple.

Additionally I would love, if sttp would be easier to debug. I would like to be able to call show (or similar) on a Request and see (Http method, request URL, request headers, request body and ideally even the corresponding curl command). The same would be great for Response. (Maybe this is already possible and I just haven't discovered it yet.)

[adamw]

@DaniRey thanks for sharing your perspective :) There is .showBasic and .show on RequestT already, as well as .toCurl and .toRfc2616Format - these might be helpful when debugging.

[DaniRey]

Thanks @adamw, that's amazing! Don't know how I missed that. I will check it out after my vacation.

[adamw]

For completeness, here's a (positive) comment by @baldram: https://functional.cafe/@baldram/109806495134758932

[adamw]

I've started working on a PR changing a bit how Backend is implemented - see adpi2#5 // cc @adpi2 @szymon-rd

[adamw]

Progress report:

• I've closed my attempt at another approach at structuring the backends - turns out, we either loose some type-inference, or inheritance between backends
• coming back to the original design, I've removed the Backend.send methods, instead making Request.send the only "proper" way to send requests from client code. Also I've renamed AbstractBackend to GenericBackend as this isn't an abstract class, and can be instantiated stand-alone if needed. Additionally GenericBackend.internalSend became simply GenericBackend.send (+ comments), as this isn't an internal method: people implementing their own backends will write its implementation, so the name might be misleading.
• I rewrote some of the comments, and added an ADR explaining the motivation behind restructuring the backends (@adpi2 take a look if you'd add something)

[adamw]

More updates:

• I've added another ADR describing the rationale behind refactoring Requests
• I've added some comments / made them consistent across the request subtypes on request classes
• the requests classes are now grouped in request.scala, builder classes in requestBuilder.scala, and backends in backend.scala. This should help decrease the number of top-level files in the core module, and make it easier to explore the codebase

[adamw]

Another round: time for ResponseAs. Unfortunately, we cannot classify what used to be InternalResponseAs as an "internal" API - as it is used by backend implementations. And the possibility to implement your own backend is part of the public API, so this is not internal.

I made an attempt at renaming this so it would still make sense, and I ended up with:

• ResponseAsDelegate as the base trait, with implementations ResponseAs, StreamResponseAs etc., which are simple wrappers
• GenericResponseAs as the old hierarchy of all the response-as cases

This works, but reading the code I'm getting lost myself (which response-as variant we are currently working with - the wrapped or generic). Moreover, both GenericResponseDelegate and GenericResponseAs are generic - as they contain the capability type parameter.

So I'm looking at some way to improve this.

One solution would be to simply keep the old ResponseAs hierarchy, exposing the capability type parameter to clients. So the base would be that we would have asString: ResponseAs[String, Any] etc.

I think that's a reasonable tradeoff, as far as I'm aware the response-as descriptions are rarely captured as top-level values (instead being used directly in request descriptions), so the type would be barely visible. A downside is that the type signatures would be a bit more complicated, as we would need to require implicit evidence when setting the Request.response to ensure that there are no capabilities. Error messages could be made user-friendly with @implicitNotFound. What do you think @adpi2 @szymon-rd ?

[adpi2]

This works, but reading the code I'm getting lost myself (which response-as variant we are currently working with - the wrapped or generic). Moreover, both GenericResponseDelegate and GenericResponseAs are generic - as they contain the capability type parameter.

I agree this is over-complicated and confusing.

I think that's a reasonable tradeoff, as far as I'm aware the response-as descriptions are rarely captured as top-level values (instead being used directly in request descriptions), so the type would be barely visible.

That's right, I don't think we often capture them as top-level values. But the type is still visible in the signatures of the client3 package object, which is the main entrypoint of the library. Many programmers are not familiar with implicit evidence, so I would try to avoid them as well.

The problem I had with the original ResponseAs and the reason why I created the ResponseAsDelegate is that I could not split the ResponseAs hierarchy in 4 distinct branches (ResponseAs, WebSocketResponseAs, StreamResponseAs and WebSocketStreamResponseAs) because of the generic ResponseAsFromMetadata, MappedResponseAs and ResponseAsBoth. Maybe if we can pull those generic forms out of ResponseAs then it would make things easier.

My best proposal would be to put the raw responses (StringResponse, BinaryResponse, WebSocketResponse, MultiPartResponse...) in one type hierarchy, the transformed responses (MappedResponse, ResponseAsBoth, ResponseFromMetadata) in another, and to glue them together in the GenericResponseAs hierarchy. So it is one more type hierarchy but also a better separation of concerns: fundamentally each backend should only implement the raw response part. The downside is that it brings more changes to the backend part. I may try to flesh this out and let you know if it works or not.

[adamw]

My best proposal would be to put the raw responses (StringResponse, BinaryResponse, WebSocketResponse, MultiPartResponse...) in one type hierarchy, the transformed responses (MappedResponse, ResponseAsBoth, ResponseFromMetadata) in another, and to glue them together in the GenericResponseAs hierarchy. So it is one more type hierarchy but also a better separation of concerns: fundamentally each backend should only implement the raw response part. The downside is that it brings more changes to the backend part. I may try to flesh this out and let you know if it works or not.

Hmm I guess this might work. Let's see in code :) As for separation of concerns, that's more or less how the code works today, see BodyFromResponseAs - although without type-level representation. So maybe this could be cleaned up as well, by removing the inheritance from BodyFromResponseAs, and making it a function which can be called with a parameter, capturing handling of "raw" responses

[adamw]

@adpi2 can you maybe share what was the rationale behind the changes to the AbstractBody (used to be RequestBody) hierarchy? At first glance I don't see anything in the old one that would be less programmer-friendly than the current one, but I'm probably missing something :)

[adamw]

Hmm I guess this might work. Let's see in code :) As for separation of concerns, that's more or less how the code works today, see BodyFromResponseAs - although without type-level representation. So maybe this could be cleaned up as well, by removing the inheritance from BodyFromResponseAs, and making it a function which can be called with a parameter, capturing handling of "raw" responses

@adpi2 will you be attempting to implement this, or should I take a shot?

[adpi2]

@adpi2 can you maybe share what was the rationale behind the changes to the AbstractBody (used to be RequestBody) hierarchy? At first glance I don't see anything in the old one that would be less programmer-friendly than the current one, but I'm probably missing something :)

First I think we can rename AbstractBody to RequestBody, but I would find GenericRequestBody more clear. I renamed it to AbstractBody for some reason, when I was in the experimentation phase, and probably forgot to rename it back to RequestBody.

The rationale behind the changes are:

• All implementations of RequestBody[Any] now extends BasicBody, so that we can hide the Any capability from the user. So Request.body now returns a BasicBody, instead of a RequestBody[Any], (but they are equivalent). Maybe BasicBody should be renamed BasicRequestBody.
• I split MultipartStreamBody and BasicMultipartBody so that BasicMultipartBody can extend BasicBody.
• I introduced BodyPart and BasicBodyPart to define explicily which body types can be part of a MultipartStreamBody and a BasicMultipartBody. A NoBody and a MultipartBody cannot be part of a MultipartBody anymore: it would not type check. That simplifies some of the exhaustivity checks in the backend implementations.

[adpi2]

About 904ef13, in sttp 3 it is possible to define a request with a stream body and a websocket response, but it was not tested anywhere, so I don't know if it really makes sense or not (do people really need it ?). 904ef13 makes it valid also in sttp4.

[adpi2]

@adpi2 will you be attempting to implement this, or should I take a shot?

I am starting now and will continue tomorrow.

[adamw]

The rationale behind the changes are:

All clear, thanks! I'll do the renaming (later, not to collide with your work) & maybe add the above points to an ADR so that it gets persisted for future generations :)

I am starting now and will continue tomorrow.

Great :)

About 904ef13, in sttp 3 it is possible to define a request with a stream body and a websocket response, but it was not tested anywhere, so I don't know if it really makes sense or not (do people really need it ?). 904ef13 makes it valid also in sttp4.

Request & response bodies are independent, so while this is highly unlikely that you'd stream a request body to get back a websocket, there might be use-cases for it out there ;) So well spotted, 👍 for fixing this.

[adpi2]

So I tried to define a new type hierarchy of GenericResponseAs to get rid of ResponseAsDelegate. This new hierarchy has four sub-traits: ResponseAs, WebSocketResponseAs, StreamResponseAs and WebSocketStreamResponseAs. The MappedResponseAs, ResponseAsFromMetadata and ResponseAsBoth are still part of the hierarchy but distributed among the 4 sub-traits.

It works well in the sense that it is now possible to pattern match exhaustively on GenericResponseAs on two axes:

ra match {
  case _: ResponseAs[_] =>
  case _: WebSocketResponseAs[_, _] =>
  case _: StreamResponseAs[_, _] =>
  case _: WebSocketStreamResponseAs[_, _] =>
}

ra match {
  case InoreResponse =>
  case ResponseAsByteArray =>
  case ResponseAsFile(file) =>
  case ResponseAsWebSocket(f) =>
  case ResponseAsWebSocketUnsafe =>
  case ResponseAsStream(s, f) =>
  case ResponseAsStreamUnsafe(s) =>
  case ResponseAsWebSocketStream(s, p) =>
  case MappedResponseAs(raw, f, showAs) =>
  case ResponseAsFromMetadata(conditions, default) =>
  case ResponseAsBoth(l, r) =>
}

For the record, this is how I defined the GenericResponseAs in a gist. I did not complete the refactoring because of the problems explained below.

1. I lose the sub-type relationship between, for instance, ResponseAs[T] <: StreamResponseAs[T, Nothing] or ResponseAs[T] <: WebSocketResponseAs[Nothing, T]. It is problematic because:
   • we cannot combine several types of responseAs into a correctly typed form of ResponseAsFromMetadata
   • we cannot put a ResponseAs in a StreamRequest (with a StreamBody)
2. I cannot implement MapEffect because when I have an instance of ResponseAsFromMetadata or ResponseAsFromBoth I don't know how to copy it while preserving its type. (I can fix this by adding a copy method, adding more boilerplate).

My conclusion is that the current implementation with the ResponseAsDelegate is better because simpler.

I am going to try a few more things and let you know when I am done.

[adamw]

@adpi2 thanks for the ongoing investigation! :)

[adamw]

I'll merge this as-is, and we can iterate on further design improvements in subsequent PRs. Thanks @adpi2 for your work! :)

[adpi2]

Amazing! Thanks @adamw for supporting this initiative and reviewing it thoroughly.

[adpi2]

Would it be possible to have a SNAPSHOT, nightly or milestone version released any time soon?

[adamw]

@adpi2 yes, next week :)

[adamw]

Ready: https://github.com/softwaremill/sttp/releases/tag/v4.0.0-M1