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

404 redirect bug #6356

Closed
wants to merge 5 commits into from
Closed

404 redirect bug #6356

wants to merge 5 commits into from

Conversation

corwintines
Copy link
Member

@corwintines corwintines commented May 17, 2022

Description

  • Fix redirects on pages that dont exist to properly 404

https://6295310882fe9e2a5c53ad57--ethdotorg-corwin-test.netlify.app/en/testsestse
Can test with the above link, personal Netlify deploy to test production behaviour.

Screen Shot 2022-05-30 at 2 27 39 PM

Related Issue

@gatsby-cloud
Copy link

gatsby-cloud bot commented May 17, 2022

Gatsby Cloud Build Report

ethereum-org-website-dev

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 12m

Performance

Lighthouse report

Metric Score
Performance 🔶 24
Accessibility 💚 100
Best Practices 💚 100
SEO 💚 92

🔗 View full report

@corwintines corwintines marked this pull request as draft May 17, 2022 00:56
@corwintines
Copy link
Member Author

Test with linked Netlify deploy in description :)

@corwintines corwintines marked this pull request as ready for review May 30, 2022 20:32
@github-actions github-actions bot added the needs review 👀 Review is needed for this issue or pull request label May 30, 2022
Copy link
Member

@pettinarip pettinarip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@corwintines I think that now it is always resolving a 404.

// valid url
curl -I https://6295310882fe9e2a5c53ad57--ethdotorg-corwin-test.netlify.app/en/staking
HTTP/2 404

// invalid url, correct response
curl -I https://6295310882fe9e2a5c53ad57--ethdotorg-corwin-test.netlify.app/asadasd
HTTP/2 404

// invalid nested url, correct response
curl -I https://6295310882fe9e2a5c53ad57--ethdotorg-corwin-test.netlify.app/en/staking/asadasd
HTTP/2 404

On existing paths, it is first returning a 404 but then it fetches the correct content.

=====

I would suggest turning off the flag generateMatchPathRewrites in our two redirect packages.

That will avoid the rule gatsby-plugin-intl is writing for us, ending up in these files.
More context: that intl plugin is writing some matchPaths for each lang we have https://github.com/wiziple/gatsby-plugin-intl/blob/master/src/gatsby-node.js#L102

@corwintines corwintines marked this pull request as draft June 1, 2022 18:05
@samajammin
Copy link
Contributor

@pettinarip will the recent change to our intl plugin #7022 change any of this?

@minimalsm minimalsm added the needs grooming Needs discussion during a grooming session label Jul 18, 2022
@pettinarip
Copy link
Member

@pettinarip will the recent change to our intl plugin #7022 change any of this?

I doubt it. The plugin is not touching many of these things. I mean, we are still doing the same as before in terms of redirects and 404s.

But I can take a look at this. I think it is related to the nested routes problem #6488

@pettinarip pettinarip mentioned this pull request Jul 20, 2022
@pettinarip
Copy link
Member

@corwintines closing this in favor of #7137

@pettinarip pettinarip closed this Jul 21, 2022
@github-actions github-actions bot added the abandoned This has been abandoned or will not be implemented label Jul 21, 2022
@corwintines corwintines deleted the redirect-bug branch September 26, 2023 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abandoned This has been abandoned or will not be implemented internal 🏠 needs grooming Needs discussion during a grooming session needs review 👀 Review is needed for this issue or pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants