Skip to content
This repository was archived by the owner on Mar 24, 2025. It is now read-only.

Conversation

@rubenmarcus
Copy link
Contributor

promise.any calls both apis at same type and coingecko has ratelimits that make it call many times with error.. Binance always give 200.

Screenshot 2024-07-14 at 23 18 06

Copy link
Member

@bh2smith bh2smith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the proposed solution does eliminate console errors, it does so by swallowing errors and requests from the different price feeds asymmetrically. So when we do see the rate limit errors (which we will) we will only ever see the BinanceAPI error.

I think this problem could be addressed more appropriately with some basic caching. Our servicies shouldn't be requesting the price of near more than once every minute or two. I would propose using something like https://www.npmjs.com/package/cache-manager with an expiry of 120 seconds.

Oh - I just realized that this is mintbase-js. The caching should be done on the application layer.

@rubenmarcus
Copy link
Contributor Author

rubenmarcus commented Jul 15, 2024

@bh2smith
The caching should be done on the application layer.

you already answered yourself.

Binance doesnt have rate limit, as I said on desc. Coingecko has.

Comment on lines 19 to 24
let res: NearPriceData;
try {
res = await fetchPrice<CoinGeckoNearPriceData>(COIN_GECKO_API, data => ({ price: data.near.usd }));
} catch (err) {
res = await fetchPrice<NearPriceData>(BINANCE_API, data => data);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are prioritizing CoinGecko by calling it first and swallowing their errors.

These two APIS are different. Just keep the Promise.any, catch rate limit errors and handle them differently (like return null).

Have you considered adding another API?

https://coinmarketcap.com/api/pricing/

https://coinpaprika.com/api/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered adding another API?

https://coinmarketcap.com/api/pricing/

https://coinpaprika.com/api/

this is something I needed to do later, user can pass it api and we fetch for him.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2024-07-15 at 11 00 11

@rubenmarcus rubenmarcus merged commit bde29d4 into beta Jul 15, 2024
@bh2smith bh2smith deleted the fix-fetch-token-price branch November 19, 2024 14:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants