Skip to content
This repository has been archived by the owner on Mar 3, 2020. It is now read-only.

Mixing early returns with async_blocks #107

Open
illicitonion opened this issue Jul 14, 2018 · 2 comments
Open

Mixing early returns with async_blocks #107

illicitonion opened this issue Jul 14, 2018 · 2 comments

Comments

@illicitonion
Copy link

I realise that this isn't really about async/await itself, but using it in surrounding code made me question my instincts, so I figured I'd open a discussion...

I want to write some code along the lines of:

fn foo(&self) -> impl Future<...> {
  if let Some(early_value) = self.check() {
    // Doesn't compile because Ok is a Result, but function returns an `impl Future`.
    // Need to return a futures::future::ok(early_value) instead.
    return Ok(early_value);
  }

  // Doesn't compile because the ? operator can't be used in a function which returns an impl Future.
  // Need to explicitly match and early-return instead.
  let request = self.prepare_request()?;

  // Clone value out because borrows aren't allowed in async_blocks.
  let client = self.client.clone();

  async_block! {
    let response = await!(client.make_request(request));
    await!(parse(response))
  }
}

There are three big things that stop me from being able to do this today; the two things commented on in the code, and that my function returns an impl Future, but after making the two changes in the comments, I'm no longer returning one unique anonymous Future type, so I need to Box everything up rather than returning an impl Trait.

My instinct is that my async_block should be as short as possible, because minimising the scope where the reader needs to consider asynchrony feels good. But if I'd moved the lines before the client cloning as-is into the async_block, the code would compile fine (modulo theself borrows), because the async_block itself does effectively return a Result.

Is my instinct to minimise the size of aync_blocks wrong? Is there a more ergonomic way of doing what I'm doing without enlarging the async_block?

Thanks!

@withoutboats
Copy link
Collaborator

withoutboats commented Jul 17, 2018

Is my instinct to minimise the size of aync_blocks wrong?

I think so. Using an async_block means the code outside of that block will be run immediately; you should only do this if you need that code to be run immediately. I don't think its true that users need to "consider asynchronicity," as you say, when inside the async block; in contrast, I think by using an async_block you are signaling to readers that they need to worry about the immediacy of the code that runs outside of it; that that code has some sort of side effect & the timing of it matters.

In other words, the great ambition of async/await in Rust, with its type system, is that everything in the async will run "eventually," it doesn't matter when.

@Nemo157
Copy link
Contributor

Nemo157 commented Jul 17, 2018

One other consideration is whether users of this API would prefer to handle errors in the pre-async part of the code differently to the async part. Probably not in the case of a Future, but I've purposefully used -> Result<impl Stream> before to allow handling setup errors separately to errors during the stream.

The big thing I note in your implementation is // Clone value out because borrows aren't allowed in async_blocks., while that is a limitation of the macro you're using it won't be a limitation of async {...}. However it can be a deliberate API design choice to have fn foo(&self) -> impl Future<...> + 'static (or probably more commonly fn foo(&self, temp: &Bar) -> impl Future<...> + '_ where temp is only needed during the pre-async part). In that case you might need to be doing the same sort of early return so having an ergonomic way to do that would be useful.

Closest I can currently see is

fn foo(&self) -> impl Future<...> {
  let result: Result<_, Box<Error>> = loop {
    break do catch {
      if let Some(early_value) = self.check() {
        break Ok(Either::A(early_value));
      }

      let request = self.prepare_request()?;
      let client = self.client.clone();

      Either::B(async_block! {
        let response = await!(client.make_request(request));
        await!(parse(response))
      })
    }
  };

  future::result(result).flatten()
}

Maybe some parts of this could be extracted out to a macro to do the early return plumbing for you?

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

No branches or pull requests

3 participants