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

Serve missing internal files as 404 instead of soft-404 page #2983

Merged
merged 1 commit into from
Oct 6, 2023

Conversation

infinite-persistence
Copy link
Collaborator

@infinite-persistence infinite-persistence commented Oct 4, 2023

This addresses the deeper root-cause for the ChunkLoadError.

Issue

Requesting for an missing/invalid js file like https://odysee.com/public/some-non-existent-file.js yields HTML, which is usually incorrectly redirected to an actual claim, or the React soft-404 page.

Besides incorrect, it is also being cached. Users are stuck until cache is manually cleared.

Approach

  • In the final catch-all stage, try to detect if it's a file request and return a 404 instead.
  • Do not cache 404s. (Same reasons as https://community.cloudflare.com/t/404s-and-caching/337151. The file could eventually be corrected).
  • The body is now "Resource not found". I think we can add plain HTML (no redirect to claims) to beautify it, as long as the status is 404. But not sure if it's worth it.

Warning

Is the detection too loose? Any better ideas to not over-catch?

## Issue
Requesting for an invalid js file like `https://odysee.com/public/some-non-existent-file.js` yields HTML, which is usually incorrectly redirect to an actual claim, or a soft-404 page.

It's not only showing incorrectly data, but it is being cached. This is one of the potential causes for the problem of users being stuck with bad cache.

## Approach
- In the final catch-all stage, try to detect if it's a file request, and return a 404 instead.
- Do no cache 404s. Same as https://community.cloudflare.com/t/404s-and-caching/337151. The file could eventually be corrected, but users remain stuck with 404.

## Concerns
- Is the detection too loose?
@tzarebczan
Copy link
Contributor

This is a good approach, thank you! Should help with future edge issues potentially too.

@tzarebczan tzarebczan merged commit a38cf06 into master Oct 6, 2023
2 checks passed
@tzarebczan tzarebczan deleted the file.404 branch October 6, 2023 13:25
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.

2 participants