Custom HTTP Response abstraction #923

zeylahellyer · 2021-06-11T21:20:46Z

Our HTTP async request builders return a deserialized model in the output of their Future implementations, but only if the response has an expected body. For example, the GetUser request's Future implementation returns a deserialized User. GetGuildRoles' Future implementations returns Vec<Role>.

This design has three problems worth noting:

It's impossible to know the headers and status code of a response: it may be useful to check if the response is a success (2xx status code range) prior to deserializing; and certain headers may be useful for users to know;
Not all responses are used: in many instances of creating a message the created message is not used, so we're spending CPU time on a useless operation; and
~~Response bodies with a list will entirely fail if one entry fails to deserialize.~~ not included in this PR.

By creating a custom Response type we can solve all three of these problems. Take the first example: checking the status code prior to deserializing a response body. With a Response struct this is easy:

use std::env;
use twilight_http::Client;

let client = Client::new(env::var("DISCORD_TOKEN")?);
let response = client.user(user_id).await?;

if !response.status().is_success() {
    println!("failed to get user");

    return Ok(());
}

// Twilight already knows to deserialize it into a
// `twilight_model::user::User`.
let user = response.model().await?;

println!("user's name: {}:{}", user.name, user.discriminator);

In this example it's easy to check if the status code of the response is in the success range (2xx).

Looking at the example, we can see how the second issue is solved: you can simply opt to not deserialize the model. This may not be useful for getting a user, but can be for creating a message:

use std::env;
use twilight_http::Client;

let client = Client::new(env::var("DISCORD_TOKEN")?);
client.create_message(channel_id)
    .content("test")?
    .await?;

Here we create a message and return an error if the request failed. We don't care about the message itself, so the response is discarded without deserializing it.

The Response struct also has a few other cool functions: bytes which returns a Future that resolves to a Vec<u8> of the bytes of the response body, text which returns a String of the body if it's UTF-8 valid, and headers which returns an iterator of the names and values of the response headers.

In the simplest case, this PR causes users that want the returned deserialized model to append .model().await? or .models().await? to their request calls.

http/src/response/iter.rs

http/src/response/mod.rs

7596ff · 2021-06-13T22:06:45Z

Can probably force push this to rescue it

zeylahellyer · 2021-06-16T00:50:54Z

Addressed comments.

gateway/queue/src/day_limiter.rs

http/src/request/channel/webhook/execute_webhook.rs

http/src/request/mod.rs

zeylahellyer · 2021-06-16T22:50:18Z

Answered comments

zeylahellyer · 2021-06-27T01:09:34Z

This PR is ready now. I have removed response body iterator support via StreamDeserializer. Please see commit 0c3cd07 for details.

7596ff

Iter needs to be removed from the PR description as well

zeylahellyer · 2021-06-27T18:08:55Z

Iter needs to be removed from the PR description as well

Done. Merged commit will need to account for it, so I can do that.

Instead of using the `write!` and `format_args!` macros use `Display` and `Formatter` method calls. The motivation for doing this is shown in PR #942. This does not include the work required to replace `write!` calls in `http/src/routing.rs`; this work will be done in a separate PR. Additionally one call is left in `http/src/request/channel/reaction/mod.rs` and will be resolved in another PR extracted from further work on #923. Relates to #943.

7596ff

Needs updated to latest next

