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.

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

fix: handle api 404 error to display data properly #1802

Merged
merged 1 commit into from
Oct 26, 2024
Merged

fix: handle api 404 error to display data properly #1802

Show file tree
Hide file tree
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
40 changes: 34 additions & 6 deletions src/models/api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ export async function getFeaturedGrowers() {
const res = await axios.get(url);
const data = await res.data;
log.warn('url:', url, 'took:', Date.now() - begin);
return data.grower_accounts;
return data.grower_accounts || [];
} catch (err) {
log.error(err.message);
throw err;
log.error('Error in getFeaturedGrowers:', err.message);
return [];
}
}

Expand All @@ -24,10 +24,10 @@ export async function getCaptures() {
const res = await axios.get(url);
const data = await res.data;
log.warn('url:', url, 'took:', Date.now() - begin);
return data.captures;
return data.captures || [];
} catch (err) {
log.error(err.message);
throw err;
log.error('Error in getCaptures:', err.message);
return [];
Copy link
Collaborator

Choose a reason for hiding this comment

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

@yunchipang why you disable the throwing here?

Copy link
Collaborator Author

@yunchipang yunchipang Oct 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

because if it throws an error, getCaptures() will not return anything, then captures in static props gets undefined, and the entire top page just shows 404.

for now, to inform the error i'm just logging it. would you like this to be handled in some other way? my purpose is to have some props display on the page even when some other props were not populated properly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure, let's merge your code first, and we can continue, we can back to this if there are some problem.

BTW, I also don't like the try-catch way to handle error, I prefer to use Promise function to do it:

promise
.then(goodHandler)
.catch(badHandler)

}
}

Expand Down Expand Up @@ -73,6 +73,20 @@ export async function getCountryLeaderboard() {
}
}

export async function getOrganizations() {
try {
const url = apiPaths.featuredOrganizations;
const begin = Date.now();
const res = await axios.get(url);
const data = await res.data;
log.warn('url:', url, 'took:', Date.now() - begin);
return data.organizations || [];
} catch (err) {
log.error('Error in getOrganizations:', err.message);
return [];
}
}

export async function getOrganizationById(id) {
try {
const url = apiPaths.organization(id);
Expand Down Expand Up @@ -176,6 +190,20 @@ export async function getOrgLinks({
};
}

export async function getWallets() {
try {
const url = apiPaths.featuredWallets;
const begin = Date.now();
const res = await axios.get(url);
const data = await res.data;
log.warn('url:', url, 'took:', Date.now() - begin);
return data.wallets || [];
} catch (err) {
log.error('Error in getWallets:', err.message);
return [];
}
}

export async function getWalletById(id) {
try {
const url = apiPaths.wallets(id);
Expand Down
2 changes: 2 additions & 0 deletions src/models/apiPaths.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ const apiPaths = {
filterSpeciesByWalletId: (id = '') =>
urlJoin(host, `/species?wallet_id=${id}`),
tokens: (id = '') => urlJoin(host, `/tokens/${id}`),
featuredOrganizations: urlJoin(host, '/organizations/featured'),
featuredWallets: urlJoin(host, '/wallets/featured'),
};

export default apiPaths;
23 changes: 8 additions & 15 deletions src/pages/top.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ import {
getCaptures,
getCountryLeaderboard,
getFeaturedGrowers,
getFeaturedTrees,
getOrganizations,
getWallets,
} from 'models/api';
import * as utils from 'models/utils';

Expand Down Expand Up @@ -250,24 +251,16 @@ function Top(props) {
);
}

async function serverSideData(params) {
async function serverSideData() {
const [captures, countries, growers, organizations, wallets] =
await Promise.all([
getCaptures(), //
getCaptures(),
process.env.NEXT_PUBLIC_COUNTRY_LEADER_BOARD_DISABLED === 'true'
? []
: getCountryLeaderboard(),
getFeaturedGrowers(),
(async () => {
const data = await utils.requestAPI('/organizations/featured');
log.warn('organizations', data);
return data.organizations;
})(),
(async () => {
const data = await utils.requestAPI('/wallets/featured');
log.warn('wallets', data);
return data.wallets;
})(),
getOrganizations(),
getWallets(),
]);
return {
captures,
Expand All @@ -278,8 +271,8 @@ async function serverSideData(params) {
};
}

const getStaticProps = utils.wrapper(async ({ params }) => {
const props = await serverSideData(params);
const getStaticProps = utils.wrapper(async () => {
const props = await serverSideData();
return {
props,
revalidate: Number(process.env.NEXT_CACHE_REVALIDATION_OVERRIDE) || 30,
Expand Down
Loading