[bug] rendering 404 instead of / or localized /en/ or /ru/ pages after updating gatsby from 2.15.22 to 2.15.23 or newer

[JustFly1984]

Description

We have an application with internationalization support. All pages created with gatsby-node createPages api. With garsby@2.15.22 and older there is normal behavior, but after update to 2.15.23 or newer (2.15.28), root page and localized versions of the root page displaying 404 template content.

All other pages displayed correctly. If I roll back to 2.15.22 root page and localized versions displayed properly.

This incorrect behavior is consistent in development and production.

I do not have steps to reproduce, but expect other people to have same issue.

Environment

System:
OS: macOS 10.14.6
CPU: (8) x64 Intel(R) Core(TM) i7-4960HQ CPU @ 2.60GHz
Shell: 3.2.57 - /bin/bash
Binaries:
Node: 12.9.0 - ~/.nvm/versions/node/v12.9.0/bin/node
Yarn: 1.17.3 - /usr/local/bin/yarn
npm: 6.10.2 - ~/.nvm/versions/node/v12.9.0/bin/npm
Languages:
Python: 2.7.15 - /usr/local/bin/python
Browsers:
Chrome: 77.0.3865.90
Firefox: 69.0
Safari: 13.0.1

[JustFly1984]

currently bug persists in 2.15.28

[mi-na-bot]

Do you have a GitHub of the affected project?

[JustFly1984]

