vercel / next.js

The React Framework
https://nextjs.org
MIT License
126.28k stars 26.89k forks source link

next/navigation notFound() function do not apply HTTP 404 status code #51021

Closed robinComa closed 7 months ago

robinComa commented 1 year ago

Verify canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: x64
      Version: Darwin Kernel Version 22.3.0: Mon Jan 30 20:42:11 PST 2023; root:xnu-8792.81.3~2/RELEASE_X86_64
    Binaries:
      Node: 18.13.0
      npm: 8.19.3
      Yarn: N/A
      pnpm: N/A
    Relevant packages:
      next: 13.4.4
      eslint-config-next: 13.3.0
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.0.4

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

App directory (appDir: true), Routing (next/router, next/navigation, next/link)

Link to the code that reproduces this issue or a replay of the bug

https://codesandbox.io/p/sandbox/cranky-pine-f9s292?file=%2Fapp%2Fpage.tsx%3A1%2C1

To Reproduce

const BlogPage = async ({ params }: { params: { categorySlug: string; blogSlug: string; locale: string } }) => {
  const blog = (
    await dataService.collection<{ blogs: Blog[] }>('Blog', params.locale, {
      filters: {
        blog_category: {
          slug: { eq: params.categorySlug },
        },
        slug: { eq: params.blogSlug },
      },
    })
  ).blogs?.[0];

  if (!blog) {
    // Here the issue
    notFound();
  }

  return <>{JSON.stringify(blog, null, 2)}</>;
}

Describe the Bug

When notFound is called, the 404 page is correctly displayed, but the status is 200 :

image

Expected Behavior

The HTTP status code should be 404

Which browser are you using? (if relevant)

Chrome Version 114.0.5735.106 (Build officiel) (x86_64)

How are you deploying your application? (if relevant)

next start

fab1an commented 1 year ago

Is there a workaround for this? Will this be fixed?

caiolrm commented 1 year ago

@fab1an @robinComa

Two things were causing this behavior on my project:

Anyway, hope this is relevant to someone.

fab1an commented 1 year ago

@caiolrm Thank you for the tipp. I just removed the dynamic route and copy and pasted all files for every [id] that I need to support.

fab1an commented 1 year ago

@ijjk Sorry for involving you, but will this ever be fixed? This is a terrible, reproducible bug, that's been open for half a year.

Wrong status-codes served by a webserver should be a core concern.

The only way to workaround it is to not use dynamic routes. At that point I can go back to nginx/apache2.

Enngage commented 1 year ago

Same here... Are we seriously not able to set 404 status codes? The docs for notFound says:

/**
 * When used in a React server component, this will set the status code to 404.
 * When used in a custom app route it will just send a 404 status.
 */
export declare function notFound(): never;

But it clearly doesn't work.

fab1an commented 1 year ago

@Enngage The issue is with the compbination of caching and dynamic routes.

If you route uses notFound() AND contains an id like [id] it will serve 404 on the first load, but 200 on the subsequent cached loads.

I suspect this is why it wasn't fixed yet. It is too complex to reproduce.

Enngage commented 1 year ago

@Enngage The issue is with the compbination of caching and dynamic routes.

If you route uses notFound() AND contains an id like [id] it will serve 404 on the first load, but 200 on the subsequent cached loads.

I suspect this is why it wasn't fixed yet. It is too complex to reproduce.

Hey, it doesn't return 404 on first load... that's the problem because then google indexes the page. It's true we are using dynamic routes with url parameters.

We actually "solved" it by creating a "/404" route and redirected notFound items to this route manually. The /404 route seems special as it applies the 404 status code, but I haven't found it in docs. It was likely there in the old pages router, but not with the new one. It's all bit odd.

fab1an commented 1 year ago

@Enngage So you are using redirect to send 307 to the user, which then redirects to a 404 page. Are you sure this is not hurting SEO?

Enngage commented 1 year ago

@Enngage So you are using redirect to send 307 to the user, which then redirects to a 404 page. Are you sure this is not hurting SEO?

We are redirecting from server straight to '/404' route which applies the 404. Its the only way we could achieve 404 response status code and stop google from indexing every single path even if it does not exist.

I wish the 'notFound' method worked as documented, but it doesn't.

fab1an commented 1 year ago

Can you show us the function you use to do the redirect?

Enngage commented 1 year ago

Can you show us the function you use to do the redirect?

We are using redirect -> import { redirect } from 'next/navigation';

externl commented 1 year ago

This bug seems like a huge pain. Maybe I'm missing something, but how are you supposed to use dynamic routes if they can't be 404'd?

fab1an commented 1 year ago

Is this also fixed by #55542?

MaciejWira commented 1 year ago

@caiolrm , thanks for the tip! I removed loading.tsx and it worked.

sabaMohamadii commented 10 months ago

I have the same issue , i call notFound() but my doc of page is return 200 status code , how can change that to 404 ?????

Sitiapp commented 8 months ago

Next js 14 is so popular i wonder how such an issue is still present on the framework logic.. please put this as high priority we need 404 status code after loading not found pages!

leesaxby commented 8 months ago

We were also experiencing this issue and as mentioned by @caiolrm, removing the use client providers we had wrapping our component hierarchy fixed the issue.

Digging a little deeper we found the issue was actually being caused by useEffects in the client providers that were making async calls. Refactoring to remove these useEffects fixed the issue and the "not found" page now returns a status code of 404.

This also incidentally fixed an issue we were experiencing with generateMetadata not rendering the app if there was a delay when fetching dynamic meta data.

GXM245 commented 8 months ago

Also experiencing the same issue. Our structure looks like this and this will recreate the problem:

app -> [slug-route] -> homepage -> another-static-page -> another-static-page -> loading.js -> not-found.js

By removing the loading.js page the issue goes away and 404 is returned instead of 200 status for the not found page (for both client side and server side).

This is a serious issue as it forces you to choose between User Experience (loading page) and SEO (where 404s are critical for crawl budget and clarity) - two of the main reasons we chose Next JS. If anyone can look into/resolve this (e.g. @balazsorban44 ), we'd be very grateful.

MaciejWira commented 8 months ago

@GXM245, as a compromise - you can use something like progress bar: https://www.npmjs.com/package/next13-progressbar?activeTab=code (or write something similar by yourself). You can see such solution on github too. So there is no loading.js, but there is still an effect of loading state when clicking internal links.

GXM245 commented 8 months ago

Thanks @MaciejWira! I'll take a closer look at that - and yes, you're right, the most suitable option at this point would be to write something ourselves. I wonder how many of us have similar setups but unaware that our pages are, in fact, returning a 200 when they should be a 404. I only discovered this myself after a SEO crawl where a load of pages I expected to be a 404 were returning a 200.

@timneutkens would you be able to lend us your ear on this one? This is quite fundamental to the response codes that core Next JS functionality returns.

Javad9s commented 7 months ago

So right now Nextjs starts streaming data immediately with loading.js and because how the web works it has to have a status code 200. Later on while processing the data we decide it should be a notfound (which is cool btw) and can't change the status code. I suggest devs expose an experimental flag in NextConfig so that people can opt out of loading.js only if route is dynamic or even if it uses some special function that requires a change in status code (e.g. notfound, forbidden) Like experimental.ignoreSuspenseForStatus Code

danjiro commented 7 months ago

the same issue with redirect() where it will have a status code of 200 instead of 307. This is a pretty big issue SEO-wise

KurtRogers commented 7 months ago

Lvw

github-actions[bot] commented 6 months ago

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.