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

GetStaticProps returning initial notFound never triggers a rebuild on revalidate with dynamic routes #21453

Closed
alizeait opened this issue Jan 22, 2021 · 21 comments · Fixed by #28097
Assignees
Milestone

Comments

@alizeait
Copy link

alizeait commented Jan 22, 2021

What version of Next.js are you using?

10.0.5

What version of Node.js are you using?

12.13.0

What browser are you using?

Chrome

What operating system are you using?

Windows

How are you deploying your application?

next start

Describe the Bug

If getStaticProps returns notFound: true in the initial build with a revalidate value, Nextjs never tries to rebuild the page

Expected Behavior

Nextjs should try rebuilding the page instead of assuming permanent 404

To Reproduce

export const getStaticProps = async (props) => {
  try {
    const dictionaries = await apiFetch();
    return {
      props: {
        dictionaries,
      },
      revalidate: 60,
    };
  } catch (error) {
// assume the api fetch fails with the initial build
   return {
      notFound: true,
      props: {
        dictionaries: {},
      },
      revalidate: 60,
    };
  }

// On subsequent re-builds after revalidate has passed and the api returns a ok response, Nextjs never tries to rebuild the page and always returns the 404 page instead
@alizeait alizeait added the bug Issue was opened via the bug report template. label Jan 22, 2021
@janus-reith
Copy link

janus-reith commented Feb 3, 2021

I actually observe the opposite behavior, here is a sandbox:
https://codesandbox.io/s/awesome-greider-11ugg
(Not sure though if your try/catch block might make a difference.)

You might need to fork and restart the sandbox to see it, it runs rimraf .next on start to ensure there is no cached content as I was unsure if something might get cached from previous runs otherwise.

If I return notFound: true in getStaticProps, it actually builds the page right on demand during runtime, and doesnt serve any stale page from build time, which is great for a usecase I have, where Static generation should happen after the build only.

I'm wondering however if that behavior of notFound during build is intended to work this way and meant to stay or just some sideeffect - I'm unable to find any documentation about notFound during build.

**Edit: ** Created a separate issue here: #21809

@alizeait
Copy link
Author

alizeait commented Feb 3, 2021

Thanks for the sandbox @janus-reith, I actually forgot to mention that this happens with dynamic routes, I think how static routes work like in your Sandbox should be the way to go. I made a fork of your sandbox here with dynamic routes. Notice that the /intl/docs/intl-page always serves the 404 page even after revalidate time has passed since it errors during build while /us/docs/us-page page works as intended.

@alizeait alizeait changed the title GetStaticProps returning initial notFound never triggers a rebuild on revalidate GetStaticProps returning initial notFound never triggers a rebuild on revalidate with dynamic routes Feb 5, 2021
@bscaspar
Copy link

bscaspar commented Feb 10, 2021

I am encountering a similar experience (but somewhat in reverse).

The initial static page with a dynamic route is built successfully. getStaticPaths uses {fallback: 'blocking'} and getStaticProps uses {revalidate: 1}. Then then the page data is removed from the db so getStaticProps returns {notFound: true}. Despite all this, the static page continues to be served.

I confirmed that the data fetching call that should happen in getStaticProps returns no data, so it appears as though getStaticProps isn't running at all.

@jatbone
Copy link

jatbone commented Feb 24, 2021

Same here, when {notFound: true} revalidate doesn't happen. Is this gonna be fixed soon? It's pretty big issue. Thanks

@LotharVM
Copy link

Any update on this? Also experiencing this and especially with dynamic routes being controlled by a client it's impossible to manually keep track of all the routes that change from/to 404..

@ramiel
Copy link

ramiel commented Apr 27, 2021

Same here, when a page is 404 it never gets rebuilt.

@timneutkens timneutkens added this to the 11.x.x milestone Jun 21, 2021
@timneutkens timneutkens added kind: bug and removed bug Issue was opened via the bug report template. labels Jun 21, 2021
@tbgse
Copy link

tbgse commented Aug 10, 2021

We're experiencing this problem as well. It's a pretty big issue for us if products are being added to our product catalog that had been a 404 before but won't revalidate. Would appreciate an update as well. (next v11.0.1)

@ijjk
Copy link
Member

ijjk commented Aug 10, 2021

@tbgse is revalidate: 1 (or a different value) being returned when returning notFound: true on v11.0.1? The default value for revalidate is false or no revalidation so if it's not explicitly returned it will not revalidate. If so can you provide a minimal reproduction with steps to reproduce we can investigate further!

@tbgse
Copy link

tbgse commented Aug 10, 2021

hi @ijjk we're setting revalidate: 300 if notFound: true is being returned. The problem is pretty much perfectly summarized by the original poster of this issue.

Our page generation logic is fairly complex, but below a small excerpt where we'd basically return a page with props from makeResult or a simple { notFound: true, revalidate: 300 } if a collection wasn't found.

I will try to create a small repo to reproduce this.

const makeResult = async (pageName: string, specifics: PageSpecificProps) => ({
  props: {
    footer,
    locale,
    menuNavigation,
    messages,
    pageName,
    params: props.params,
    path,
    webStore,
    ...specifics
  },
  revalidate: 300
});

const entryType = result.entry?.sys.contentType.sys.id;

if (entryType === 'Collection') {
  const collectionPage = result.entry;
  const collection = await retrieveCollectionPageData(collectionPage, locale, webStore);
  return makeResult('collection', {
    collection,
    type: 'collection',
    urlRoute: result.urlRoute
  });
} else {
  return { notFound: true, revalidate: 300 };
}

@tbgse
Copy link

tbgse commented Aug 10, 2021

@ijjk I just created a repo here: https://github.com/tbgse/next-no-revalidate

But it seems like in this scenario everything is working as expected. I tried revalidating both for routes with and without fallback and it seems like they recover nicely from it (see here https://next-no-revalidate-okhtkqp1j-tbgse.vercel.app/revalidate), you'd have to refresh a couple of times until you get the right number. But everything works as expected. Maybe something else was at play, our getStaticProps logic is very complex. I wonder if it might behave differently than the simple example when a real API is used.

EDIT: I think i found what our problem is. We use a custom 404 page and this page also uses getStaticProps but does not have a revalidate set. If this is the case, setting { notFound: true, revalidate: 300 } in the original component seems to be ignored and the getStaticProps of the 404 component is used instead. Does that sound like it could be right?

@la55u
Copy link

la55u commented Aug 11, 2021

I created an other reproduction here: https://github.com/la55u/revalidate-test
I cannot deploy it to Vercel due to this bug: #27948, but here is how to test it locally:

  1. clone the repo
  2. npm run build
  3. npm run start
  4. visit: https://localhost:3000/test

The important part is that steps 2 & 4 should happen at a time when the minute is even e.g. 11:32, so the notFound will return true and the first time you open the page will be 404. Try refreshing the page until the minute is odd and you will still see the 404 page.

@ijjk
Copy link
Member

ijjk commented Aug 11, 2021

@la55u I tried testing out your reproduction and tweaked it a bit to make triggering the notFound easier but it doesn't seem to behave incorrectly, see below screen recording:

Screen.Recording.2021-08-11.at.10.05.22.AM.mov

@alizeait
Copy link
Author

@ijjk keep in mind that the original issue only happens with dynamic routes

@ijjk
Copy link
Member

ijjk commented Aug 11, 2021

@alizeait just double checked dynamic routes and it seems to be working correctly as well

Screen.Recording.2021-08-11.at.10.21.26.AM.mov.recording.mov.mov

@la55u
Copy link

la55u commented Aug 11, 2021

@la55u I tried testing out your reproduction and tweaked it a bit to make triggering the notFound easier but it doesn't seem to behave incorrectly, see below screen recording

@ijjk You did not satisfy the condition of the bug because when you ran yarn build the file contained 200. It has to be the first
run that returns notFound: true not one later at some point.

@artmsv
Copy link

artmsv commented Aug 12, 2021

Can confirm for 11.0.1, but looks like it got resolved from 11.1.0
Might need to recheck tests for this use case

11.0.1

nextjsDoesntWork.mov

11.1.0

nextjsdoeswork.mov

@artmsv
Copy link

artmsv commented Aug 13, 2021

Update: just got the same issue with the current version 11.1.0

@kodiakhq kodiakhq bot closed this as completed in #28097 Aug 14, 2021
kodiakhq bot pushed a commit that referenced this issue Aug 14, 2021
This fixes revalidation not occurring correctly when `notFound: true` is returned during build, additional tests have been added to ensure this is working correctly for dynamic and non-dynamic pages returning `notFound: true` during build and then revalidating afterwards.  

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Errors have helpful link attached, see `contributing.md`

Fixes: #21453
@ijjk
Copy link
Member

ijjk commented Aug 16, 2021

Hi, this should be updated in the latest version of Next.js v11.1.1-canary.9, please update and give it a try!

@NikitaMazur
Copy link

Still reproduce with revalidate: 60, fallback: "blocking" in next 11.1.2

@NikitaMazur
Copy link

Still reproduce with revalidate: 60, fallback: "blocking" in next 11.1.2

My mistake. Didn't added revalidate to return with notFound

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.