Add custom future type, ResponseFuture, for HTTP responses

[analogrelay]

Closes #1831

This is a long one, but I really wanted to try and lay out all the details of the change

This PR introduces a new type, ResponseFuture, which is intended to be returned by service client APIs. For example:

impl ContainerClient {
    fn read(
        &self,
        options: Option<ReadContainerOptions>,
    ) -> azure_core::ResponseFuture<ContainerProperties>;
}

The Response<T> type has been simplified dramatically. It is now a plain-ol data object containing the status code, headers, and "body" (which is of type T). The default value of T is ResponseBody, which is our existing type that wraps a "Pinned Stream" representing an asynchronous stream of bytes coming from the transport.

How is ResponseFuture different to the user?

Our current service client methods return impl Future<Output = Result<Response<T>, Error>>. However, in the current code, Response<T> is itself a promise as well. It does not contain a fully downloaded and deserialized HTTP response body, only the promise of one, when deserialize_body is called. By default, a ResponseFuture<T>, when directly awaited, will execute the request, receive the status code and headers, download the body, and deserialize it into a T. I believe this represents the "majority" of use cases, which means we can turn our current long chain of method calls:

let response = secret_client.get_secret("secret_password")
    .await?.
    .deserialize_body()
    .await?;
println!("Secret Value", response.into_body().value);

Into just one await:

let response = secret_client.get_secret("secret_password").await?;
println!("Secret Value", response.into_body().value);

In a situation where most users want to read the body (if there is one) into the default model provided by the service client, this is much more ergonomic. It also removes the need for the Model trait, since the service client gave the ResponseFuture a closure it could use to perform the deserialization.

What about more advanced scenarios?

While I believe eagerly reading and deserializing the body is the majority case, it is not a good approach in all cases. Consider, for example, that Cosmos DB often returns the full JSON of a newly-created item. Often, the user doesn't need this since they just created the item (in fact, we even provide a request header to stop the server from sending the new item in the response). Or consider an API that returns a large payload, but also includes some response headers that the user may want to use to decide if it even wants to download that whole payload. Or any other case where the user wants to download the response headers but defer downloading the body until later. Until the ResponseFuture is awaited, it only represents the promise of a future response, and the user has the ability to modify it's behavior using methods on ResponseFuture.

Calling .lazy() will change the ResponseFuture into an impl Future<Output = LazyResponse<T>> which executes the requests and processes the response headers, but does not process the body until LazyResponse::into_body is called.

Calling .raw() will change the ResponseFuture into an impl Future<Output = Response<ResponseBody>>, in effect removing the deserialization logic so that the returned value will be a Response<ResponseBody>. That gives the user the ability to download the body and deserialize however they want¹, including deserializing it to a custom type:

#[derive(Debug, Deserialize)]
struct MyCustomSecret {
    #[serde(rename = "name")]
    pub my_custom_name: String,
    #[serde(rename = "value")]
    pub my_custom_value: String,
}
let response = secret_client.get_secret("secret_password").raw().await?;
let raw_body = response.into_body();
let secret: MyCustomSecret = raw_body.json().await?;

How does a service client create a ResponseFuture?

Rather than being async fns that return Result<Response<T>>, service client methods should be non-async fns that return ResponseFuture<T>. The main pipeline's send method returns a ResponseFuture<T>, so for most client methods that are assembling a request, sending it, and returning the resulting ResponseFuture<T>, there's no need for the client method itself to be async. In the less common event that a client needs to perform follow-up work after a response is received that cannot be achieved using a custom Policy, we have ResponseFuture::new which allows a caller to synthesize a ResponseFuture out of any other future that returns a Result<Response>.

ResponseFuture<T> represents the promise of a Response<ResponseBody> (a raw HTTP response) to come at a future time and the ability to deserialize that raw body to a T when it does arrive. The send method on the pipeline returns ResponseFuture<ResponseBody> (i.e. a promise for a future Response<ResponseBody> representing a raw, unparsed, HTTP body, with no deserialization logic). From there, service client methods can call ResponseFuture::json and ResponseFuture::xml to convert that "raw" ResponseFuture into one that deserializes the body into their target type. For example:

impl ContainerClientMethods for ContainerClient {
    fn read(
        &self,
        options: Option<ReadContainerOptions>,
    ) -> azure_core::ResponseFuture<ContainerProperties> {
        let req = Request::new(self.container_url.clone(), azure_core::Method::Get);
        self.pipeline
            .send(Context::new(), req, ResourceType::Containers)
            .json() // Convert `ResponseFuture<ResponseBody>` to `ResponseFuture<ContainerProperties>` using JSON
    }
}

Boxes. Boxes Everywhere.

The main downside is the liberal use of boxing and dynamic dispatch. We Box the future that performs the request, we Box a closure to deserialize the body, which itself returns another Boxed future representing the downloading and deserialization of the body. I did this because I wanted to avoid having to have per-operation types. If we want to optimize out the boxing, we have another option.

We would turn ResponseFuture<T> into a trait, intended to be used via impl Trait syntax, and use macros to have each service client return its own custom struct that implements ResponseFuture<T>. Those client methods would look something like this:

impl ContainerClientMethods for ContainerClient {
    fn read(
        &self,
        options: Option<ReadContainerOptions>,
    ) -> impl azure_core::ResponseFuture<ContainerProperties> {
        // This macro expects async code that returns a raw response
        // The "actual" deserialization to JSON can't be done inside the code block
        // because we need to be able to defer it when "raw" is called
        response_future!(Json, {
            let req = Request::new(self.container_url.clone(), azure_core::Method::Get);
            self.pipeline
                .send(Context::new(), req, ResourceType::Containers)
                .await
        });
    }
}