I have project in private repo under NDA (

I have found commit which supposedly breaks my dev and build

this is screenshots of 2.15.22 build and 2.15.23:

They both shows that 404 and index.html build correctly. if I run gatsby serve, I see correct content for a second, and it rerenders ad 404 content after a moment.

[mi-na-bot]

I believe that you have the issue, just trying to create a reproduction. How do you have localization configured?

[JustFly1984]

@Sever1an with gatsby-node createPages api

[JustFly1984]

@Sever1an we have 2 locales at the time, en and ru. everything works perfectly up until 2.15.23.

[mi-na-bot]

Are you using a plugin for internationalization or is this a completely custom solution?

What happens if you remove the 404 matchpath, L113-L118? Gatsby should route to 404 anyways.

[JustFly1984]

@Sever1an this is custom solution, we are loading dictionary jsons from locize.com, and iterating on locales to create 2x pages from templates by passing locale to the context.

trying your advice, will be back with answer soon

[mi-na-bot]

I have not been able to recreate the bug yet, but have some observations. In the included screenshots there is a matchPath for a 404 page at /*. This forces all pages in the site to be handled by match-paths.json instead of the new page-data.json technique for static pages.

exports.onCreatePage = async({page,actions})=>{
  const {createPage}=actions;
  if (page.path.match(/^\/404/)) {
    page.matchPath = "/*";
    createPage(page);
  }

  if (page.path.match(/^\/zdex/)) {
    page.matchPath = "/zdex/*";
    createPage(page);
  }
  
  if (page.path.match(/^\/1dex/)) {
    page.matchPath = "/1dex/*";
    createPage(page);
  }
}

bailey:~/environment/error-test (master) $ ls src/pages/
1dex.js  404.js  index.js  innocent-bystander.js  zdex.js
bailey:~/environment/error-test (master) $ 

.cache/match-paths.json

[
    {
        "path": "/zdex/",
        "matchPath": "/zdex/*"
    },
    {
        "path": "/innocent-bystander/",
        "matchPath": "/innocent-bystander/"
    },
    {
        "path": "/1dex/",
        "matchPath": "/1dex/*"
    },
    {
        "path": "/dev-404-page/",
        "matchPath": "/dev-404-page/"
    },
    {
        "path": "/404.html",
        "matchPath": "/*"
    },
    {
        "path": "/",
        "matchPath": "/"
    },
    {
        "path": "/404/",
        "matchPath": "/*"
    }
]

Addendum: the matchpaths / and /* seem to cause trouble together because navigating to / causes the 404 page to be shown, but navigating to /innocent-bystander/ works.

[mi-na-bot]

@JustFly1984 Could you take a look in .cache/match-paths.json and see if there is anything interesting/unexpected there?

[JustFly1984]

Well, I have tried your solution.

{
      path: '/404.html',
      matchPath: '/*',
      component: NotFoundTemplate,
},

actually transforms to

{
      path: '/en/404.html',
      matchPath: '/en/*',
      component: NotFoundTemplate,
      context: { locale: 'en' }
},
{
      path: '/ru/404.html',
      matchPath: '/ru/*',
      component: NotFoundTemplate,
      context: { locale: 'ru' }
},
{
      path: '/404.html',
      matchPath: '/*',
      component: NotFoundTemplate,
      context: { locale: 'en' }
},

I have removed localized 404 pages, but left / page at the end, and localized pages stop turning to 404, but / itself rendering 404.

I have removed last 404, and / page stopped to show 404, but id I type incorrect url, which should render 404, it fails with error:

[mi-na-bot]

I think that the bug here is that match paths for /* are not overridden by the index page because somehow /* and / are of equal specificity.

An add-on problem is that using this technique to supply a 404 page for a path causes that entire path to be routed with match-paths.json instead of the static page routing.

Regarding the error with the 404 page, yikes! Could it be that you now have no 404 page at all or the 404 page is broken such that gatsby routes to non-existent page?

[JustFly1984]

Yes, it last case I do not have 404 pages at all, cos I'm not using src/pages directory at all.

in case if I do not specify 404 pages at all I have next match-paths.json:

in case if I have only /404.html:

I have bigger match-paths.json with / at the end!

In case I turn back on localized pages

match-paths.json at the end looks like this:

[JustFly1984]

@Sever1an I do not understand why / is at the end of the list of match-paths.json
I think an order is important here.

[mi-na-bot]

If you have a lot of static content under /en and /ru I would suggest seeing if a regular 404 page in src/pages could be made to work.

[JustFly1984]

@Sever1an
I need to have localized 404 pages. before 2.15.23 it worked perfectly as intended.

[JustFly1984]

@Sever1an
I have content both static and dynamic for /en and /ru routes, and we will have 6 more languages soon. I need to have localized 404 pages for each language.

For now I will roll back to 2.15.22 and wait till it will be fixed. I'm sure there is other people who has same issue.

This is totally breaking change, and should not be published as patch version. We have e2e tests, and issue was catched, so we could track issue to gatsby update commit, so we have not pushed to production. but it is silent bug, and who knows how many people have it in production.

[JustFly1984]

running 2.15.22 match-paths.json looks like this:

[mi-na-bot]

It is not the expected behavior, index pages in match paths should take precedence.

I was suggesting that you might have the 404 page localize its self using props.location. This would remove the need for catch all paths and get static pages out of match-paths

[JustFly1984]

well, this is not really feasible to use props.location, cos we are using react-intl...
and I do not really understand how it could work.

I will wait till bug is fixed and update then.

[mi-na-bot]

If the language is in the url like https://myapp.com/en/more-info, props.location.pathname would contain /en/more-info which the 404 page could parse and show localized content. It is probably not that big of a deal, 404 pages are supposed to be short.

What changed with match-paths.json is that now any static page which should take precedence over a wildcard match path route now has an entry there. Using a super broad one to handle 404 will result in a bloated match-paths.json to replicate something that is already kind of built in.

[mi-na-bot]

The issue seems to occur in this function: https://github.com/wardpeet/gatsby/blob/35cb2389e15438518b711812a111bf414a1ca4c4/packages/gatsby/src/bootstrap/requires-writer.js#L28-L39

Particularly this line.

    let score = page.matchPath.replace(/\/$/, ``).split(`/`).length

Paths are assigned a score based on how specific they are by basically counting slashes, but it removes the trailing slash first. For an index page the trailing slash is significant, as it implies /index. Globs like /* and /en/* score one point higher than / and /en/. Removing the .replace(/\/$/, ``) seems to result in correct behavior, but I am not certain it is never necessary.

[JustFly1984]

Is there any movement on issue?

[mi-na-bot]

The core team has all been away for the past week, I wouldn't expect anything before Monday.

I anticipate this issue will get plenty of attention.

[JustFly1984]

this part of code should be tested, so tests will fail before anything could be released

[mi-na-bot]

They are pretty serious about CI and have lots of tests, it is actually surprising that they do not have one for this. Perhaps a more formal specification of paths and trailing slashes is needed to allow effective tests. There are certainly a few possibly undefined cases with stripped trailing slash like: /page.js or /page/index.js. And as we now know, /page/* vs /page/index.js. Another issue would probably be the most appropriate place to discuss the testing part... a long thread will distract from the bug fix.

[JustFly1984]

is there any movement?

[pieh]

Published gatsby@2.17.3

[JustFly1984]

It does work as expected after update! Thank you!