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

Javascript breaking data when using Fetch API and Serde Serialization/Deserialization .json() #2894

Closed
2 of 8 tasks
vpochapuis opened this issue Sep 29, 2022 · 9 comments
Closed
2 of 8 tasks

Comments

@vpochapuis
Copy link

This is about:

  • A typo
  • Innaccurate/misleading documentation (e.g. technically incorrect advice)
  • Undocumented code
  • Outdated documentation
  • Other

Problem

When building JSON APIs using Rust on both the Backend and Frontend, one would expect that the Serialization and Deserialization of any Serde Derived Struct would be without issues. However, Javascript JSON evaluation used by gloo_net::http::Response with the .json() call, is limited by Javascript capabilities. See 64bit number storage issue in JS over JSON.
When I built my APIs following this tutorial (which is great BTW, just I think this specific issue could hit other people than me), I had a Rust Struct containing a i64 value that was at i64::MAX. And to my big surprise the JSON RAW value was correct: 9223372036854775807, but the .json() value from the Response was 9223372036854776000 hence triggering a Parsing error in the Serde library.
The solution i found (might not be the best, I would be pleased to get feedbacks, I am very new to Web Development), is to get the raw body content using the .text() function on the Response, and then use serde_json::from_str() on that String, hence bypassing any Evaluation of the content by Javascript, and avoiding the flaw (thats why we use Rust in the WebBrowser right? XD ).

Details about the solution you'd like

Maybe precise in the tutorial that to avoid issues related to Javascript JSON evaluation, one could use .text() instead of .json() and use the serde_json::from_str() call instead to make sure that if Serde can Serialize on the Backend, it can also Deserialize it in the Frontend.

Questionaire (Optional)

  • I'd like to write this documentation if you teach me how to do? Or you can do it (I am not sure whether I would phrase it the best)
  • I'd like to write this documentation but I'm not sure what's needed
  • I don't have time to add this right now, but maybe later

Others
Thank you very much for the Yew Framework~~
And if you think i am wrong or there is any details i missed, please let me know, i would be happy to learn more!

Wish you a nice day!

@WorldSEnder
Copy link
Member

WorldSEnder commented Sep 29, 2022

Thanks for the report. I would say that I actually expected "better" from the gloo_net library, since this could be handled under the hood without having to rely on the underlying web_sys::Response::json method which is just a projection of the web IDL Api, i.e. calling the javascript environment. The current behavior strikes me as counter to the initial goal of gloo of provided convenient, instead of just 1-to-1, wrappers of the js api.

@hamza1311 might have an opinion on this, maybe it makes sense to upstream the issue to gloo?

@WorldSEnder WorldSEnder assigned hamza1311 and unassigned hamza1311 Sep 29, 2022
@vpochapuis
Copy link
Author

Its my pleasure to report my findings and I hope it will help the community. And at the same time hopefully I can learn more about bug reporting and how to contribute while following the resolution of this issue~

@vpochapuis
Copy link
Author

I am terribly sorry i just found out that in my use statements it wasnt gloo_net::http::Request but reqwasm::http::Request. Most likely this is a non-issue! I will check and let you know

@vpochapuis
Copy link
Author

Wow my honor is safe (kinda (^ ▽ ^;) ) , the bug is also present in gloo_net is seems like XD

@vpochapuis
Copy link
Author

Ah I know why: in the reqwasm crate This re-exports all the API from [gloo-net](https://crates.io/crates/gloo-net) crate.

@hamza1311
Copy link
Member

hamza1311 commented Sep 29, 2022

Can you try response.text() and use serde_json on the string? If that solves the issue, it would be great if you could PR a fix into gloo-net.

It might be better to use .bytes() method on response instead of .text() but I don't exactly remember how serde handles that

@vpochapuis
Copy link
Author

vpochapuis commented Sep 29, 2022

Yes that's what I did and explained in the original Issue post to fix the issue and it is working as intended as its not using the Javascript JSON Eval~

However I dont know how should I do to fix it in gloo-net

@vpochapuis
Copy link
Author

vpochapuis commented Sep 29, 2022

I will look into it tomorrow

I see where:

/// Reads the response to completion, parsing it as JSON.
    #[cfg(feature = "json")]
    #[cfg_attr(docsrs, doc(cfg(feature = "json")))]
    pub async fn json<T: DeserializeOwned>(&self) -> Result<T, Error> {
        let promise = self.response.json().map_err(js_to_error)?; // --> Here
        let json = JsFuture::from(promise).await.map_err(js_to_error)?;
        Ok(JsValueSerdeExt::into_serde(&json)?)
    }

Maybe with this

/// Reads the response to completion, parsing it as JSON.
    #[cfg(feature = "json")]
    #[cfg_attr(docsrs, doc(cfg(feature = "json")))]
    pub async fn json<T: DeserializeOwned>(&self) -> Result<T, Error> {
        serde_json::from_str::<T>(&self.text().await?).map_err(Error::from)
    }

@hamza1311
Copy link
Member

Closing this as this has been resolved

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

No branches or pull requests

3 participants