fix: handle coingecko 404 responses gracefully#3914
Conversation
✅ Deploy Preview for wormhole-connect-mainnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for wormhole-connect ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| headers, | ||
| }) | ||
| .then((resp) => resp.json()) | ||
| .then((resp) => { |
There was a problem hiding this comment.
This file is quite nested and is hard to read, but has a deterministic output. I think it would greatly benefit from claude auto-refactoring. Before refactoring we can get claude to write a whole bunch of tests, then get claude to refactor and tests should still work. Obvious fixes can be switching to async/await, moving coingecko call out for fetching native vs non native tokens, moving request building and response parsing/mapping out as functions, reduce loops after loops (for eg last few lines maps, filters, flats and still loops), etc.
| console.error( | ||
| `Coingecko request failed with status ${resp.status}: ${path}`, | ||
| ); | ||
| return null; |
There was a problem hiding this comment.
Why not throw since we are already logging below and returning null
| if (data['error'] !== undefined || data['error_code'] !== undefined) { | ||
| if (data === null) { | ||
| // Request failed (e.g., 404, network error) | ||
| resolve([]); |
There was a problem hiding this comment.
Why resolve and not reject like other errors below? Or inversely, why reject below and not return []. In the end all its checking if its fullfilled or not.
2 participants
fixes seeing errors in the console related to coins not being found on coingecko, e.g.: