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

Octocrab doesn't compile with Yew (for me) #224

Open
NicMcPhee opened this issue Jul 9, 2022 · 13 comments
Open

Octocrab doesn't compile with Yew (for me) #224

NicMcPhee opened this issue Jul 9, 2022 · 13 comments

Comments

@NicMcPhee
Copy link

I was hoping to use Octocrab on a Yew-based project that I'm working on, but when I added the octocrab dependency to my Cargo.toml file, octocrab failed to compile.

I created a small demo app that illustrates the issue: https://github.com/NicMcPhee/octocrab_test This is about the simplest possible Yew app, and if you uncomment the octocrab dependency in Cargo.toml and run trunk serve, you get a whole pile of compilation errors when it's trying to build the octocrab code (see below).

I'm guessing it's some kind of dependency conflict, but I really don't know.

Yew + Octocrab seems like an "obvious" combination, so it was definitely a bummer that this didn't work. Any suggestions or advice would definitely be appreciated.

Rust versions, etc.

rustup show provides the following output if that's helpful:

Default host: x86_64-apple-darwin
rustup home:  /Users/mcphee/.rustup

installed targets for active toolchain
--------------------------------------

wasm32-unknown-unknown
x86_64-apple-darwin

active toolchain
----------------

stable-x86_64-apple-darwin (default)
rustc 1.62.0 (a8314ef7d 2022-06-27)

The error text

   Compiling octocrab v0.16.0
error[E0599]: no method named `user_agent` found for struct `ClientBuilder` in the current scope
   --> /Users/mcphee/.cargo/registry/src/github.com-1ecc6299db9ec823/octocrab-0.16.0/src/lib.rs:338:14
    |
338 |             .user_agent("octocrab")
    |              ^^^^^^^^^^ method not found in `ClientBuilder`

error[E0599]: no method named `user_agent` found for struct `ClientBuilder` in the current scope
   --> /Users/mcphee/.cargo/registry/src/github.com-1ecc6299db9ec823/octocrab-0.16.0/src/lib.rs:432:18
    |
432 |                 .user_agent("octocrab")
    |                  ^^^^^^^^^^ method not found in `ClientBuilder`

error: future cannot be sent between threads safely
  --> /Users/mcphee/.cargo/registry/src/github.com-1ecc6299db9ec823/octocrab-0.16.0/src/from_response.rs:11:80
   |
11 |       async fn from_response(response: reqwest::Response) -> crate::Result<Self> {
   |  ________________________________________________________________________________^
12 | |         let text = response.text().await.context(crate::error::HttpSnafu)?;
13 | |
14 | |         let de = &mut serde_json::Deserializer::from_str(&text);
15 | |         serde_path_to_error::deserialize(de).context(crate::error::JsonSnafu)
16 | |     }
   | |_____^ future created by async block is not `Send`
   |
   = help: within `impl Future<Output = std::result::Result<T, error::Error>>`, the trait `Send` is not implemented for `*mut u8`
note: captured value is not `Send`
  --> /Users/mcphee/.cargo/registry/src/github.com-1ecc6299db9ec823/octocrab-0.16.0/src/from_response.rs:11:28
   |
11 |     async fn from_response(response: reqwest::Response) -> crate::Result<Self> {
   |                            ^^^^^^^^ has type `Response` which is not `Send`
   = note: required for the cast to the object type `dyn Future<Output = std::result::Result<T, error::Error>> + Send`

error: future cannot be sent between threads safely
  --> /Users/mcphee/.cargo/registry/src/github.com-1ecc6299db9ec823/octocrab-0.16.0/src/from_response.rs:11:80
   |
11 |       async fn from_response(response: reqwest::Response) -> crate::Result<Self> {
   |  ________________________________________________________________________________^
12 | |         let text = response.text().await.context(crate::error::HttpSnafu)?;
13 | |
14 | |         let de = &mut serde_json::Deserializer::from_str(&text);
15 | |         serde_path_to_error::deserialize(de).context(crate::error::JsonSnafu)
16 | |     }
   | |_____^ future created by async block is not `Send`
   |
   = help: within `impl Future<Output = std::result::Result<T, error::Error>>`, the trait `Send` is not implemented for `Rc<RefCell<wasm_bindgen_futures::Inner>>`
note: future is not `Send` as this value is used across an await
  --> /Users/mcphee/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.11.11/src/wasm/mod.rs:22:41
   |
22 |     let js_val = JsFuture::from(promise).await.map_err(crate::error::wasm)?;
   |                  -----------------------^^^^^^ await occurs here, with `JsFuture::from(promise)` maybe used later
   |                  |
   |                  has type `wasm_bindgen_futures::JsFuture` which is not `Send`
note: `JsFuture::from(promise)` is later dropped here
  --> /Users/mcphee/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.11.11/src/wasm/mod.rs:22:76
   |
22 |     let js_val = JsFuture::from(promise).await.map_err(crate::error::wasm)?;
   |                                                                            ^
   = note: required for the cast to the object type `dyn Future<Output = std::result::Result<T, error::Error>> + Send`

error: future cannot be sent between threads safely
   --> /Users/mcphee/.cargo/registry/src/github.com-1ecc6299db9ec823/octocrab-0.16.0/src/page.rs:88:80
    |
88  |       async fn from_response(response: reqwest::Response) -> crate::Result<Self> {
    |  ________________________________________________________________________________^
89  | |         let HeaderLinks {
90  | |             first,
91  | |             prev,
...   |
126 | |         }
127 | |     }
    | |_____^ future created by async block is not `Send`
    |
    = help: within `impl Future<Output = std::result::Result<page::Page<T>, error::Error>>`, the trait `Send` is not implemented for `*mut u8`
note: captured value is not `Send`
   --> /Users/mcphee/.cargo/registry/src/github.com-1ecc6299db9ec823/octocrab-0.16.0/src/page.rs:88:28
    |
88  |     async fn from_response(response: reqwest::Response) -> crate::Result<Self> {
    |                            ^^^^^^^^ has type `Response` which is not `Send`
    = note: required for the cast to the object type `dyn Future<Output = std::result::Result<page::Page<T>, error::Error>> + Send`

error: future cannot be sent between threads safely
   --> /Users/mcphee/.cargo/registry/src/github.com-1ecc6299db9ec823/octocrab-0.16.0/src/page.rs:88:80
    |
88  |       async fn from_response(response: reqwest::Response) -> crate::Result<Self> {
    |  ________________________________________________________________________________^
89  | |         let HeaderLinks {
90  | |             first,
91  | |             prev,
...   |
126 | |         }
127 | |     }
    | |_____^ future created by async block is not `Send`
    |
    = help: within `impl Future<Output = std::result::Result<page::Page<T>, error::Error>>`, the trait `Send` is not implemented for `Rc<RefCell<wasm_bindgen_futures::Inner>>`
note: future is not `Send` as this value is used across an await
   --> /Users/mcphee/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.11.11/src/wasm/mod.rs:22:41
    |
22  |     let js_val = JsFuture::from(promise).await.map_err(crate::error::wasm)?;
    |                  -----------------------^^^^^^ await occurs here, with `JsFuture::from(promise)` maybe used later
    |                  |
    |                  has type `wasm_bindgen_futures::JsFuture` which is not `Send`
note: `JsFuture::from(promise)` is later dropped here
   --> /Users/mcphee/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.11.11/src/wasm/mod.rs:22:76
    |
22  |     let js_val = JsFuture::from(promise).await.map_err(crate::error::wasm)?;
    |                                                                            ^
    = note: required for the cast to the object type `dyn Future<Output = std::result::Result<page::Page<T>, error::Error>> + Send`

error: future cannot be sent between threads safely
   --> /Users/mcphee/.cargo/registry/src/github.com-1ecc6299db9ec823/octocrab-0.16.0/src/models/repos.rs:119:80
    |
119 |       async fn from_response(response: reqwest::Response) -> crate::Result<Self> {
    |  ________________________________________________________________________________^
120 | |         let json: serde_json::Value = response.json().await.context(crate::error::HttpSnafu)?;
121 | |
122 | |         if json.is_array() {
...   |
132 | |         }
133 | |     }
    | |_____^ future created by async block is not `Send`
    |
    = help: within `impl Future<Output = std::result::Result<ContentItems, error::Error>>`, the trait `Send` is not implemented for `*mut u8`
note: captured value is not `Send`
   --> /Users/mcphee/.cargo/registry/src/github.com-1ecc6299db9ec823/octocrab-0.16.0/src/models/repos.rs:119:28
    |
119 |     async fn from_response(response: reqwest::Response) -> crate::Result<Self> {
    |                            ^^^^^^^^ has type `Response` which is not `Send`
    = note: required for the cast to the object type `dyn Future<Output = std::result::Result<ContentItems, error::Error>> + Send`

error: future cannot be sent between threads safely
   --> /Users/mcphee/.cargo/registry/src/github.com-1ecc6299db9ec823/octocrab-0.16.0/src/models/repos.rs:119:80
    |
119 |       async fn from_response(response: reqwest::Response) -> crate::Result<Self> {
    |  ________________________________________________________________________________^
120 | |         let json: serde_json::Value = response.json().await.context(crate::error::HttpSnafu)?;
121 | |
122 | |         if json.is_array() {
...   |
132 | |         }
133 | |     }
    | |_____^ future created by async block is not `Send`
    |
    = help: within `impl Future<Output = std::result::Result<ContentItems, error::Error>>`, the trait `Send` is not implemented for `Rc<RefCell<wasm_bindgen_futures::Inner>>`
note: future is not `Send` as this value is used across an await
   --> /Users/mcphee/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.11.11/src/wasm/mod.rs:22:41
    |
22  |     let js_val = JsFuture::from(promise).await.map_err(crate::error::wasm)?;
    |                  -----------------------^^^^^^ await occurs here, with `JsFuture::from(promise)` maybe used later
    |                  |
    |                  has type `wasm_bindgen_futures::JsFuture` which is not `Send`
note: `JsFuture::from(promise)` is later dropped here
   --> /Users/mcphee/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.11.11/src/wasm/mod.rs:22:76
    |
22  |     let js_val = JsFuture::from(promise).await.map_err(crate::error::wasm)?;
    |                                                                            ^
    = note: required for the cast to the object type `dyn Future<Output = std::result::Result<ContentItems, error::Error>> + Send`
@XAMPPRocky
Copy link
Owner

XAMPPRocky commented Jul 9, 2022

Thank you for your issue! I'm afraid AFAICT, there's not much that I can do to fix this issue, as it seems to be an issue with reqwest's futures. I'm not the one storing a *mut u8 or Rc<RefCell<wasm_bindgen_futures::Inner>> in the future. I agree with you that these should work together, which is why I eventually want to remove reqwest as a dependency (see #99), so people can use their clients and even if reqwest doesn't work in your environment, you can use another HTTP client that does.

@NicMcPhee
Copy link
Author

Thanks for the prompt response! I'm sad that Octocrab and Yew aren't gonna be friends for a bit, but I get that it's not always easy to get different tools to play nice together.

I'm pretty new to Rust and am just working on my project as a way to learning Rust. I have a lot of programming experience, but more in spaces like Java and Clojure. How hard do you feel the migration away from reqwest will be? Is that something that I could contribute to in a meaningful way? Or should I just try something else like Octorust?

Thanks for your thoughts and work!

@XAMPPRocky
Copy link
Owner

Is that something that I could contribute to in a meaningful way? Or should I just try something else like Octorust?

Absolutely, contributions are always welcome and encouraged.

How hard do you feel the migration away from reqwest will be?

It's fairly straightforward in terms of complexity, as all we're doing is replacing request::Client with Arc<dyn tower::Service<http::Request, Response=http::Response, Error=Box<dyn std::error::Error>>, and migrating the methods to use that. It just needs someone to dedicate the time which I haven't been able to.

If you want to take this, and you get stuck, or don't understand something, please feel free to post in the issue and I'll try my best to help. I'd recommend looking at kube-client's code if you need something to start from, as we essentially want the same thing, but for Octocrab.

@NicMcPhee
Copy link
Author

Thanks – I might have a look at it, but no promises for anything (fast). 😜

In #99 you suggest trying to address this one endpoint or group at a time. Any suggestions on a good "simple" endpoint to start with just so I can convince myself I understand the issues at hand?

@XAMPPRocky
Copy link
Owner

They’re all roughly the same complexity because the code is very simple. Thinking about it now, I would maybe take a different approach of just changing the HTTP methods (e.g. Octocrab::get and Octocrab::_get) and that should make it work for all endpoints.

@NicMcPhee
Copy link
Author

Looking at lib.rs, it seems like everything ultimately goes through Octocrab::execute. So one option would be to just rewrite execute to use http, tower, etc., and then chase all the bazillion resulting compiler errors until everything works again. That would get everything done, but I could imagine that it might bleed change across fairly large chunks of the project and end up being a pretty big PR.

Another option would perhaps be to add a new http_execute function (or some other name as you prefer) that returns the non-reqwest response type. We could then deprecate the existing execute(), and work through the dependencies in a more one-at-a-time fashion.

Thoughts? Preferences?

@Carreau
Copy link

Carreau commented Sep 3, 2022

I have a similar issue trying to compile to wasm32-unknown-unknown for use in cloudflare workers – also learning rust. Though I am mostly interested only using the structs in octocrab::models. Is there any way to feature-gate reqwest/FromResponse, or only compile only the the structs ?

Maybe make the structs themselves part of a different module/crate that can be compiled separately ? Sorry not too quite sure what it possible.

@MRuecklCC
Copy link

There is also surf which abstracts different HTTP client implementations behind a common interface. It claims to support wasm environment.

@IgnisDa
Copy link

IgnisDa commented Mar 30, 2023

@XAMPPRocky Now that #297 has been merged, I think it might be possible to support wasm32-unknown-unknown target. Would be great if this was possible.

Right now having octocrab = { git = "https://github.com/XAMPPRocky/octocrab.git", rev = "dd02ea", default-features = false } as a dependency gives a lot of errors:

image

The main problem is the mio dependency.

@XAMPPRocky
Copy link
Owner

The main problem is the mio dependency.

That seems like a bug, we don't use mio directly, so a dependency we're using is incorrectly depending on mio for wasm.

@martinitus
Copy link

I was playing around with a similar idea last year. I think to make that work, one would have to implement the tower::Service<Request<B>> based on the websys crate. I had a look at gloo-net which does ship its own client implementation based on websys (here: https://github.com/rustwasm/gloo/blob/master/crates/net/src/http/mod.rs). It might be a good starting point.
Essentially if such an implementation would exist, this would solve the same problem for all kinds of API client libraries (not only octocrab). I even asked in gloo/wasm/tower discord, there seems to be no such thing right now and i was encouraged to give it a try. Maybe you can ask there again :)

@IgnisDa
Copy link

IgnisDa commented Mar 30, 2023

So right now, it is still not possible to use octocrab with wasm32-unknown-unknown right?

@Tehnix
Copy link

Tehnix commented Aug 9, 2024

Since #591 brought in the feature flag default-client, we can now avoid the parts that are incompatible with WASM.

E.g.

# Disable default features since default-client is incompatible with WASM targets.
octocrab = { version = "0.39.0", default-features = false, features = [
  "follow-redirect",
  "retry",
  "rustls",
  "timeout",
  "tracing",
] }

However, now we'll need to construct a compatible client that can be used in place of the original default client.

I more of less got as far as:

    let octocrab = octocrab::OctocrabBuilder::new_empty()
        .with_service(client)
        .with_layer(&BaseUriLayer::new(Uri::from_static(
            "https://api.github.com",
        )))
        .with_layer(&ExtraHeadersLayer::new(Arc::new(vec![(
            USER_AGENT,
            "octocrab".parse().unwrap(),
        )])))
        .with_auth(octocrab::auth::Auth::PersonalToken(
            api_token.to_owned().into(),
        ))
        .build()?;

but still need something to plug in as the Client, and have been failing to find a good solution so far 😅

Has anyone succeeded in this for WASM targets?

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

8 participants
@NicMcPhee @Carreau @Tehnix @XAMPPRocky @martinitus @IgnisDa @MRuecklCC and others