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

Provide SinkError #213

Closed
carllerche opened this issue Oct 20, 2016 · 8 comments
Closed

Provide SinkError #213

carllerche opened this issue Oct 20, 2016 · 8 comments

Comments

@carllerche
Copy link
Member

This issue represents a discussion had in https://gitter.im/tokio-rs/tokio w/ @aturon.

At a high level, any type that has values pushed into it needs a way to return an error that contains this value in the event that it is not currently able to process the value (similar to TrySendError in std)

The conclusion was to provide something like:

/// Custom errors should provide a conversion to SinkError.
///
/// For example:
///     impl From<HttpError> for SinkError<HttpRequest, HttpError> {
///     }
///
pub enum SinkError<T, E> {
    NotReady(T),
    Other(E),
}

/// Example middleware that uses SinkError
impl<S> Service for Retry<S>
    where SinkError<S::Request, S::Error>: From<S::Error>,
{
    type Request = S::Request;
    type Error = S::Error;

    fn call(&self, request: S::Request) -> impl Future<S::Response, S::Error> {
        // Bump Arc count
        let retry = self.clone();

        self.inner.upstream.call(request).then(move |res| {
            match res {
                Ok(resp) => Ok(resp),
                Err(e) => {
                    match SinkError::from(e) {
                        SinkError::NotReady(request) => {
                            // Some global timer
                            timer::sleep(Duration::from_secs(10))
                                .and_then(|_| retry.call(request))
                        }
                        SinkError::Other(e) => Err(e),
                    }
                }
            }
        })
    }
}
@alexcrichton
Copy link
Member

cc #164, this'd likely be used there. Also a time to perhaps consider addition of a Sink trait

@alexcrichton alexcrichton added this to the 0.2 release milestone Oct 20, 2016
@Ralith
Copy link
Contributor

Ralith commented Oct 21, 2016

Sink::map would certainly be useful to have.

@tailhook
Copy link
Contributor

Does this request also mean that poll_ready in Service might be removed?

I wish tokio_service to land on crates.io soon, and with poll_ready removed, because I don't think it can be useful in the current form.

The idea described here and some kind of Service + Sink might be more useful.

@carllerche
Copy link
Member Author

Would you be able to elaborate on "I don't think it can be useful in the current form"? Leaving your statement without any support doesn't provide much for us to understand your reasoning.

@alexcrichton
Copy link
Member

@tailhook it sounds like you may want to open an issue on tokio-service? Perhaps that could be where we continue discussing Service?

For now I believe 0.1 of tokio-service is not blocked on this at all.

@tailhook
Copy link
Contributor

@carllerche, @alexcrichton created an issue tokio-rs/tokio-service#5

@alexcrichton
Copy link
Member

@tailhook thanks!

@alexcrichton
Copy link
Member

I believe this was basically done with sinks, so closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants