404 page fails to load javascript
I've noticed what appears to be a bug on 404 pages. Tested in both Chrome and Safari.
- Search cannot be used (clicking the search field does nothing)
- Left-side navigation cannot be expanded.
Link to 404 page.
The megamenu also does not seem to open on 404 pages as well.
It might be related to this issue. Needs further investigation
After some tests here is what I've found out:
- When opening a microsite non existing page I see something like this:
Essentially the first 404 is returned by the microsite 404 rule (which works as expected):
{
src: `/${workingDirectoryName}/(.*)`,
status: 404,
headers: { "x-custom-404": "microsite404" }, // custom header added for debug
dest: `/${workingDirectoryName}/404`,
},
When the 404 page loads, the gatsby code tries to load various resources. In the screenshot you can see the code tries to load https://docs.commercetools.com/getting-started/page-data/ss/page-data.json which returns a 404, that's fine too and it's handled by gatsby correcly.
What's wrong is the fact that the code tries also to load https://docs.commercetools.com/ss (note the HEAD request) which matches the catchall rule BUT doesn't return a 404 but a 200. It seems like this rule:
{
// any other 404 goes to the "docs" homepage 404
src: '/(.*)',
status: 404,
dest: `https://docs.commercetools.com/docs/404`,
},
gets executed but it doesn't send 404 error.
That somehow causes the issue that surfaces with an internal Gatsby error.
- Removing the catchall rule resolve the problem with the microsites but any 404 page in the proxy will cause the vercel 404 page to be displayed
This is how the request would look like without the catchall:
as you can see, the fact that the https://docs-proxy-ga-test-404.commercetools.vercel.app/sssd actually returns a 404 error, seems to fix the issue.
@nkuehn can you have a look to my previous 2 comments and let me know if that's expected and if would be acceptable to just remove the catchall 404 rule?
Just a reminder @nkuehn when you have time ☝
Googled it for gatsyby first before burning time on network traces.
The problem inside a microsite a known bug and they don't care at all https://github.com/gatsbyjs/gatsby/issues/32142 - there is even a one-line fix available from the community: https://github.com/gatsbyjs/gatsby/pull/34237/files#diff-177ac96ca321818b0022cdaa9fda344808eed92150a112f28ff205cda29b8049L606
Outside an existing microsite it's a bit weirder, I think we're not even returning 404 at all. I am getting the 404 page content, but with a 200 status code. Apart from that it has the same bug as the "inside a microsite" case. A 200 is problematic because search crawlers then think this is an actual page location. -> an actual bug on our side vs. a gatsby bug.
I had hoped we can work around it by sending a "Location" header together with the 404 response to redirect but that is not specified in HTTP, Location header is only valid on redirect status codes.
A "dumb but working temporary mitigation" could be to have the 404 page have a piece of javascript that redirects to the home page after 5 seconds. At least people would be practically "unstuck" and the search would work again without having to type into the URL bar. It's not a good web practice but I don't really see gatsby maintainers do anything here. (or, you want to apply a patch to the gatsby codebase via yarn but that would require maintaining it etc. )
Thank you @nkuehn ,
following your advice i created a PR introducing a new <PageRedirection> component which can be used directly in .mdx files. By default it redirects to / after 5s but it can be configured in case we need it in other context.
Thank you for making it a generic component from the beginning!
I would suggest to nevertheless keep the ticket open and mark it blocked by the Gatsby issue.
The mitigation fix will be deployed to production soon
It turns out that the fact React doesn't get loaded (and in general any javascript), it prevents the redirection (which is javascript based) to happen (doh...). So, back to the drawing board. IMHO, since the gatsby fix is unlikely to happen, removing the catchall rule could be the way to go:
- no maintainance cost for us (compared to applying the fix through yarn)
- the docs site will be degraded to the default 404 vercel page, but all the rest of website will be fixed.
could have thought about that... Even if we hardwire it a raw javascript into the static generated file ( https://github.com/commercetools/commercetools-docs-kit/blob/main/packages/gatsby-theme-docs/gatsby-ssr.js ) it would only run when the page is fully loaded and not when navigating inside the microsite. All a mess, damn.
How about going back to patching gatsby through yarn patch? It's a one-line change ( https://github.com/gatsbyjs/gatsby/pull/34237/files#diff-177ac96ca321818b0022cdaa9fda344808eed92150a112f28ff205cda29b8049L606 ) so it's likely git reliably patches it even in newer versions and the file does not even change a lot ( history: https://github.com/gatsbyjs/gatsby/commits/master/packages/gatsby/cache-dir/loader.js )
I would not even do it in the kit but only in the docs repo (we may have to update it with gatsby version changes, so it should have to be done in as few places as possible. If the error remains in the mc app kit website I can live with that.).
I'll give it a try
@nkuehn It seems the patching is working as expected, please have a look to https://github.com/commercetools/commercetools-docs/pull/4238
Patching gatsby seems to break at the moment of merging the feature branch into main:
https://github.com/commercetools/commercetools-docs/actions/runs/6878197802
Further investigation is needed
oh, that's a pity. The error logs reads more like an issue with the UI-Kit dependencies though (but you never know what side effects could be going on)