Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: use url as cache key which contains query params #68

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

alfetopito
Copy link
Contributor

Summary

Now that the caching was fixed on #67, the next issue was exposed.

Unfortunately I only noticed after it was already deployed to prod :(

Right now, the cache is the SAME for all requests.
That's because the cache key used in the request path: req.routerPath which is /proxies/coingecko/* for all requests.

Changed to req.url which does contain the full url path including the search params. E.g.: /proxies/coingecko/simple/token_price/ethereum?contract_addresses=0x6B175474E89094C44Da98b954EedeAC495271d0F&vs_currencies=usd

Testing

  1. Start the server locally with yarn start
  2. Query one address: curl -X GET http://localhost:3000/proxies/coingecko/simple/token_price/ethereum?contract_addresses=0x6B175474E89094C44Da98b954EedeAC495271d0F&vs_currencies=usd
  • Should receive the price for the asset
  1. Query the same asset again
  • Should receive the same response
  1. Query a different asset: curl -X GET http://localhost:3000/proxies/coingecko/simple/token_price/ethereum?contract_addresses=0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2&vs_currencies=usd
  • Should have a DIFFERENT price and the token address in the response should match the one in the query
image

@alfetopito alfetopito self-assigned this Jul 25, 2024
@alfetopito alfetopito requested a review from a team July 25, 2024 15:32
@alfetopito
Copy link
Contributor Author

Tested on infra with https://app.pulumi.com/cowprotocol/bff/staging/updates/183

All good, proceeding with release. Please post merge review.

@alfetopito alfetopito merged commit 68e22c5 into v0.6 Jul 25, 2024
4 checks passed
@alfetopito alfetopito deleted the hotfix/v0.6.5 branch July 25, 2024 15:40
@@ -74,7 +73,7 @@ export const bffCache: FastifyPluginCallback<BffCacheOptions> = (fastify, opts,
};

function getKey(req: FastifyRequest) {
return `GET:${req.routerPath}`;
return `GET:${req.url}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

🙈

Thanks

anxolin added a commit that referenced this pull request Jul 26, 2024
* chore: hotfix v0.6.4 (#67)

* fix: consume the payload stream before caching it

* fix: fix start script

* fix: use url as cache key which contains query params (#68)

* Make tests pass

---------

Co-authored-by: Leandro <alfetopito@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants