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

ISR caches 404 result, but not status code #43831

Closed
1 task done
perevernihata opened this issue Dec 7, 2022 · 8 comments · Fixed by #55542
Closed
1 task done

ISR caches 404 result, but not status code #43831

perevernihata opened this issue Dec 7, 2022 · 8 comments · Fixed by #55542
Labels
bug Issue was opened via the bug report template. locked

Comments

@perevernihata
Copy link

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
  Platform: darwin
  Arch: x64
  Version: Darwin Kernel Version 22.1.0: Sun Oct  9 20:14:54 PDT 2022; root:xnu-8792.41.9~2/RELEASE_X86_64
Binaries:
  Node: 16.18.1
  npm: 8.19.2
  Yarn: 1.22.19
  pnpm: N/A
Relevant packages:
  next: 13.0.7-canary.1
  eslint-config-next: 13.0.6
  react: 18.2.0
  react-dom: 18.2.0

Which area(s) of Next.js are affected? (leave empty if unsure)

App directory (appDir: true), Middleware / Edge (API routes, runtime), Routing (next/router, next/navigation, next/link)

Link to the code that reproduces this issue

https://github.com/perevernihata/nextjs-isr-404-caching

To Reproduce

  1. clone the repo https://github.com/perevernihata/nextjs-isr-404-caching
  2. yarn && yarn build && yarn start
  3. Open the browser, open the development console network tab
  4. Navigate to http://localhost:3000/articles/test-404-then-200-page-result
  5. Observe that the first-time request returns 404, and the second and all subsequent page refreshes return the same Not Found page with a 200 status code. Also, subsequent requests have x-nextjs-cache: HIT header.

image

Describe the Bug

With ISR enabled, nextjs caches the 404-page content, but it does not cache the status code, therefore 404 is only returned the first time, and the rest of the time it is 200.
I would think, that 400 to 500 status codes should not be cached on ISR at all.
What makes it worse is that I can not conditionally disable caching or ISR for specific results in runtime.

Expected Behavior

I would expect 1 ouf these 2 options:

  • All status codes above 400 are not cached by ISR at all
  • If the response is cached then the status code is also preserved

Alternatively, it would be also great to have API to decide if we want this specific response to be cached or not, e.g.:


const Article = ({ params }: Props) => {
  const { slug } = params;

  if (slug !== "works") {
    // currently notFound does not support any params
    return notFound({ disableIsr: true });
  }
  return <div>Articles page with slug</div>;
};

Which browser are you using? (if relevant)

Chrome, but irrelevant

How are you deploying your application? (if relevant)

AWS, but irrelevant

@perevernihata perevernihata added the bug Issue was opened via the bug report template. label Dec 7, 2022
@pshemek
Copy link

pshemek commented Feb 4, 2023

I saw it in my app and I looked at how Next.js 13 playground handles things.
When the route like https://app-dir.vercel.app/ssg/751283748274 is hit, notFound() is triggered but the rendered page is returned with status 200. I would expect the non-existing page would come with the status 404.

BTW: The console says there:
Error: An error occurred in the Server Components render. The specific message is omitted in production builds to avoid leaking sensitive details. A digest property is included on this error instance which may provide additional details about the nature of the error.

@fab1an
Copy link

fab1an commented Jul 5, 2023

Will this ever be fixed, this is a pretty bad bug in my opinion. Having a 404 page not return 404.

@Tekutorukushi
Copy link

Tekutorukushi commented Aug 27, 2023

The error is still present in last version (13.4.19)
it turns out that if I allow all pages for indexing in the robots.txt, then if the page is non-existent, it will still be indexed by robots. Because it will be created on the server as html

@gabiinicial
Copy link

I currently have the same error, I am using the latest version of next.js and I have integrated i18next when I trace my links the not-found page returns status code 200 and not a 404.

@DennieMello

This comment has been minimized.

ijjk added a commit that referenced this issue Sep 18, 2023
This ensures we properly set/restore the status code with ISR paths in
app router so that when we set the 404 status code with `notFound` it is
persisted properly.

Fixes: #43831
Closes: #48342
x-ref:
#49387 (comment)
@ijjk
Copy link
Member

ijjk commented Sep 18, 2023

Hi, this has been updated in v13.4.20-canary.39 of Next.js, please update and give it a try!

@Tekutorukushi
Copy link

If you use Suspense, then the status is 200. Therefore, you need to remove the wrapper from the Suspense in layout

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot added the locked label Oct 4, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. locked
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants