Skip to content

fix: handle coingecko 404 responses gracefully #3914

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.

Sign up for GitHub

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

Open
kev1n-peters wants to merge 1 commit into
base: development
Choose a base branch
from
Open

fix: handle coingecko 404 responses gracefully #3914

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 39 additions & 20 deletions src/utils/coingecko.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,15 @@ const coingeckoRequest = async (
signal: params?.abort?.signal,
headers,
})
.then((resp) => resp.json())
.then((resp) => {

@priyajeet priyajeet Nov 20, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

if (!resp.ok) {
console.error(
`Coingecko request failed with status ${resp.status}: ${path}`,
);
return null;

@priyajeet priyajeet Nov 20, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why not throw since we are already logging below and returning null

}
return resp.json();
})
.catch((err) => {
console.error('Error fetching from Coingecko', err);
return null;
Expand Down Expand Up @@ -136,7 +144,13 @@ export const fetchTokenPrices = async (
params,
)
.then((data) => {
if (data['error'] !== undefined || data['error_code'] !== undefined) {
if (data === null) {
// Request failed (e.g., 404, network error)
resolve([]);

@priyajeet priyajeet Nov 20, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

} else if (
data['error'] !== undefined ||
data['error_code'] !== undefined
) {
reject(data['error']);
} else {
resolve(
Expand Down Expand Up @@ -186,24 +200,29 @@ export const fetchTokenPrices = async (
`/api/v3/simple/price?ids=${ids.join(',')}&vs_currencies=usd`,
params,
)
.then((data) =>
resolve(
nativeTokens
.map((tokenId) => {
const cgid = NATIVE_TOKEN_IDS[tokenId.chain];
if (cgid) {
const { usd } = data[cgid];
return {
tokenId,
price: usd,
};
} else {
return null;
}
})
.filter((v) => !!v),
),
)
.then((data) => {
if (data === null) {
// Request failed (e.g., 404, network error)
resolve([]);
} else {
resolve(
nativeTokens
.map((tokenId) => {
const cgid = NATIVE_TOKEN_IDS[tokenId.chain];
if (cgid) {
const { usd } = data[cgid];
return {
tokenId,
price: usd,
};
} else {
return null;
}
})
.filter((v) => !!v),
);
}
})
.catch(reject);
}),
);
Expand Down
Loading