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

fix: Page error codes #116

Merged
merged 6 commits into from
Jun 13, 2022
Merged

fix: Page error codes #116

merged 6 commits into from
Jun 13, 2022

Conversation

tlgimenes
Copy link
Contributor

@tlgimenes tlgimenes commented Jun 10, 2022

What's the purpose of this pull request?

This PR fixes a bug introduced on #88. Also, fixes one of the TODOs regarding GraphQL status code.

How does it work?

#88 fixed wrong 404s on PDPs when an error occurred between function -> ecommerce API. This fix, however, didn't take into account that users entering wrong slugs should still get a 404, not a 500. This PR fixes all of these problems.

To way it works is that now it reads the error status thrown inside @faststore/api before returning the response.

How to test it?

  1. Pick a PDP, say /4k-philips-monitor-99988213/p
  2. When visiting /4k-philips-monitor-99988213/p you should get a 200 with the right page being rendered
  3. Visiting a non existing page, say ``/4k-philips-monitor/p` should get you a 404 page
  4. When an error occurs between function -> ecommerce API, you should get a 500. (To simulate this behavior, run locally and turn off the wifi)

References

Also, check the reference PR on @faststore/api

@vercel
Copy link

vercel bot commented Jun 10, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
nextjs-store-storybook ✅ Ready (Inspect) Visit Preview Jun 13, 2022 at 5:22PM (UTC)

@tlgimenes tlgimenes force-pushed the feat/export-errors branch from cc6eaaa to 8aca7c1 Compare June 10, 2022 18:23
@vercel vercel bot temporarily deployed to preview June 10, 2022 18:27 Inactive
@vtex-sites
Copy link

vtex-sites bot commented Jun 10, 2022

Preview is ready

This pull request generated a Preview

👀   Preview: https://sfj-c248449--nextjs.preview.vtex.app
🔬   Go deeper by inspecting the Build Logs
📝   based on commit c248449

@tlgimenes tlgimenes added the Bug Fixes Something isn't working label Jun 10, 2022
@tlgimenes tlgimenes changed the title fix: Error codes | WIP fix: Error codes Jun 10, 2022
@tlgimenes tlgimenes changed the title fix: Error codes fix: Page error codes Jun 10, 2022
@tlgimenes tlgimenes requested a review from a team June 10, 2022 18:38
@vercel vercel bot temporarily deployed to preview June 10, 2022 19:02 Inactive
@vtex-sites
Copy link

vtex-sites bot commented Jun 10, 2022

Lighthouse Reports

Here are the Lighthouse reports of this Pull Request

📝 Based on commit c248449

Lighthouse Report by page
📎   /
📎   /apple-magic-mouse-99988212/p
📎   /office

@vercel vercel bot temporarily deployed to preview June 10, 2022 19:49 Inactive
package.json Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to preview June 13, 2022 17:22 Inactive
@tlgimenes tlgimenes merged commit c248449 into main Jun 13, 2022
@tlgimenes tlgimenes deleted the feat/export-errors branch June 13, 2022 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Fixes Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants