Skip to content

Switch to std::future::Future #485

Closed

Description

@ebkalderon

Transitioning from futures 0.1 to std::future::Future would greatly improve the user experience in the near future, especially with async/await stabilizing in Rust 1.39 and std::future::Future compatible versions of tokio, hyper, and tower on the horizon. It seems that jsonrpc-core is currently the only library in my personal JSON-RPC projects that still relies exclusively on futures 0.1, and it is blocking me from switching over to the new trait completely. I understand if this issue will sit on the backburner for the time being until at least tokio 0.2.0 is released, though.

Activity

  1. tomusdrw commented on Aug 30, 2019

    @tomusdrw
    Contributor

    Yeah, I think it would be great to switch at least jsonrpc-core (and derive maybe) initially, the transports would be fine with the Compat layer.

    @tomaka you have some experience with porting to std::future, any hints? :)

  2. tomaka commented on Aug 31, 2019

    @tomaka

    Most of the transition should be straight-forward, except that you might have to add where F: Unpin a bit everywhere. The tokio runtime can be replaced with what is in futures-preview::executor.

    One part where it might be difficult is that libraries compatible with new futures have ditched lots of combinators in favour of async-await-friendly methods that borrow objects. For example, calling incoming() of async-std::TcpListener (which returns a stream of incoming connections) no longer takes ownership of the TcpListener but borrows it. This is very user-friendly with async/await, but is a pain in the ass if you don't have access to async/await yet. In libp2p, we decided to wait for async/await to be stable for this reason.

    Also, unless that changed, if you use tokio for networking, you will have to use tokio for executing futures as well. I'd encourage using futures-timer and async-std instead of tokio.

  3. ebkalderon commented on Sep 7, 2019

    @ebkalderon
    ContributorAuthor

    According to Rust Forge, version 1.39 will land on November 7th, which will include stable async/await. In that case, should we start transitioning to std::future::Future now? Or should we wait for the API redesign that @tomaka is working on to land first?

  4. tomusdrw commented on Sep 8, 2019

    @tomusdrw
    Contributor

    I think it's worth start the transition, I didn't have time to look into it yet. I'd be really happy to accept a PR though if you are interested in working on this @ebkalderon :)

  5. ebkalderon commented on Sep 8, 2019

    @ebkalderon
    ContributorAuthor

    @tomusdrw Sure, I suppose I can try!

  6. ebkalderon commented on Sep 8, 2019

    @ebkalderon
    ContributorAuthor

    I've successfully converted jsonrpc-core to std::future::Future, but I am blocked on jsonrpc-server-utils since it requires tokio, and tokio-sync >=0.2.0-alpha.2 apparently requires a nightly compiler and async/await.

    EDIT: See the switch-to-std-future branch for details.

    One particularly unfortunate change in futures 0.3 is the loss of the IntoFuture trait, which means that closures passed to IoHandler::handle_request(), IoDelegate::add_method(), and all related methods look a bit less nice than they do currently. This isn't a deal-breaker, though, as the crate is still perfectly usable without it.

  7. tomusdrw commented on Sep 8, 2019

    @tomusdrw
    Contributor

    Can we just convert jsonrpc-core for now and use the Compat layer for the rest?

  8. ebkalderon commented on Sep 8, 2019

    @ebkalderon
    ContributorAuthor

    @tomusdrw Almost, but not quite. It seems we will also need to make some changes to jsonrpc-derive, since the code produced by MethodRegistration::generate() makes use of the now-nonexistent IntoFuture trait. We will likely need to explicitly check whether each RPC method returns Result or a future type, and wrap the former in a futures::future::ready() where necessary. I'm not exactly sure how to do this with syn, but I assume there is a way.

  9. ebkalderon commented on Sep 24, 2019

    @ebkalderon
    ContributorAuthor

    Apparently, there was mention of IntoFuture in the original async/await Rust RFC, as I had learned from this recent blog post, but it seems it was permanently dropped. I'm not exactly sure how to move forward with the conversion process since the lack of IntoFuture will likely substantively break the functionality of jsonrpc-derive.

  10. tomusdrw commented on Sep 24, 2019

    @tomusdrw
    Contributor

    We can force the methods to return a Future instead of IntoFuture. So sync calls won't be a first-class citizen any more.

    Please also follow https://github.com/paritytech/jsonrpsee/ as that's a new library we are working on that is fully Futures-based.

  11. frondeus commented on Dec 16, 2019

    @frondeus

    Now async/await is stable - do you need any help with migration @ebkalderon?

  12. ebkalderon commented on Dec 16, 2019

    @ebkalderon
    ContributorAuthor

    @frondeus Stability of async/await wasn't exactly a blocker for starting migration, but rather the removal of the IntoFuture trait. I don't believe this library can be migrated to async/await without changing the API semantics since it relies very heavily on IntoFuture in order to have the API accept both Result and Future types in numerous areas, with the distinction being that the former is used for synchronous requests and the latter for asynchronous requests. I'm personally waiting for jsonrpsee to stabilize and replace this library completely, since the IoHandler architecture was apparently overdue for a redesign anyway. See the comment from @tomusdrw above.

  13. ebkalderon commented on Dec 19, 2019

    @ebkalderon
    ContributorAuthor

    Good news, looks like IntoFuture is coming back! It might be possible to convert jsonrpc-core as-is to async/await if this lands. 🎉

  14. Thegaram commented on Feb 1, 2020

    @Thegaram

    Hey @ebkalderon, now that IntoFuture is back, do you think it would be more straightforward to migrate to std::future?

    I'm trying to call an async function from within my RPC handler but I find it troublesome to convert the result to futures::Future. Would be awesome if we could just write async handlers directly.

  15. frondeus commented on Feb 1, 2020

    @frondeus

    It is not back yet - it was reverted cause of some strange performance drops.
    rust-lang/rust#67982

  16. Thegaram commented on Feb 2, 2020

    @Thegaram

    Oh I see, thanks for the info @frondeus!

    Then I assume the only way to use std::future::Future with this crate for now is to convert it to a futures 0.1 Future? Any recommendations for how to do this properly? Relevant resources I found [1, 2] seem to be outdated. I tried the following but suspect there must be an easier way...

    impl<T, X: std::future::Future<Output=Result<T, Error>>> futures::future::Future for X {
        type Item = T;
        type Error = Error;
        // ...
    }
    
  17. Thegaram commented on Feb 4, 2020

    @Thegaram

    If anyone wants to use async functions in RPC handlers, this is how I resolved it.

    First, you'll need the compat feature:

    futures = { version = "0.3.2", features = ["compat"] }

    Then you can have an RPC handler like this (see this and this):

    use jsonrpc_core::BoxFuture;
    use futures::future::{FutureExt, TryFutureExt};
    
    fn handler(&self, ...) -> BoxFuture<...> {
        // ...
    
        // this block should produce an `RpcResult`
        let fut = async move {
            // ...
            Ok(res)
        };
    
        Box::new(fut.boxed().compat())
    }

    Additionally, you need to make sure that all your types are Send. This helped me a lot in understanding why my fut was not Send. You can also annotate fut to learn more:

    use futures::future::Future;
    let fut2: Box<dyn Future<Output=Result<..., ....>> + Send> = Box::new(fut);

    You also need to make sure that there's no reference to self within your async block, so try to clone anything you might need before that. Otherwise you'll run into lifetime issues.

    let field = self.field.clone();
    
    let fut = async move {
        // ...
        let x = field.bar().await;
        // ...
    };

    (Sorry if this is off-topic; it seems useful to me before this crate is updated to use std::future.)

  18. ebkalderon commented on Feb 23, 2020

    @ebkalderon
    ContributorAuthor

    @Thegaram Thanks for the tips! Using your instructions, I was able to get async fn methods to be usable as RPC handlers in tower-lsp and support async/await (see ebkalderon/tower-lsp#101). However, there are multiple levels of nested boxing and pinning involved, since I am in turn converting the IoHandler::handle_request() future back into an std::future::Future, so I doubt this approach is resource efficient.

    I'm still holding out for jsonrpsee to gain a bidirectional stdio transport so I can switch tower-lsp over to it, but it doesn't appear to be a high priority at the moment.

  19. ebkalderon commented on Mar 7, 2020

    @ebkalderon
    ContributorAuthor

    @Thegaram I've found that you can eliminate the double nested future boxing by creating the following type alias to use in place of jsonrpc_core::BoxFuture:

    pub type BoxFuture<T> = Compat<Pin<Box<dyn Future<Output = Result<T, Error>> + Send>>>;

    Then you would change your RPC handler example to this instead:

    use futures::future::{FutureExt, TryFutureExt};
    
    type BoxFuture<T> = Compat<Pin<Box<dyn Future<Output = Result<T, Error>> + Send>>>;
    
    fn handler(&self, ...) -> BoxFuture<...> {
        // ...
    
        // this block should produce an `RpcResult`
        let fut = async move {
            // ...
            Ok(res)
        };
    
        fut.boxed().compat()
    }
  20. JanisGailis commented on May 25, 2020

    @JanisGailis

    Hi!

    Thanks for all the work involved in making this library!

    I'm exploring what options for using JSON-RPC in Rust there are for a project I'm working on now and I was wondering what's the status of converting jsonrpc-core to std::future::Future, or alternatively, what's the status of jsonrpsee?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions