Timeout improvements

[danieleades]

Description

By wrapping the return type of the error returned by the timeout method, the restrictions on the type of error are removed, and a consumer can differentiate whether the timeout deadline was hit, or whether the inner future errored. This is a breaking change.

Motivation and Context

In its present state, this method can only be used with error types that implement Fromio::Error which is restrictive. additionally, if the error type already has a Fromio::Error implementation, it becomes unergonomic to distinguish between timeouts and other types of error

How Has This Been Tested?

Yep (see completed checks below)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[danieleades]

build issues appear to be related to http-rs/tide#305

[danieleades]

can't for the life of me figure out why this isn't building. It's complaining that the trait bound 'ext::TimeoutStream<S>: futures_core::stream::Stream' is not satisfied

   --> /home/dan/.cargo/registry/src/git.cakeli.workers.dev-1ecc6299db9ec823/futures-timer-0.2.1/src/ext.rs:170:9
    |
170 | impl<S> TryStream for TimeoutStream<S>
    |         ^^^^^^^^^ the trait `futures_core::stream::Stream` is not implemented for `ext::TimeoutStream<S>`                                                                                                   
    |
    = note: required because of the requirements on the impl of `futures_core::stream::private_try_stream::Sealed` for `ext::TimeoutStream<S>`                                                                    

It looks like the blanket implementation of TryStream is failing, but I can't see why it should.

two things-

• It only fails when running cargo test, not when running cargo build
• the master branch is also failing in the exact same way

[taiki-e]

@danieleades Build failure on the master branch was fixed in #27.

[danieleades]

@danieleades Build failure on the master branch was fixed in #27.

aha! thanks for this. that should do it

[yoshuawuyts]

@danieleades thanks heaps for filing this! -- I agree with the reasoning you've outlined here, and in fact we came to a similar conclusion when designing the APIs in async-std.

Something I'm moderately concerned about is how it would work when integrating with some of the codebases we've built on top of this. cc/ @stjepang I feel you might have a good sense of whether these changes would be compatible with async-std (I assume so, but want to make sure) -- could you perhaps take a look?

[ghost]

I like this set of changes! :) I think my only comment is to rename the Waited struct to TimeoutError, like @yoshuawuyts also suggested.

[danieleades]

any progress on this @yoshuawuyts ?