FIX: Removed 404 Error Page on Navigation and Reload

[iain801]

Overview

In this PR, we remove the issue of the 404 Error screen appearing briefly whenever a page is loaded or reloaded. We accomplish this by rendering a blank page until the correct one has been served to the client. This also has a secondary benefit of reducing page loading times, as it removes the initial React Router query (Which resulted in 404).

Screenshots (Optional)

Insert absence of 404 Error

Future Possibilities (Optional)

Some possible points could be:

• Rather than serving a blank page, we could maintain the current page (if applicable) until the new one is ready.
• It could also be nice to refactor the app to use the newer React data routers, which have some nice features for mitigating similar issues

Validation (Optional)

1. Deploy this branch on dokku (localhost is too fast to replicate the issue).
2. Navigate to a page other than root
3. Reload the page repeatedly and make sure “404” never appears

Tests

• [x] Backend Unit tests (mvn test) pass
• [x] Backend Test coverage (mvn test jacoco:report) 100%
• [x] Backend Mutation tests (mvn test pitest:mutationCoverage) 100%
• [x] Frontend Unit tests (npm test) pass
• [x] Frontend Test coverage (npm run coverage) 100%
• [x] Frontend Mutation tests (npx stryker run) 100%
• [x] Frontend Linting (npx eslint --fix src)

Linked Issues

Closes #33

[chriscerie]

We accomplish this by rendering a blank page until the correct one has been served to the client.

The changes don't seem to do that. It tries to hide the issue by delaying loading by 200ms in hopes the currentuser query wins the introduced race condition, which will fail on high latency networks. Instead I suggest solving the root of the issue and use react-query's flags (e.g., isLoading) to determine when to fallback to blank page.

[pconrad]

We accomplish this by rendering a blank page until the correct one has been served to the client.

The changes don't seem to do that. It tries to hide the issue by delaying loading by 200ms in hopes the currentuser query wins the introduced race condition, which will fail on high latency networks. Instead I suggest solving the root of the issue and use react-query's flags (e.g., isLoading) to determine when to fallback to blank page.

I think I agree with @chriscerie here. This does "appear" to fix the problem, but only with a hack that is tailored to very specific network conditions. I think a better solution involves tackling this at the root of the problem, which is to say, at the react-query call, along the lines that @chriscerie suggests. That may mean that there is no "silver bullet" that solves this across the entire app (though some refactoring in https://github.com/ucsb-cs156-m23/proj-happycows-m23-10am-4/blob/main/frontend/src/main/utils/currentUser.js might go a long way towards resolving it.)

It's a great first step though, in terms of investigating and understanding the problem.

[chriscerie]

Just a hint, react-query used in the hook already gives you the necessary states for whether a resource is still fetching. If you use that to determine whether to display 404 or blank page it’ll be a fairly straightforward fix.

[iain801]

That was something I originally thought, but I tested it on varying latency networks on my dev server with a production build deployment from 0 offset (around 10 ms ping latency) to 10 seconds and never saw the 404 page. I do know the race condition is between the client rendering the page and the currentUser being returned from the backend.

[iain801]

idk how I never noticed the initialData flag in useCurrentUser

[iain801]

just kidding, didnt work

[iain801]

Also, currently we serve the 404 Error when the client doesn't have the correct permissions, however, the HTTP standard has 403 Forbidden for this purpose.

[iain801]

@phtcon check it out now, thanks @chriscerie for the hint

[iain801]

@phtcon are deleting all the comments the correct move here?

[chriscerie]

You don’t need a comment to tell you a variable called adminRoutes defines the admin routes. Favor meaningful comments that tell you why the code is there than what the code does.