Our HTTP async request builders return a deserialized model in the output of their `Future` implementations, but only if the response has an expected body. For example, the `GetUser` request's `Future` implementation returns a deserialized `User`. `GetGuildRoles`' `Future` implementations returns `Vec<Role>`. This design has three problems worth noting: 1. It's impossible to know the headers and status code of a response: it may be useful to check if the response is a success (2xx status code range) prior to deserializing; and certain headers may be useful for users to know; 2. Not all responses are used: in many instances of creating a message the created message is not used, so we're spending CPU time on a useless operation; and 3. Response bodies with a list will entirely fail if one entry fails to deserialize. By creating a custom `Response` type we can solve all three of these problems. Take the first example: checking the status code prior to deserializing a response body. With a `Response` struct this is easy: ```rust use std::env; use twilight_http::Client; let client = Client::new(env::var("DISCORD_TOKEN")?); let response = client.user(user_id).await?; if !response.status().is_success() { println!("failed to get user"); return Ok(()); } // Twilight already knows to deserialize it into a // `twilight_model::user::User`. let user = response.model().await?; println!("user's name: {}:{}", user.name, user.discriminator); ``` In this example it's easy to check if the status code of the response is in the success range (2xx). Looking at the example, we can see how the second issue is solved: you can simply opt to not deserialize the model. This may not be useful for getting a user, but can be for creating a message: ```rust use std::env; use twilight_http::Client; let client = Client::new(env::var("DISCORD_TOKEN")?); client.create_message(channel_id) .content("test")? .await?; ``` Here we create a message and return an error if the request failed. We don't care about the message itself, so the response is discarded without deserializing it. The final problem of one failure in a list causing the deserialization of the entire list to fail can be solved with a lazily deserializing iterator. Instead of deserializing everything up front, we can use an iterator which will deserialize entries as the iterator is advanced. We can see how that looks like when retrieving the roles of a guild and printing their names: ```rust use std::env; use twilight_http::Client; let client = Client::new(env::var("DISCORD_TOKEN")?); let response = client.roles(guild_id).await?; // Create an iterator over the roles. Each role in the response body is only // deserialized as the iterator advances. let mut roles = response.iter().await?; while let Some(maybe_role) = roles.next() { // The role may not have deserialized properly, so we need to check. match maybe_role { Ok(role) => println!("role {}'s name: {}", role.id, role.name), Err(source) => println!("failed to deserialize role: {:?}", source), } } ``` The `Response` struct also has a few other cool functions: `bytes` which returns a `Future` that resolves to a `Vec<u8>` of the bytes of the response body, `text` which returns a String of the body if it's UTF-8 valid, and `headers` which returns an iterator of the names and values of the response headers. In the simplest case, this PR causes users that want the returned deserialized model to append `.model().await?` or `.models().await?` to their request calls. Signed-off-by: Vivian Hellyer <vivian@hellyer.dev>

Signed-off-by: Vivian Hellyer <vivian@hellyer.dev>

`serde_json::StreamDeserializer` doesn't work by consuming bodies like `[{"a": 1}, {"b": 2}]`, it instead works by consuming bodies like: ```json {"a": 1} {"b": 2} ``` Working around this in a performant way will take some work, so for now I am removing `MemberIter` and `ModelIter` support. Signed-off-by: Vivian Hellyer <vivian@hellyer.dev>

Signed-off-by: Vivian Hellyer <vivian@hellyer.dev>

Signed-off-by: Zeyla Hellyer <zeyla@hellyer.dev>

zeylahellyer · 2021-07-03T02:16:44Z

Updated. I have some additional performance work I would like to do but that isn't strictly in the scope of this PR. Should be ready for review and merging now.

Resolved

Signed-off-by: Zeyla Hellyer <zeyla@hellyer.dev>

Our HTTP async request builders return a deserialized model in the output of their `Future` implementations, but only if the response has an expected body. For example, the `GetUser` request's `Future` implementation returns a deserialized `User`. `GetGuildRoles`' `Future` implementations returns `Vec<Role>`. This design has two problems worth noting for this work: 1. It's impossible to know the headers and status code of a response: it may be useful to check if the response is a success (2xx status code range) prior to deserializing; and certain headers may be useful for users to know; and 2. Not all responses are used: in many instances of creating a message the created message is not used, so we're spending CPU time on a useless operation. By creating a custom `Response` type we can solve all three of these problems. Take the first example: checking the status code prior to deserializing a response body. With a `Response` struct this is easy: ```rust use std::env; use twilight_http::Client; let client = Client::new(env::var("DISCORD_TOKEN")?); let response = client.user(user_id).await?; if !response.status().is_success() { println!("failed to get user"); return Ok(()); } // Twilight already knows to deserialize it into a // `twilight_model::user::User`. let user = response.model().await?; println!("user's name: {}:{}", user.name, user.discriminator); ``` In this example it's easy to check if the status code of the response is in the success range (2xx). Looking at the example, we can see how the second issue is solved: you can simply opt to not deserialize the model. This may not be useful for getting a user, but can be for creating a message: ```rust use std::env; use twilight_http::Client; let client = Client::new(env::var("DISCORD_TOKEN")?); client.create_message(channel_id) .content("test")? .await?; ``` Here we create a message and return an error if the request failed. We don't care about the message itself, so the response is discarded without deserializing it. The `Response` struct also has a few other cool functions: `bytes` which returns a `Future` that resolves to a `Vec<u8>` of the bytes of the response body, `text` which returns a String of the body if it's UTF-8 valid, and `headers` which returns an iterator of the names and values of the response headers. In the simplest case, this PR causes users that want the returned deserialized model to append `.model().await?` or `.models().await?` to their request calls.

