search

thomaseizinger / rust-jsonrpc-client

A macro-driven JSON-RPC client for Rust.
https://docs.rs/jsonrpc_client
4 stars 6 forks source link

Add `ureq` backend #30

Closed tcharding closed 3 years ago

tcharding commented 3 years ago

Add a ureq backend. Implementation is basically identical to the reqwest backend (incl. a similar example module).

tcharding commented 3 years ago

Thanks!

ureq is a blocking client so this is unfortunately incompatible with async unless we add an executor in or provide an abstraction that allows spawning blocking tasks.

Thoughts?

I've been mulling this over since I first read it a couple of days ago. What's got me stumped is that I tested this patch with a real project. I.e., I have project using this lib and I patched the lib then tested it, unless I forgot to change the manifest and wasn't using the patched version or something brain dead like that. I'll be back in touch once I've played with it some more.

Thanks

thomaseizinger commented 3 years ago

I am not saying it doesn't technically work :)

An async function, once spawned, translates to a task in a runtime like tokio. Depending on the configuration, you will have one or more executor threads that run a lot of these tasks, constantly switching between them as they become ready, much like the kernel schedular switches between threads except that task switches are a lot cheaper. Look up some benchmarks if you are interested.

The problem is that ureq will block the current thread on IO. If your app is thread-based, that is cool because every other code that needs to run concurrently is in another thread.

If you are in async/await world, the runtime is executing hundreds of tasks on a single thread. If one of these tasks blocks on IO, everything on this executor thread is stalled.

tcharding commented 3 years ago

Oh I'm with you now, thanks for the explanation. I was assuming if one wanted to use ureq they were operating in an environment without async, no one does sync IO in Rust for fun. How about putting ureq behind a feature gate sync, putting tokio and all the rest behind async and setting default features to async? Would that be useful or do you think that the usecase of non-async IO is so limited as to not warrant it?

thomaseizinger commented 3 years ago

If we wanted to support that, I would also want to make the function signature conditionally async / non-async.

But generally, I don't think it is a good idea. Features should be additive and async/non-async would not be compatible. It would mean we can't have sync and async in a single dependency tree and that is not acceptable.

Hence for simplicity, I'd like to support only async backends. Why do you specifically need ureq?

tcharding commented 3 years ago

Hence for simplicity, I'd like to support only async backends.

No worries.

Why do you specifically need ureq?

I'm using SGX,. The solution consists of two components, a trusted component that runs in an SGX enclave and an untrusted component that is 'normal'. We are using rust-jsonrpc-client in the untrusted component and writing json rpc clients by hand in the trusted component. I'd like to use rust-jsonrpc-client in the trusted component also. For now, I'll probably maintain a fork that uses ureq. If I make any other changes I'll push them up for to look at though.

Closing this PR.

Cheers, Tobin.

thomaseizinger commented 3 years ago

Thanks for clarifying.

I guess you are using ureq because it is the only one that compiles on the SGX target? Have you investigated what would be necessary to make any of the supported clients compile on SGX?

For now, I'll probably maintain a fork that uses ureq. If I make any other changes I'll push them up for to look at though.

I'd recommend to remove everything in regards to async from your fork then to avoid future confusion and problems.

I am not quite sure how it would play out but I'd be open to have support for a complete sync interface under the following conditions:

In other words, if you end up make a blocking-IO version of rust-jsonrpc-client, I am happy to consider integrating it.

tcharding commented 3 years ago

I guess you are using ureq because it is the only one that compiles on the SGX target?

Yes, we use Fortanix's implementation and it does not have async/await - sad, I know.

Cheers mate