Make async-std optional #246
Conversation
src/body.rs
Outdated
| @@ -160,7 +159,7 @@ impl Body { | |||
| /// # Examples | |||
| /// | |||
| /// ``` | |||
| /// # fn main() -> Result<(), http_types::Error> { async_std::task::block_on(async { | |||
| /// # fn main() -> Result<(), http_types::Error> { futures_lite::future::block_on(async { | |||
There was a problem hiding this comment.
Instead of relying on block_on (which futures_util does not have), these should move to using #[async_std::main]
| /// # fn main() -> Result<(), http_types::Error> { futures_lite::future::block_on(async { | |
| /// # #[async_std::main] | |
| /// # async fn main() -> Result<(), http_types::Error> { |
There was a problem hiding this comment.
Replaced with async_std::task::block_on since that was already being used.
There was a problem hiding this comment.
These should still be moved to #[async_std::main], either in this PR or in another.
There was a problem hiding this comment.
Using #[async_std::main] does increase compile times though. It's probably better to make that change in a separate PR.
There was a problem hiding this comment.
Ah I suppose we may want to leave the hidden ones as is then...
|
This looks good. Is it semver-major? |
I don't believe so. |
|
|
| /// | ||
| /// Unlike `async_std::sync::channel` the `send` method on this type can only be | ||
| /// called once, and cannot be cloned. That's because only a single instance of | ||
| /// `Connection` should be created. | ||
| #[must_use = "Futures do nothing unless polled or .awaited"] |
There was a problem hiding this comment.
Is this comment no longer relevant? Why was it removed?
There was a problem hiding this comment.
This is the receiver, the comment was meant for the sender I assume.
| Self { sender } | ||
| } | ||
|
|
||
| /// Send a `Trailer`. | ||
| /// | ||
| /// The channel will be consumed after having sent trailers. | ||
| pub async fn send(self, trailers: Connection) { | ||
| self.sender.send(trailers).await | ||
| let _ = self.sender.send(trailers).await; |
There was a problem hiding this comment.
This line seems worth discussing. async-std channels cannot be closed as long as there's a sender, but async-channels can be, and return an Err variant if the channel is closed. Probably the best fix for this is to return that Result, making this semver-major. As written, this silently swallows the possible SendError.
There was a problem hiding this comment.
Functionally, this is the same as existing behavior. It's a oneshot channel where the sender doesn't care if the receiver is dropped. It might be a good idea to return a Result here if the receiver has been dropped, but I see it as a separate issue.
There was a problem hiding this comment.
Since this Sender/Receiver code is duplicated a couple times, I'd recommend making a generic oneshot submodule that's shared by trailers, upgrade, and whatever else relies on this pattern.
There was a problem hiding this comment.
I was planning that as a follow-up, but I wanted to keep this PR non-breaking.
jbr
dismissed
their stale review
concern about behavior with closed channels addressed
closes #244