vercel / next.js

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

Preflight HEAD Request for Pages with Middlewares #34643

Closed deve-sh closed 2 years ago

deve-sh commented 2 years ago

Verify canary release

Provide environment information

Operating System: Platform: darwin Arch: x64 Version: Darwin Kernel Version 20.5.0: Sat May 8 05:10:33 PDT 2021; root:xnu-7195.121.3~9/RELEASE_X86_64 Binaries: Node: 14.17.3 npm: 6.14.13 Yarn: 1.22.11 Relevant packages: next: 12.0.10 react: 17.0.2 react-dom: 17.0.2

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

Describe the Bug

For each client-side navigation to a page that has a middleware associated with it, Next.js makes a HEAD request before the actual chunks are loaded and API Calls are processed from the getInitialProps or getServerSideProps blocks. This HEAD request is part of the preflight requests made by the Next.js router.

https://github.com/vercel/next.js/blob/eddabd98f8e4e5a82f4341bc1d8926959327919a/packages/next/shared/lib/router/router.ts#L1973-L1994

The issue is that this request is extremely slow and blocks all processing while it happens, i.e: Chunk loading and all other page calls. If I remove the middleware the HEAD Call goes away and the navigation is performant again, question is: Is this call required? And is there a way for it to be more optimized/faster?

image image

Expected Behavior

The HEAD request isn't adding anything to the page in terms of middleware since Middlewares don't run on the client-side, can the HEAD Call be removed on the client-side or at least made to run in parallel with the page-chunk load and other API Calls In getInitialProps or getServerSideProps or made more performant?

To Reproduce

In order to reproduce this issue, use the following steps:

balazsorban44 commented 2 years ago

This preflight request is needed so the client knows how to respond after the page transition. Middleware runs before any data fetching methods as well.

It is not supposed to be slow though, so could you maybe show your code so we can determine where the performance issue might be?

Note that we are working on failing on preflight request errors gracefully: https://github.com/vercel/next.js/issues/34199

balazsorban44 commented 2 years ago

For even more context, by default, the preflight request is made when the Link to the other page enters the viewport, so by the time you click it, it should already have been executed/finished. There is a high chance that you are doing an expensive request inside the Middleware (but even that is cached in production so should be fast), so seeing your code would be very helpful.

deve-sh commented 2 years ago

@balazsorban44 Thanks for the response. The thing is the main application code I can't share here, but here's a reproduction of the issue: https://codesandbox.io/s/empty-sun-ns5764

Can middleware preflight requests be run in parallel to chunk loads at least since the request is blocking the page from loading in the first place?

image

In dev it takes >500-1000ms for an empty page, in production for a slightly heavier request there is also a significant delay for the preflight HEAD request.

balazsorban44 commented 2 years ago

I get 502 Bad gateway on your reproduction.

Do note that the provided reproduction uses req.page which is due deprecation https://github.com/vercel/next.js/issues/34521

For performance debugging, using development (next dev), or even CodeSandbox is not ideal. You should test it on a production environment like Vercel, which uses Edge Functions to ensure fast response for Middleware. See a live demo at https://vercel.com/features/edge-functions (Note this is only to showcase an alternative to CodeSandbox. You can choose whatever optimized deployment environment that supports Next.js and Middleware.)

Are you doing any expensive fetch/database calls from your _middleware?

deve-sh commented 2 years ago

@balazsorban44 Hey, sorry about that 502, I've fixed the issue present in the sandbox/reproduction. You can access it at this new sandbox: https://kmn01t.sse.codesandbox.io/

We actually encountered this issue in the production environment itself, we're not making any heavy API Calls in the middleware, in fact, it runs only for one single page using an if block and all API Calls run inside an if block.

This entire issue will be solved if there is a scoped middleware for index pages from the discussion: https://github.com/vercel/next.js/discussions/33276

balazsorban44 commented 2 years ago

Could you please attach your code? :pray: That would be very helpful to further the investigation.

May I ask where you are hosting your site?

CodeSandbox is running a dev server of Next.js, so it is not a good example here, as it isn't optimized for performance and the longer response times can be explained by page compilation happening before the page transition.

I don't see how #33276 is related to slow Middleware calls.

deve-sh commented 2 years ago

Hey @balazsorban44 The code is as follows:

// _middleware.ts
export default async function middleware(req: NextRequest) {
    if (req.page.name === "/entity/[entitySlug]/[entityId]") {
        // Run middleware only for the root route in the pages/entity/[entitySlug]/[entityId] folder.

        const { token } = req.cookies;
        if (!token) return NextResponse.redirect("/");

        const { entityId } = req.page.params || {};

        const [user, entityInfo, ...rest] = await Promise.all([
            getUserInfo(token),
            getEntityInfo(entityId),
            // ...3 Other API Calls,
        ]);

        if (!user || !entityInfo) return NextResponse.redirect("/");

        // Performs some checks for the entity static page, based on user info.
        // Redirects to appropriate routes based on those checks.
    }
    return NextResponse.next();
}

We are hosting on a private cluster on AWS servers.

The reason I said #33276 will solve our issue is that the directory /pages/[entitySlug]/[entityId] has a lot of other pages as well where we don't need a middleware, we need it only for the root/index page, adding a middleware file to that directory invokes the middleware for all other pages as well.

balazsorban44 commented 2 years ago

So after discussing this, based on your comment:

We are hosting on a private cluster on AWS servers.

This is likely an issue with your infrastructure. Next.js does not have a guarantee for fast responses if the underlying infra isn't optimized.

Keep in mind that hosting providers like Vercel do not run your Middleware on AWS Lambda, in order to always have a fast response (by avoiding cold starts, etc). You will have to open a ticket with them to ask if there is a way to eliminate slow responses for your case.

Also again, make sure that you do not test performance in the development environment.

github-actions[bot] commented 2 years ago

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