Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convenience methods to consume response body #257

Closed
sagebind opened this issue Nov 17, 2020 · 0 comments · Fixed by #284
Closed

Convenience methods to consume response body #257

sagebind opened this issue Nov 17, 2020 · 0 comments · Fixed by #284
Labels
feature A new feature!

Comments

@sagebind
Copy link
Owner

It would be nice to provide convenience methods for consuming a response body without storing it. This is a useful feature because if the user drops the response without reading the response body fully, the underlying connection cannot be reused and must be closed prematurely. While totally acceptable, this is not a good way to maintain high throughput since it prevents connection reuse.

The API could be something as simple as:

impl Body {
    pub fn consume(&mut self) -> io::Result<u64> {
        // ...
    }

    pub async fn consume_async(&mut self) -> io::Result<u64> {
        // ...
    }
}

Which would be essentially a convenience around io::copy(&mut response.body_mut(), &mut io::sink()).

@sagebind sagebind added the feature A new feature! label Nov 17, 2020
sagebind added a commit that referenced this issue Dec 29, 2020
Previously we were using two different atomic sources to discover when a response has been dropped, but only emitting a warning for an unconsumed response body if one of the sources indicated a drop. This was causing inconsistent results of when the warning would be emitted. Fix this by having just one source of truth for when the response is dropped.

In addition, change the warning log into an info log instead with a better message. Aborting a response stream is not incorrect and may be intentionally desired by the user. After #257 we could directly suggest using `consume()` in the log message.

See #270.
sagebind added a commit that referenced this issue Dec 29, 2020
Previously we were using two different atomic sources to discover when a response has been dropped, but only emitting a warning for an unconsumed response body if one of the sources indicated a drop. This was causing inconsistent results of when the warning would be emitted. Fix this by having just one source of truth for when the response is dropped.

In addition, change the warning log into an info log instead with a better message. Aborting a response stream is not incorrect and may be intentionally desired by the user. After #257 we could directly suggest using `consume()` in the log message.

See #270.
sagebind added a commit that referenced this issue Dec 31, 2020
Add `ReadResponseExt::consume` and `AsyncReadResponseExt::consume` methods for reading and discarding a response bod as described in #257.

Also update future types used by `AsyncReadResponseExt` to be `Send` whenever `T` is `Send` as appropriate.

Fixes #257 and #283.
sagebind added a commit that referenced this issue Jan 12, 2021
Add `ReadResponseExt::consume` and `AsyncReadResponseExt::consume` methods for reading and discarding a response body as described in #257.

Also update future types used by `AsyncReadResponseExt` to be `Send` whenever `T` is `Send` as appropriate. Consolidate the newtype futures with a macro that handles conditional `Send` properly to reduce boilerplate and chance of mistakes.

Fixes #257.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant