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 getting block date and price #9

Merged
merged 2 commits into from
Jan 31, 2024

Conversation

KirillLykov
Copy link
Contributor

While using sys account add or sys sync I often get error Error: reqwest::Error { kind: Decode, source: Error("missing field id", line: 1, column: 187) }. It happens when get_block_date_and_price fails and because intermediate progress is not written to db, user has to restart the whole process from the begging.
This simple change adds retry logic around aforementioned call.

@mvines
Copy link
Owner

mvines commented Jan 24, 2024

Thanks! get_block_date_and_price() internally touches two network resources:

  1. Solana RPC endpoint: rpc_client.get_block_time() (
    let block_time = rpc_client.get_block_time(slot)?;
    )
  2. Coingecko endpoint: coin_gecko::get_historical_price() (
    pub async fn get_historical_price(
    )

Are you seeing both (1) and (2) fail? My guess is that it's mostly (2)?
I'm thinking it might be better to push the retrying down a level to either (1) and/or (2), to be more network conscious and avoid retrying both (1) and (2) if it's only one of the two that's actually failing

@KirillLykov
Copy link
Contributor Author

@mvines moved this code to get_price as suggested

@mvines
Copy link
Owner

mvines commented Jan 31, 2024

Thanks! I'll add a follow-up commit about potentially moving the retry even lower. Ideally we'd do it from coin_gecko.rs and honour the exact http backoff time that their API is responding with one day

@mvines mvines merged commit 6192bbd into mvines:master Jan 31, 2024
1 check passed
mvines added a commit that referenced this pull request Jan 31, 2024
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

Successfully merging this pull request may close these issues.

2 participants