@zeylahellyer

Enhancements Many functions have been made constant ([#1010] - [@zeylahellyer]). Changes There are significant changes to how users make HTTP requests. When users make a request, they must pass borrowed types instead of owned types. To execute the request, users must call `exec` on the request builder. Once the request has completed execution, users may use the `ResponseFuture` struct methods to access the status code of the request. To access a returned model, if there is one, users must call `model` on the response. A call to `Client::create_message` like this: ```rust client.create_message(ChannelId(1)) .content("some content")? .embed(Embed {})? .await?; ``` is now written like this: ```rust client.create_message(ChannelId(1)) .content(&"some conntent")? .embeds(&[&Embed {}])? .exec() .await? .model() .await?; ``` For more information on the motivation behind these changes, see the PR descriptions of [#923], [#1008], and [#1009]. These changes were authored by [@zeylahellyer]. Rename `ErrorCode::UnallowedWordsForPublicStage` variant to `UnallowedWords` ([#956] - [@7596ff]) `CreateGlobalCommand`, `CreateGuildCommand`, `SetGlobalCommands`, and `SetGuildCommands` now return command(s) ([#1037] - [@vilgotf]). A few spelling errors have been fixed by adding the `codespell` Action ([#1041] - [@Gelbpunkt]. [#923]: #923 [#956]: #956 [#1008]: #1008 [#1009]: #1009 [#1010]: #1010 [#1037]: #1037 [#1041]: #1041 [@7596ff]: https://github.com/7596ff [@Gelbpunkt]: https://github.com/Gelbpunkt [@vilgotf]: https://github.com/vilgotf [@zeylahellyer]: https://github.com/zeylahellyer

Our HTTP async request builders return a deserialized model in the output of their `Future` implementations, but only if the response has an expected body. For example, the `GetUser` request's `Future` implementation returns a deserialized `User`. `GetGuildRoles`' `Future` implementations returns `Vec<Role>`. This design has two problems worth noting for this work: 1. It's impossible to know the headers and status code of a response: it may be useful to check if the response is a success (2xx status code range) prior to deserializing; and certain headers may be useful for users to know; and 2. Not all responses are used: in many instances of creating a message the created message is not used, so we're spending CPU time on a useless operation. By creating a custom `Response` type we can solve all three of these problems. Take the first example: checking the status code prior to deserializing a response body. With a `Response` struct this is easy: ```rust use std::env; use twilight_http::Client; let client = Client::new(env::var("DISCORD_TOKEN")?); let response = client.user(user_id).await?; if !response.status().is_success() { println!("failed to get user"); return Ok(()); } // Twilight already knows to deserialize it into a // `twilight_model::user::User`. let user = response.model().await?; println!("user's name: {}:{}", user.name, user.discriminator); ``` In this example it's easy to check if the status code of the response is in the success range (2xx). Looking at the example, we can see how the second issue is solved: you can simply opt to not deserialize the model. This may not be useful for getting a user, but can be for creating a message: ```rust use std::env; use twilight_http::Client; let client = Client::new(env::var("DISCORD_TOKEN")?); client.create_message(channel_id) .content("test")? .await?; ``` Here we create a message and return an error if the request failed. We don't care about the message itself, so the response is discarded without deserializing it. The `Response` struct also has a few other cool functions: `bytes` which returns a `Future` that resolves to a `Vec<u8>` of the bytes of the response body, `text` which returns a String of the body if it's UTF-8 valid, and `headers` which returns an iterator of the names and values of the response headers. In the simplest case, this PR causes users that want the returned deserialized model to append `.model().await?` or `.models().await?` to their request calls.

zeylahellyer added t-feature Addition of a new feature c-http Affects the http crate c-model Affects the model crate c-gateway Affects the gateway crate m-breaking change Breaks the public API. c-gateway-queue Affects the gateway queue crate labels Jun 11, 2021

Erk- self-requested a review June 11, 2021 21:31

zeylahellyer mentioned this pull request Jun 12, 2021

Remove dependencies from public APIs #714

Closed

BlackHoleFox reviewed Jun 13, 2021

View reviewed changes

7596ff force-pushed the next branch from e72b4ff to d0d69a4 Compare June 13, 2021 22:00

BlackHoleFox previously approved these changes Jun 16, 2021

View reviewed changes

zeylahellyer dismissed BlackHoleFox’s stale review via 7a3dceb June 16, 2021 02:18

7596ff suggested changes Jun 16, 2021

View reviewed changes

gateway/queue/src/day_limiter.rs Show resolved Hide resolved

gateway/queue/src/day_limiter.rs Show resolved Hide resolved

http/src/request/channel/webhook/execute_webhook.rs Show resolved Hide resolved

http/src/request/mod.rs Show resolved Hide resolved

7596ff previously approved these changes Jun 17, 2021

View reviewed changes

Erk- previously approved these changes Jun 17, 2021

View reviewed changes

zeylahellyer marked this pull request as draft June 18, 2021 01:09

zeylahellyer mentioned this pull request Jun 18, 2021

Use Formatter calls over write! #944

Merged

zeylahellyer dismissed stale reviews from Erk- and 7596ff via 0c3cd07 June 27, 2021 00:49

zeylahellyer marked this pull request as ready for review June 27, 2021 01:09

7596ff suggested changes Jun 27, 2021

View reviewed changes

7596ff approved these changes Jun 27, 2021

View reviewed changes

7596ff requested review from BlackHoleFox and tbnritzdoge June 28, 2021 22:27

BlackHoleFox previously approved these changes Jun 28, 2021

View reviewed changes

7596ff previously requested changes Jun 29, 2021

View reviewed changes

Erk- previously approved these changes Jun 30, 2021

View reviewed changes

zeylahellyer marked this pull request as draft July 2, 2021 16:52

zeylahellyer added 5 commits July 2, 2021 22:11

address comments

8aae832

Signed-off-by: Vivian Hellyer <vivian@hellyer.dev>

remove option from pending response types

90162a5

Signed-off-by: Vivian Hellyer <vivian@hellyer.dev>

remove docs mentioning iter

a2d431c

Signed-off-by: Vivian Hellyer <vivian@hellyer.dev>

zeylahellyer dismissed stale reviews from Erk- and BlackHoleFox via a2d431c July 3, 2021 02:11

cargo fmt

c61852e

Signed-off-by: Zeyla Hellyer <zeyla@hellyer.dev>

zeylahellyer marked this pull request as ready for review July 3, 2021 02:16

zeylahellyer requested a review from 7596ff July 3, 2021 02:19

add some non exhaustives to body markers

03d95a2

Signed-off-by: Zeyla Hellyer <zeyla@hellyer.dev>

zeylahellyer mentioned this pull request Jul 3, 2021

Add an HTTP ResponseFuture struct #1008

Merged

7596ff approved these changes Jul 3, 2021

View reviewed changes

7596ff requested review from Erk- and BlackHoleFox July 3, 2021 19:18

Erk- approved these changes Jul 3, 2021

View reviewed changes

BlackHoleFox approved these changes Jul 3, 2021

View reviewed changes

zeylahellyer merged commit d09c1ed into twilight-rs:next Jul 3, 2021

zeylahellyer deleted the feature/http/response branch July 3, 2021 23:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom HTTP Response abstraction #923

Custom HTTP Response abstraction #923

zeylahellyer commented Jun 11, 2021 •

edited

Loading

7596ff commented Jun 13, 2021

zeylahellyer commented Jun 16, 2021

zeylahellyer commented Jun 16, 2021

zeylahellyer commented Jun 27, 2021

7596ff left a comment

zeylahellyer commented Jun 27, 2021

7596ff left a comment

zeylahellyer commented Jul 3, 2021

Custom HTTP Response abstraction #923

Custom HTTP Response abstraction #923

Conversation

zeylahellyer commented Jun 11, 2021 • edited Loading

7596ff commented Jun 13, 2021

zeylahellyer commented Jun 16, 2021

zeylahellyer commented Jun 16, 2021

zeylahellyer commented Jun 27, 2021

7596ff left a comment

Choose a reason for hiding this comment

zeylahellyer commented Jun 27, 2021

7596ff left a comment

Choose a reason for hiding this comment

zeylahellyer commented Jul 3, 2021

zeylahellyer commented Jun 11, 2021 •

edited

Loading