-
Notifications
You must be signed in to change notification settings - Fork 22.6k
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
fix(ci/preview): add custom 404 page for PR preview pages #37769
base: main
Are you sure you want to change the base?
Conversation
Preview URLs (comment last updated: 2025-01-24 05:05:09) |
d65dee3
to
d2eada7
Compare
41ee55b
to
a44664f
Compare
This pull request has merge conflicts that must be resolved before it can be merged. |
Note that due to the fix mentioned here, for your custom 404 page to work, you'll need to overwrite I'll let content folks (cc @Rumyra) decide whether your proposed custom 404 page with a link to production is more useful than the generic 404 page. |
Related issue in yari: mdn/yari#7185 |
If we have |
Thanks for adding this feature Onkar. This looks like it will be useful. One suggestion, though people may disagree: |
I have temporarily deployed a change on the review environment that automatically replaces HTTP 404 responses with HTTP 303 redirecting to the same path on https://developer.mozilla.org. Advantage:
Disadvantage:
This is not a permanent change, and we have plans to migrate the review environment from AWS to GCP soon, which should resolve the issues without any caveat. |
The automatic redirect could be confusing. The reviewer needs to know when they cross PR boundary. How about we throw 404, and the 404 page auto redirects to the prod server after five seconds? We can redirect images directly.
😕 This is desirable, right? We add a new image in a PR and we want to see it in the preview.
If an image is removed, then the author must have removed the corresponding rending code as well, right? |
The current 404 page shows following content:
From the error, when 404 occurs, the Cloudfront is trying to look for
/en-us/_spas/404.html
. We can put a custom404.html
file there to provide the link to prod pages.Tested the changes successfully:
404_page.mp4
I had to put JS in a separate file to avoid inline script CSP error. To have the JS in the same HTML we'll have to add CSP hash to
mdn/yari//libs/constants/index.js
. The preview build is not part of the MDN platform so let's not involve yari repo.