Skip to content

Commit

Permalink
Add an HTTP ResponseFuture struct (#1008)
Browse files Browse the repository at this point in the history
We currently take owned heap allocated types, such as Vecs and Strings.
This is due to the lifetimes around requests, their inner polled
futures, and the caller are difficult to work around.

The way requests currently work is that they implement
`std::future::Future` and "start" a future when initially polled for the
first time. They then store this future internally in a field like so:

```rust
pub struct SomeRequest {
    fut: Option<Pending<'a, Message>>,
    // more fields...
}
```

This requires a lot of boilerplate on our part: we have a macro
implementing `Future` on every request with more or less unique
implementations and need fields for the inner future.

For users, this design comes at a cost of performance and clarity. Users
need to give us owned arguments. It is also impossible for users to
determine whether a request is in flight using the type system due to
that information being hidden. We need to clone arguments if we want to
avoid panics from users accidentally double polling a request after
completion, when it may not be clear that a request has been used due to
the request not being consumed.

Instead, we can turn the inner request functions that "start" a future
into ones that consume and execute the request and then return a future
resolving to a response directly to the user. The new
`response::ResponseFuture` implements `Poll` and will begin the
execution of a request - if a failure did not occur during construction
- while polling it to completion, providing the resulting
`response::Response`.

Internally the implementation of the `CreateMessage::exec` function
looks like this:

```rust
pub fn exec(self) -> ResponseFuture<Message> {
    let request = Request::from_route(Route::GetMessage {
        channel_id: self.channel_id.0,
        message_id: self.message_id.0,
    });

    self.http.request(request)
}
```

The usage of executing the request and printing the message contents
looks like this:

```rust
let msg = client.message(channel_id, message_id).exec().await?;
let message = response.model().await?;

println!("content: {}", message.content);
```

This work does not include the optimization work that turns all of our
owned parameters to borrowed ones and will be done by a separate PR.

Signed-off-by: Zeyla Hellyer <zeyla@hellyer.dev>
  • Loading branch information
zeylahellyer authored Jul 10, 2021
1 parent d09c1ed commit 7f05145
Show file tree
Hide file tree
Showing 148 changed files with 2,152 additions and 2,176 deletions.
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,10 @@ async fn handle_event(
) -> Result<(), Box<dyn Error + Send + Sync>> {
match event {
Event::MessageCreate(msg) if msg.content == "!ping" => {
http.create_message(msg.channel_id).content("Pong!")?.await?;
http.create_message(msg.channel_id)
.content("Pong!")?
.exec()
.await?;
}
Event::ShardConnected(_) => {
println!("Connected on shard {}", shard_id);
Expand Down
3 changes: 2 additions & 1 deletion gateway/queue/src/day_limiter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ impl DayLimiter {
let info = http
.gateway()
.authed()
.exec()
.await
.map_err(|source| DayLimiterError {
kind: DayLimiterErrorType::RetrievingSessionAvailability,
Expand Down Expand Up @@ -94,7 +95,7 @@ impl DayLimiter {
} else {
let wait = lock.last_check + lock.next_reset;
time::sleep_until(wait).await;
if let Ok(res) = lock.http.gateway().authed().await {
if let Ok(res) = lock.http.gateway().authed().exec().await {
if let Ok(info) = res.model().await {
let last_check = Instant::now();
let next_reset = Duration::from_millis(info.session_start_limit.remaining);
Expand Down
2 changes: 1 addition & 1 deletion gateway/src/cluster/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl ClusterBuilder {
ClusterStartError,
> {
if (self.1).0.gateway_url.is_none() {
let maybe_response = (self.1).0.http_client.gateway().authed().await;
let maybe_response = (self.1).0.http_client.gateway().authed().exec().await;

if let Ok(response) = maybe_response {
let gateway_url = response.model().await.ok().map(|info| info.url);
Expand Down
1 change: 1 addition & 0 deletions gateway/src/cluster/impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ impl Cluster {
let gateway = http
.gateway()
.authed()
.exec()
.await
.map_err(|source| ClusterStartError {
kind: ClusterStartErrorType::RetrievingGatewayInfo,
Expand Down
1 change: 1 addition & 0 deletions gateway/src/shard/impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,7 @@ impl Shard {
.http_client()
.gateway()
.authed()
.exec()
.await
.map_err(|source| ShardStartError {
source: Some(Box::new(source)),
Expand Down
1 change: 1 addition & 0 deletions http/examples/allowed-mentions/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ async fn main() -> Result<(), Box<dyn Error + Send + Sync>> {
.user_ids(vec![user_id])
.build(),
)
.exec()
.await?;

Ok(())
Expand Down
3 changes: 2 additions & 1 deletion http/examples/get-message/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ async fn main() -> Result<(), Box<dyn Error + Send + Sync>> {
.create_message(channel_id)
.content(format!("Ping #{}", x))
.expect("content not a valid length")
.exec()
}))
.await;

let me = client.current_user().await?.model().await?;
let me = client.current_user().exec().await?.model().await?;
println!("Current user: {}#{}", me.name, me.discriminator);

Ok(())
Expand Down
3 changes: 2 additions & 1 deletion http/examples/proxy/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ async fn main() -> Result<(), Box<dyn Error + Send + Sync>> {
.create_message(channel_id)
.content(format!("Ping #{}", x))
.expect("content not a valid length")
.exec()
}))
.await;

let me = client.current_user().await?.model().await?;
let me = client.current_user().exec().await?.model().await?;
println!("Current user: {}#{}", me.name, me.discriminator);

Ok(())
Expand Down
2 changes: 1 addition & 1 deletion http/src/client/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl ClientBuilder {
proxy: self.proxy,
ratelimiter: self.ratelimiter,
timeout: self.timeout,
token_invalid: AtomicBool::new(false),
token_invalid: Arc::new(AtomicBool::new(false)),
token: self.token,
application_id: self.application_id,
default_allowed_mentions: self.default_allowed_mentions,
Expand Down
Loading

0 comments on commit 7f05145

Please sign in to comment.