Skip to content

README use of FutureResult is out of date #596

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

Closed
jbaublitz opened this issue Nov 11, 2020 · 4 comments · Fixed by #632
Closed

README use of FutureResult is out of date #596

jbaublitz opened this issue Nov 11, 2020 · 4 comments · Fixed by #632
Labels

Comments

@jbaublitz
Copy link

I tried using FutureResult as specified in the README when trying to prototype a derived interface, but the jsonrpc_core::Error type cannot be used in FutureResult as suggested by the README. I've dug into the code a little bit more and it appears that it is expecting the second type parameter to FutureResult to be a Future which jsonrpc_core::Error is not. Would you be able to update the README or provide a suggestion for how to work around this?

@niklasad1
Copy link
Contributor

niklasad1 commented Nov 11, 2020

Which version/branch are you using?

#565 updated to futures v0.3 and the README might be outdated.

//cc @tomusdrw

@jbaublitz
Copy link
Author

I'm using the latest released version which appears to be 15.1.0. I've checked both that branch and the master branch to see if there was a difference, but they both appear to be out of date. I tried playing around with this a little bit, but it appears that the derive macro only accepts the jsonrpc_core::Result or jsonrpc_core::FutureResult type as a return type for the trait methods.

@tomusdrw
Copy link
Contributor

@jbaublitz master example indeed won't work with 15.1.0. Can you post a code sample which you have problem with and the compiler error?

This code uses 15.1.x:
https://github.com/paritytech/substrate/blob/53ab714d03babdcb5538232a7c36ca89266180e6/client/rpc-api/src/state/mod.rs#L43

FutureResult is defined like so:
https://github.com/paritytech/substrate/blob/53ab714d03babdcb5538232a7c36ca89266180e6/client/rpc-api/src/state/error.rs#L28

@tomusdrw tomusdrw added the docs label Nov 13, 2020
@jbaublitz
Copy link
Author

This is no longer an issue for me. Closing.

nevir added a commit to nevir/jsonrpc that referenced this issue Aug 16, 2021
Resolves paritytech#596

This grabs the example server implementation from [the root README](https://github.com/paritytech/jsonrpc#basic-usage-with-http-transport), as the given example ([rendered in docs.rs](https://paritytech.github.io/jsonrpc/jsonrpc_http_server/index.html)) currently fails with the following:

```
error[E0277]: `Result<jsonrpc_http_server::jsonrpc_core::Value, _>` is not a future
 --> src/main.rs:6:5
  |
6 |     io.add_method("say_hello", |_| {
  |        ^^^^^^^^^^ `Result<jsonrpc_http_server::jsonrpc_core::Value, _>` is not a future
  |
  = help: the trait `std::future::Future` is not implemented for `Result<jsonrpc_http_server::jsonrpc_core::Value, _>`
  = note: required because of the requirements on the impl of `RpcMethodSimple` for `[closure@src/main.rs:6:29: 8:3]`
```
niklasad1 pushed a commit that referenced this issue Aug 18, 2021
)

Resolves #596

This grabs the example server implementation from [the root README](https://github.com/paritytech/jsonrpc#basic-usage-with-http-transport), as the given example ([rendered in docs.rs](https://paritytech.github.io/jsonrpc/jsonrpc_http_server/index.html)) currently fails with the following:

```
error[E0277]: `Result<jsonrpc_http_server::jsonrpc_core::Value, _>` is not a future
 --> src/main.rs:6:5
  |
6 |     io.add_method("say_hello", |_| {
  |        ^^^^^^^^^^ `Result<jsonrpc_http_server::jsonrpc_core::Value, _>` is not a future
  |
  = help: the trait `std::future::Future` is not implemented for `Result<jsonrpc_http_server::jsonrpc_core::Value, _>`
  = note: required because of the requirements on the impl of `RpcMethodSimple` for `[closure@src/main.rs:6:29: 8:3]`
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants