tokio-rs / doc-push

Tokio doc blitz effort - A concerted effort to improve Tokio's documentation.
MIT License
50 stars 7 forks source link

"Working with Futures" page needs improvement around implementing Future #67

Open carllerche opened 6 years ago

carllerche commented 6 years ago

Regarding: https://github.com/tokio-rs/doc-push/blob/master/outline/futures-streams-sinks.md#working-with-futures

There is currently a point "Implementing custom combinators", but this does not do justice to the topic of implementing futures.

Future is implemented more than just for custom combinators. Entire applications can be written by only implementing Future and other poll_ based functions.

Maybe there should be an additional page in that section dedicated to writing code (or entire apps) entirely without future combinators?

davidbarsky commented 5 years ago

Big plus one on this. The example on “Returning Futures” under the title “Custom types” would be great if it could be fleshed out. As a bit of low-hanging fruit, the following example (particularly the ... in fn foo) body is one such candidate:

struct MyFuture {
    inner: Sender<i32>,
}

fn foo() -> MyFuture {
    let (tx, rx) = oneshot::channel();
    // ...
    MyFuture { inner: tx }
}

impl Future for MyFuture {
    // ...
}

Assuming that several future combinator chains occur in the body of foo() it'd be nice to see how values could be send to the tx value without dropping things.

On an aside, I find working with the poll API to be far more understandable than the combinators API, and I recall @hawkw noting the best way to work Tokio/Futures 0.1 in the large is to implement the futures yourself (unlike Scala or TypeScript, where the futures are a library feature that you only interact with).

davidbarsky commented 5 years ago

An example I'd like to write:

use hyper::{Client, StatusCode};

struct MyFuture {
    inner: Receiver<i32>,
}

fn foo() -> MyFuture {
    let (sender, receiver) = oneshot::channel();
    let client = Client::new();
    client
         .get("google.com".parse::<Uri>().unwrap())
         .and_then(|res| sender.send(res.status_code));

    MyFuture { inner: receiver }
}

impl Future for MyFuture {
    // ...
}

...without getting a cancelation on the oneshot—I think the client/sender is getting deallocated before the status code value could be sent.

carllerche commented 5 years ago

@davidbarsky it looks like you are dropping the return value of and_then, which cancels the entire future. Maybe you want to spawn that work?

davidbarsky commented 5 years ago

@carllerche Good point. In this case, should I use oneshot's spawn or Tokio's spawn?

carllerche commented 5 years ago

Tokio' spawn for tokio examples I guess.

davidbarsky commented 5 years ago

Hmm, I'm seeing Spawn Error { is_shutdown: true } errors. To avoid wasting your time, can you point me to an example that implements a future over oneshot?

carllerche commented 5 years ago

@davidbarsky I can't think of an existing example off the top of my head.

The problem is most likely that you are calling foo() from off the runtime. You probably want to do:

use futures::future::lazy;

tokio::run(lazy(|| {
   // setup here, probably call `foo())`.
   Ok(())
}))

This sucks and I hope to fix it in 0.2, but for now it is what it is. I would probably ask that you figure out where some of these concepts should be introduced in the docs, because you are stumbling on issues that others have as well.

davidbarsky commented 5 years ago

The problem is most likely that you are calling foo() from off the runtime. You probably want to do:

Hmm, tried that, and the issues persisted. I'll try to create a minimal example that demonstrates what I'm doing.

This sucks and I hope to fix it in 0.2, but for now it is what it is. I would probably ask that you figure out where some of these concepts should be introduced in the docs, because you are stumbling on issues that others have as well.

I guess the biggest one would be "the how's and why's" of implementing Future yourself, because the intuition as to when someone should implement their own Future seems to be the biggest gap between intermediate and advanced users of Tokio.