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

NEX-101: Improve data fetching and error handling #236

Merged
merged 7 commits into from
Jul 30, 2024

Conversation

joshua-scott
Copy link
Member

@joshua-scott joshua-scott commented Jul 25, 2024

Link to ticket:

https://wunder.atlassian.net/browse/NEX-101

Link to feature environment:

https://feature-nex-101-data-fetchi4a2.next-drupal-starterkit.dev.wdr.io

Changes proposed in this PR:

See commits for more specifics, but the main parts of this are:

  • Cleanup page data fetching logic in general (getStaticProps, getCommonPageProps etc)
  • Encourage faster page generation by kicking off promises as soon as possible (no more waiting until all other page data has been fetched, then await getCommonPageProps(...) right at the end)
  • Abort the build if a page is 404 during next build (pages returned from getStaticPaths should always exist)
  • Throw an error if the graphql query for node data is unsuccessful in getStaticProps (and provide a link to CMS logs for further debugging). This means that if there are networking or GraphQL problems, the page will not 404 (current behaviour). Instead, the page won't be updated so the last working version of the page will continue to work. (Still, if a page should 404 as it's been deleted or unpublished, that will still correctly update the page to a 404)

How to test:

  1. Check CI logs build logs and feature environment, should work without errors.
  2. After Silta has put the feature environments to sleep, check the site still works by resurrecting the node container but not the Drupal one.

This ensures that:
1. The build fails if frontpage cannot be built
2. Frontpage would never serve a 404 if CMS has an issue (the last version of the page would be shown instead)
- If 404 encountered during a build, throw an error to abort the build (paths returned from getStaticPaths should always exist)
- If fetching node data fails:
  1. Provide a link to GraphQL logs for debugging purposes
  2. Rethrow the error to prevent page becoming 404, so website is more resilient to CMS/networking issues.

This also means that, in most cases, feature environments can be accessed after silta puts the containers to sleep, without having to wake up BOTH the CMS and Next.js containers (just the Next.js one).
@joshua-scott joshua-scott marked this pull request as ready for review July 25, 2024 13:59
@vermario
Copy link
Collaborator

I think this is a nice improvement and we can merge it.

I suggest we open a followup ticket regarding what happens when you unpublish a translation of the homepage.

at the moment though we have a bigger problem related to #205 , due to what I think is a bug in the graphql_compose module https://www.drupal.org/project/graphql_compose/issues/3464624#comment-15705017

@joshua-scott joshua-scott merged commit 0d1b755 into main Jul 30, 2024
6 checks passed
@joshua-scott joshua-scott deleted the feature/NEX-101-data-fetching branch July 30, 2024 10:12
jekku123 pushed a commit that referenced this pull request Oct 20, 2024
NEX-101: Improve data fetching and error handling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants