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

Retry mechanism broken on WASM #1683

Closed
darioAnongba opened this issue Nov 13, 2024 · 10 comments
Closed

Retry mechanism broken on WASM #1683

darioAnongba opened this issue Nov 13, 2024 · 10 comments
Labels
bug Something isn't working

Comments

@darioAnongba
Copy link

Describe the bug
The retry mechanism get_with_retry does not work on the browser (wasm32-unknown-unknown) and fails with the following error:

error output:
        panicked at library/std/src/sys/pal/wasm/../unsupported/time.rs:31:9:
        time not implemented on this platform

The reason is that WebAssembly in browsers doesn’t have direct support for timing mechanisms like std::time::Instant or std::thread::sleep.

To Reproduce
Can run the tests on this repo for sync: https://github.com/darioAnongba/bdk-wasm

Expected behavior

Build environment

  • BDK tag/commit:
    • bdk_wallet = { version = "1.0.0-beta.5" }
    • bdk_esplora = { version = "0.19.0", features = ["async-https"] }
  • OS+version: macOS 14.7
  • Rust/Cargo version: 1.79
  • Rust/Cargo target: wasm32-unknown-unknown

Additional context
Using gloo-timers instead ?

use gloo_timers::future::sleep;
use std::time::Duration;

const BASE_BACKOFF_MILLIS: u64 = 100;

async fn get_with_retry(&self, url: &str) -> Result<Response, Error> {
    let mut delay = BASE_BACKOFF_MILLIS;
    let mut attempts = 0;

    loop {
        match self.client.get(url).send().await {
            Ok(resp) if attempts < self.max_retries && is_status_retryable(resp.status()) => {
                sleep(Duration::from_millis(delay)).await;
                attempts += 1;
                delay *= 2;
            }
            Ok(resp) => return Ok(resp),
            Err(e) => return Err(e), // Handle error if send fails
        }
    }
}

or 2 functions

#[cfg(not(target_arch = "wasm32"))]
async fn get_with_retry(...) {
    // Original retry logic with task::sleep
}

#[cfg(target_arch = "wasm32")]
async fn get_with_retry(...) {
    // Non-retrying version for WebAssembly
}
@darioAnongba darioAnongba added the bug Something isn't working label Nov 13, 2024
@ValuedMammal
Copy link
Contributor

I think it is also related to bitcoindevkit/rust-esplora-client#102

@ValuedMammal
Copy link
Contributor

@darioAnongba Is it possible to test syncing in bdk-wasm with this branch of bdk based on a proposed change to rust-esplora-client that makes the async client generic over the sleep implementation? https://github.com/ValuedMammal/bdk/tree/deps/esplora-client-103

@notmandatory notmandatory added this to BDK Nov 14, 2024
@notmandatory notmandatory moved this to Discussion in BDK Nov 14, 2024
@darioAnongba
Copy link
Author

yes I will test it ASAP and let you know

@oleonardolima
Copy link
Contributor

I'm not sure if it might be useful, but you can take a look at a partial implementation/usage of gloo-timers here, which builds on top of the PR ValuedMammal mentioned above, bitcoindevkit/rust-esplora-client#103.

@darioAnongba
Copy link
Author

darioAnongba commented Nov 19, 2024

Thanks @oleonardolima and @ValuedMammal but sorry I tried my best but couldn't implement a Sleeper that would work...
It works in your examples but when using sync or full_scan fails with:

the method `full_scan` exists for struct `Ref<'_, AsyncClient<GlooTimersSleeper>>`, but its trait bounds were not satisfied
the following trait bounds were not satisfied:
`TimeoutFuture: Send`
which is required by `AsyncClient<GlooTimersSleeper>: EsploraAsyncExt`
`TimeoutFuture: Sync`
which is required by `AsyncClient<GlooTimersSleeper>: EsploraAsyncExt`

I wasn't able to implement a Sleeper that is Send + Sync

The simple Sleeper is:

#[derive(Debug, Clone)]
struct GlooTimersSleeper;

impl Sleeper for GlooTimersSleeper {
    type Sleep = TimeoutFuture;

    fn sleep(dur: Duration) -> Self::Sleep {
        sleep(dur)
    }
}

I tried using something JSFuture instead of gloo_timers, wrapping Sleep in Pin and also using spawn_local from wasm_bindgen_futures but nothing worked...

@ValuedMammal
Copy link
Contributor

Hm yes I see that TimeoutFuture cannot be Send. I'm wondering if there's a known solution/workaround to this or if it's better to forgo the retry functionality when targeting WASM.

@darioAnongba
Copy link
Author

I find it strange that passing your own Reqwest client to the Esplora Client cannot fix this as Reqwest does not implement an embedded retry mechanism.

@RCasatta
Copy link
Member

@darioAnongba
Copy link
Author

darioAnongba commented Nov 20, 2024

Guys I managed to make it work. Tried with 100 stop gap full sync on testnet Blockstream and got a lot of 429 that triggered the timer many times. Here is the code:

#[derive(Clone)]
pub struct BrowserSleeper;

impl Sleeper for BrowserSleeper {
    type Sleep = SendSyncWrapper<TimeoutFuture>;

    fn sleep(dur: Duration) -> Self::Sleep {
        SendSyncWrapper(sleep(dur))
    }
}

and the wrapper (let me know your opinion on it):

// Wrap a future that is not `Send` or `Sync` and make it `Send` and `Sync`
pub struct SendSyncWrapper<F>(pub F);

unsafe impl<F> Send for SendSyncWrapper<F> {}
unsafe impl<F> Sync for SendSyncWrapper<F> {}

impl<F> Future for SendSyncWrapper<F>
where
    F: Future,
{
    type Output = F::Output;

    fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
        // SAFETY: Since we're in a single-threaded WASM environment, this is safe.
        unsafe {
            let this = self.get_unchecked_mut();
            Pin::new_unchecked(&mut this.0).poll(cx)
        }
    }
}

What are your plans to integrate the latest version of rust-esplora-client into the next beta release ? Is there going to be a beta.6 ? Is it going into master anytime soon ?

@notmandatory
Copy link
Member

Yes we're going to have a 1.0.0-beta.6 which should be the final release candidate for the final 1.0 and it will include an updated version of the rust-esplora-client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

5 participants