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.

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

issue #533: fixed page crashing if mock-fcc-data endpoint is not running #534

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sudo-benjamin
Copy link

@sudo-benjamin sudo-benjamin commented Dec 31, 2024

Checklist:

Closes #533

@sudo-benjamin sudo-benjamin requested a review from a team as a code owner December 31, 2024 04:16
return data.json();
} catch (error) {
return [];
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of catching all possible errors, it's better to just do a defensive check to see if data is set properly (as a json). If not, return the empty array.

Choose a reason for hiding this comment

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

@sudo-benjamin - to know what to check, You should read the document of json() method. The exceptions that could be thrown if the JSON can be parsed is SyntaxError. And in other to return to handle a specific error, you will need to do as
if (error instanceof SystemError) { return [] }

Choose a reason for hiding this comment

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

Just check the open issue, and the error we are seeing on the page is TypeError instead of SyntaxError. You should do the check for TypeError instead.

@utsab
Copy link
Collaborator

utsab commented Jan 14, 2025

Overall, your fix looks good (see my comment). Can you also add a unit test which can check whether the page loads correctly in both cases: (1) the mock endpoint returns data (2) the mock endpoint does not return data).

@utsab
Copy link
Collaborator

utsab commented Jan 14, 2025

One other request for this issue: In the case where there is no data to display in the table, it does not look good to show a completely empty table. This might be a confusing user experience. Please show a custom message on the page in place of the table. For now, you could simply remind them to enable the mock-fcc-data endpoint.

return data.json();
try {
let data = await fetch(process.env.MOCK_USER_DATA_URL);
return data.json();

Choose a reason for hiding this comment

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

json() doesn't return a JSON but a promise. You need to add await when calling data.json() as well. You can read it more from here.
https://developer.mozilla.org/en-US/docs/Web/API/Response/json

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: /classes page crashes if the mock-fcc-data endpoint is not running
3 participants