In that approach, the type the macro generates would able to be fully static and avoid boxing altogether. I'm open to that idea, and returning custom futures is what the unofficial client does so there's some prior art there. It's just a lot of custom types. Having said that, these types wouldn't have to be public, they could be defined in the method itself and returned as impl Trait types so that the user doesn't actually see them.

Ownership of Context and Request

Another call I made here was to change the Pipeline::send methods to take the context and request by-value instead of taking references. That isn't totally necessary. But it allows service clients to just call Pipeline::send and give it all the necessary data that needs to be stored in the closure used by ResponseFuture. Without taking those values by-value, the service client method would have to create an async closure manually, using ResponseFuture::new.

It felt reasonable to me for the pipeline to take those values. I recognize why Policies don't receive those by value (since policies may be executed multiple times on the same Context/Request pair) but it seemed from my reading that Context and Request were not intended to be shared across multiple separate requests.

Footnotes

1. In this case, the user is expected to call ResponseBody::json() or ResponseBody::xml() manually. I wasn't a fan of that when it was in the primary use-case, but I'm OK with it when the user has explicitly decided to handle processing the response themselves. ↩

[analogrelay]

This was just duplicated, you can see it further up if you expand the context

[analogrelay]

Azure Identity doesn't use the full pipeline, it calls the HTTP client itself. It's always used the "Raw Response" and manually deserialized.

[analogrelay]

Moved these types here because they are useful across a few different modules in http. This does mean it's been promoted from typespec_client_core::http::response::PinnedStream to typespec_client_core::http::PinnedStream

[LarryOsterman]

I'm being utterly pedantic here, but there's something about the term "Lazy" that is bothering me because it feels somewhat pejorative.

My problem is that I'm not coming up with a better description of how to name it. Lazy has the advantage that it is succinct, whereas other descriptive terms (two step result?) are just way too wordy.

[RickWinter]

If it would help put you at ease, Lazy is used throughout the language
https://doc.rust-lang.org/std/cell/struct.LazyCell.html

[analogrelay]

Yeah, I largely agree @LarryOsterman , but it's definitely become a fairly industry-standard term for "A thing that isn't quite there yet". DeferredResponse could be another option, but I feel like lazy()/LazyResponse<T> is going to be the "expected" name, especially given @RickWinter 's point about its existing use in Rust.

[heaths]

I'd rather we stick with Lazy as well. It's all over not only in Rust but other languages as well. That, I'm just to lazy to think of a better term.

[LarryOsterman]

So be it. I'll put my pedantry back in its box.

[analogrelay]

Re-drafting this. Hitting some snags around dealing with requests with bodies. With the current changes, because the functions all return ResponseFuture<T> instead of being async functions returning Result<...>, it's difficult to do asynchronous or fallible things before sending the request, because there's no support in Rust for custom "Future-likes" or "Result-likes" at this time. It's still completely possible to write the necessary code, but you have to wrap it in ResponseFuture::new manually and things get messy with lifetimes fast. I'll keep poking at it though. This isn't blocking, but I think the ideal of getting the 80% scenario to a single .await? is worth pursuing if we can.

[heaths]

We should be able to do Future-likes, I thought, right? We just have to implement IntoFuture<>. As for custom Result-likes, I've been watching try_trait_v2 for a couple years and it seems like maintainers are just bike shedding over names when the community seems largely unified on "just go with FromResidual like you have now and release it". 😔

[analogrelay]

We should be able to do Future-likes, I thought, right?

Not quite. Anything that implements IntoFuture can be .awaited. The problem is constructing the future out of an async fn. From what I can see, there's no way to turn an arbitrary async fn into a custom Future-like. There's no equivalent to AsyncMethodBuilder in C#, that I can find.

I've been watching try_trait_v2 for a couple years

Same, and it's frustrating to see it languish so long.

---

Basically, the problem we have here is that creating a ResponseFuture requires two things:

1. A boxed async closure to perform the actual request (including any "pre-flight" requests¹, if your API requires that) and get the final response
2. A boxed async closure to read and deserialize the response body, which has to be separate so that .lazy() can resolve the first future while deferring the deserialization.

I'm going to keep poking and see if I can find a clean way to do this. I'm not a huge fan of every service client having to look like this:

async fn replace_item<T: Serialize>(
    &self,
    partition_key: impl Into<PartitionKey>,
    item_id: impl AsRef<str>,
    item: T,
    options: Option<ItemOptions>,
) -> azure_core::ResponseFuture<T> {
    // Create the response future to return the raw response body
    // ResponseFuture::new expects an async closure returning Result<Response>, so fallible and async operations can be done here
    let raw_response_future = ResponseFuture::new(async {
        let url = self
            .container_url
            .with_path_segments(["docs", item_id.as_ref()]);
        let mut req = Request::new(url, azure_core::Method::Put);
        req.insert_headers(&partition_key.into())?;
        req.set_json(&item)?;
        self.pipeline
            .send(Context::new(), &mut req)
            .await
    });

    // Attach the JSON deserializer and return it.
    raw_response_future.json()
}

But if we're OK with that, I think we can work with something. Though when I did that, I started hitting lifetime issues around capturing the function parameters that might start to proliferate named lifetimes, which I'd really prefer not to do 😵.

I'll keep working a bit on it. It's not blocking my Cosmos work, it's just motivated by my desire to get the 80% scenario down to a single .await?.

Footnotes

1. "Pre-flight" here doesn't refer specifically to CORS, which is a browser concept. I just mean any requests or async operations the API needs to make before sending the "primary" request that the browser cares about. Cosmos has a few cases where we do multiple requests in a single operation, for example. ↩