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

Middleware NextResponse.rewrite 404ing when rewriting "/" with another route when deployed to Vercel #33044

Closed
wadehammes opened this issue Jan 5, 2022 · 21 comments
Labels
bug Issue was opened via the bug report template. Middleware Related to Next.js Middleware

Comments

@wadehammes
Copy link

wadehammes commented Jan 5, 2022

Run next info (available from version 12.0.8 and up)

Operating System:
  Platform: linux
  Arch: x64
  Version: #1 SMP Mon Oct 18 19:27:44 UTC 2021
Binaries:
  Node: 14.18.0
  npm: 6.14.15
  Yarn: 1.22.17
  pnpm: 6.24.4
Relevant packages:
  next: 12.0.8-canary.18
  react: 17.0.2
  react-dom: 17.0.2

What version of Next.js are you using?

12.0.8-canary.18

What version of Node.js are you using?

14.18.0

What browser are you using?

Firefox

What operating system are you using?

Windows

How are you deploying your application?

Vercel

Describe the Bug

Using middleware, NextResponse.rewrite is not correctly rewriting a route with another existing route. Doing so results in a 404.

I am using i18n, but #31174 does not fix it.

Seems related to #30749

Here is my middleware file:

import { NextRequest, NextResponse } from "next/server";
import { CONSTANTS } from "src/utils/constants";

const ONE_YEAR_IN_MS = 3600 * 365 * 24 * 1000;

export function middleware(req: NextRequest) {
  const expires = new Date(Date.now() + ONE_YEAR_IN_MS);
  const initialVariant = req.cookies[CONSTANTS.RH_HOME_PAGE_TEST_COOKIE];

  // Redirect direct traffic to our test variant
  if (
    (!initialVariant ||
      initialVariant !== CONSTANTS.RH_HOME_PAGE_VARIANT_1_SLUG) &&
    req.nextUrl.pathname === `/${CONSTANTS.RH_HOME_PAGE_VARIANT_1_SLUG}`
  ) {
    return NextResponse.redirect(CONSTANTS.HOME_ROUTE);
  }

  // Home page AB testing
  if (req.nextUrl.pathname === CONSTANTS.HOME_ROUTE) {
    let variant = initialVariant;

    // Determine the variant via a random split
    if (!variant) {
      const split: boolean = Math.random() >= 0.5;

      variant = split
        ? CONSTANTS.RH_HOME_PAGE_VARIANT_1_SLUG
        : CONSTANTS.RH_HOME_PAGE_ORIGINAL;
    }

    const res =
      variant === CONSTANTS.RH_HOME_PAGE_ORIGINAL
        ? NextResponse.next()
        : NextResponse.redirect(`/${variant}`, 307); // Temporary redirect

    if (initialVariant !== variant) {
      res.cookie(CONSTANTS.RH_HOME_PAGE_TEST_COOKIE, variant, { expires });
    }

    return res;
  } else {
    return NextResponse.next();
  }
}

My next.config:

module.exports = {
  productionBrowserSourceMaps: true,
  outputFileTracing: false,
  images: {
    domains: [
      "images.ctfassets.net",
      "assets.ctfassets.net",
      "videos.ctfassets.net",
      "assets.zappyride.com",
    ],
  },
  i18n: {
    locales: ["en", "es"],
    defaultLocale: "en",
  },
  env: {},
  webpack: (config, options) => {
    if (!options.isServer) {
      config.resolve.alias["@sentry/node"] = "@sentry/react";
    }

    // Only add sentry webpack plugin when sentry DSN is defined and the
    // app is being deployed (NODE_ENV=production when making an optimized build
    // for a deployed environment).
    if (
      process.env.NEXT_PUBLIC_SENTRY_DSN &&
      process.env.NODE_ENV === "production"
    ) {
      config.plugins.push(
        new SentryWebpackPlugin({
          include: ".next",
          ignore: ["node_modules"],
          urlPrefix: "~/_next",
          release: process.env.VERCEL_GITHUB_COMMIT_SHA,
        })
      );
    }

    config.resolve.alias = {
      ...config.resolve.alias,
      "@mui/styled-engine": "@mui/styled-engine-sc",
    };

    return config;
  },
  async redirects() {
    if (process.env.ENVIRONMENT === "production") {
      return [...productionRedirects, ...sharedRedirects];
    } else {
      return [...sharedRedirects];
    }
  },
  async headers() {
    return [
      {
        source: "/",
        headers: [
          {
            key: "Cache-Control",
            value: "s-maxage=1, stale-while-revalidate",
          },
          ...securityHeaders,
        ],
      },
      {
        source: "/:path*",
        headers: [
          {
            key: "Cache-Control",
            value: "s-maxage=1, stale-while-revalidate",
          },
          ...securityHeaders,
        ],
      },
      {
        source: "/fonts/Averta/(.*)",
        headers: [
          {
            key: "Cache-Control",
            value: "public, max-age=31536000, stale-while-revalidate",
          },
          ...securityHeaders,
        ],
      },
      {
        source: "/fonts/fontface.css",
        headers: [
          {
            key: "Cache-Control",
            value: "public, max-age=31536000, stale-while-revalidate",
          },
          ...securityHeaders,
        ],
      },
    ];
  },
};

Expected Behavior

The rewrite should happen as it does locally on Vercel and not 404.

@wadehammes wadehammes added the bug Issue was opened via the bug report template. label Jan 5, 2022
@wadehammes
Copy link
Author

Changing if (req.url === "/") to if (req.nextUrl.pathname === "/") seems to have corrected this issue.

@wadehammes wadehammes reopened this Jan 6, 2022
@wadehammes
Copy link
Author

I am now getting a 404 and an error when trying to overwrite the home url "/" with another route, and accessing it from Vercel. Again, all works fine locally:

image

@balazsorban44
Copy link
Member

Judging from the error in the console, it might be a duplicate of #32549

@wadehammes
Copy link
Author

wadehammes commented Jan 6, 2022

@balazsorban44 seems similar. Here is my updated middleware:

import { NextRequest, NextResponse } from "next/server";
import { CONSTANTS } from "src/utils/constants";

const ONE_YEAR_IN_MS = 3600 * 365 * 24 * 1000;

export function middleware(req: NextRequest) {
  const expires = new Date(Date.now() + ONE_YEAR_IN_MS);

  // Home page AB testing
  if (req.nextUrl.pathname === "/") {
    let variant = req.cookies[CONSTANTS.RH_HOME_PAGE_TEST_COOKIE];

    // Determine the variant via a random split
    if (!variant) {
      const split: boolean = Math.random() >= 0.5; // 50/50 chance

      variant = split
        ? CONSTANTS.RH_HOME_PAGE_VARIANT_1_ROUTE
        : CONSTANTS.RH_HOME_PAGE_ORIGINAL_ROUTE;
    }

    // If variant is not original, redefine route to variant
    const route =
      variant === CONSTANTS.RH_HOME_PAGE_VARIANT_1_ROUTE
        ? `/${CONSTANTS.RH_HOME_PAGE_VARIANT_1_ROUTE}`
        : "/";

    const res = NextResponse.rewrite(route);

    res.cookie(CONSTANTS.RH_HOME_PAGE_TEST_COOKIE, variant, { expires });

    return res;
  } else {
    return NextResponse.next();
  }
}

Essentially, I am trying to perform an AB test using the original home route ("/") and a variant ("/variant-home-refresh"). I am able to see the variant if I navigate to the route directly, however any time the original route is rewritten, the home page 404s and any links to "/" do not work throughout the site (logo, etc.).

I definitely think this could be related to i18n.

@wadehammes
Copy link
Author

Just to follow up. NextResponse.redirect() works as intended with no issues, locally and on Vercel. This obviously comes with the caveat of having the test variant slug in the url, but we are going to work with that for now until rewrite is working properly.

Updated middleware for those who come by this:

import { NextRequest, NextResponse } from "next/server";
import { CONSTANTS } from "src/utils/constants";

const ONE_YEAR_IN_MS = 3600 * 365 * 24 * 1000;

export function middleware(req: NextRequest) {
  const expires = new Date(Date.now() + ONE_YEAR_IN_MS);
  const initialVariant = req.cookies[CONSTANTS.RH_HOME_PAGE_TEST_COOKIE];

  // Redirect direct traffic to our test variant
  if (
    (!initialVariant ||
      initialVariant !== CONSTANTS.RH_HOME_PAGE_VARIANT_1_SLUG) &&
    req.nextUrl.pathname === `/${CONSTANTS.RH_HOME_PAGE_VARIANT_1_SLUG}`
  ) {
    return NextResponse.redirect(CONSTANTS.HOME_ROUTE);
  }

  // Home page AB testing
  if (req.nextUrl.pathname === CONSTANTS.HOME_ROUTE) {
    let variant = initialVariant;

    // Determine the variant via a random split
    if (!variant) {
      const split: boolean = Math.random() >= 0.5;

      variant = split
        ? CONSTANTS.RH_HOME_PAGE_VARIANT_1_SLUG
        : CONSTANTS.RH_HOME_PAGE_ORIGINAL;
    }

    const res =
      variant === CONSTANTS.RH_HOME_PAGE_ORIGINAL
        ? NextResponse.next()
        : NextResponse.redirect(`/${variant}`, 307); // Temporary redirect

    if (initialVariant !== variant) {
      res.cookie(CONSTANTS.RH_HOME_PAGE_TEST_COOKIE, variant, { expires });
    }

    return res;
  } else {
    return NextResponse.next();
  }
}

@wadehammes wadehammes changed the title Middleware not writing cookies when deployed to Vercel Middleware NextResponse.rewrite 404ing when rewriting "/" with another route Jan 6, 2022
@wadehammes wadehammes changed the title Middleware NextResponse.rewrite 404ing when rewriting "/" with another route Middleware NextResponse.rewrite 404ing when rewriting "/" with another route when deployed to Vercel Jan 6, 2022
@javivelasco javivelasco added this to the Next.js Middleware GA milestone Jan 20, 2022
@balazsorban44
Copy link
Member

Could you add a deployed minimal reproduction @wadehammes? 🙏 And to clarify, this is only an issue when deployed, but works locally?

@balazsorban44 balazsorban44 added the Middleware Related to Next.js Middleware label Jan 20, 2022
@wadehammes
Copy link
Author

I don't think I'll have time to do that any time soon but will try.

@wadehammes
Copy link
Author

Yes works locally, not deployed.

Stack is latest Next, i18n configured with two locales, Contentful CMS. Redirect works like a charm, rewrite causes 404.

@expjazz
Copy link

expjazz commented Jan 25, 2022

I am facing the same issue. Redirect works, and rewrite causes a 404. The issue happens only when deployed to vercel.

@augustosamame

This comment has been minimized.

@JackOddy
Copy link

JackOddy commented Feb 2, 2022

not sure if this is related by we were experiencing rewrites failing to do anything when navigating client side using next/link - upgraded next.js to 12.0.10 and converted our rewrite logic to middleware and still seeing the same issue.

client side navigation does not rewrite correctly, whereas refreshing the page will trigger the rewrite - in both cases the middleware is triggered and NextResponse.rewrite(...) is returned from it

@remorses
Copy link
Contributor

remorses commented Feb 3, 2022

I had the same issue, 404 pages only when deploying on Vercel and the problem was that i was not passing the basePath to the rewrite function, the solution was to use something like this

import { NextRequest, NextResponse } from 'next/server'

export default function middleware(req: NextRequest) {
    const { pathname, basePath } = req.nextUrl

    const hostname = req.headers.get('host')

    // does not work
    // `/_hosts/${hostname}${pathname}`

    return NextResponse.rewrite(
        new URL(
            `${basePath}/_hosts/${hostname}${pathname}`,
            req.nextUrl.origin,
        ).toString(),
    )
}

The same problem probably exists when using i18n, you need to pass the base paths manually

PS: i made a full repo with an example of this working here

@wadehammes
Copy link
Author

wadehammes commented Feb 3, 2022

I had the same issue, 404 pages only when deploying on Vercel and the problem was that i was not passing the basePath to the rewrite function, the solution was to use something like this

import { NextRequest, NextResponse } from 'next/server'



export default function middleware(req: NextRequest) {

    const { pathname, basePath } = req.nextUrl



    const hostname = req.headers.get('host')



    // does not work

    // `/_hosts/${hostname}${pathname}`



    return NextResponse.rewrite(

        new URL(

            `${basePath}/_hosts/${hostname}${pathname}`,

            req.nextUrl.origin,

        ).toString(),

    )

}

The same problem probably exists when using i18n, you need to pass the base paths manually

PS: i made a full repo with an example of this working here

This didn't work for me unfortunately. Still getting a 404 using the approach above.

@remorses
Copy link
Contributor

remorses commented Feb 5, 2022

@wadehammes you have to add the locale to the path like this

return NextResponse.rewrite(
        new URL(
            `${basePath}/${locale}/_hosts/${hostname}${pathname}`,
            req.nextUrl.origin,
        ).toString(),
    )

You can skip the basePath if you are not using it

I made a working example here (locales branch)

@youminkim
Copy link

I had same 404 issue and it was because I did not put locale in the pathname as @remorses mentioned. For example,

  • /a/123 -> 404
  • /en/a123 -> works

It was pretty tricky to find this solution. Probably better to mention this in documentation at least. @steven-tey

@wadehammes
Copy link
Author

I had same 404 issue and it was because I did not put locale in the pathname as @remorses mentioned. For example,

* /a/123 -> 404

* /en/a123 -> works

It was pretty tricky to find this solution. Probably better to mention this in documentation at least. @steven-tey

Can confirm this works, thanks @youminkim:

NextResponse.rewrite(
   new URL(
      `${basePath}/${locale}/`,
      origin
   ).toString()
);

@pekac
Copy link

pekac commented Feb 18, 2022

Hey guys! 👋

Got the same issue. Middleware works locally but I keep getting 404 for the rewritten pages once the app is deployed to vercel.

I managed to create a repo for the issue that can be found here:
https://github.com/pekac/next-middleware

Also the mini-app has been deployed here:
https://next-middleware-pink.vercel.app/

If you have any questions regarding the repo - feel free to reach out!

Hope this helps 😁

@cungminh2710
Copy link

@wadehammes you have to add the locale to the path like this

return NextResponse.rewrite(
        new URL(
            `${basePath}/${locale}/_hosts/${hostname}${pathname}`,
            req.nextUrl.origin,
        ).toString(),
    )

You can skip the basePath if you are not using it

I made a working example here (locales branch)

Not working for me unfortunately

@gijsbotje
Copy link

We are having the same issue for a site we first published using docker containers.
The site works fine on the docker deployment: https://nextjs-azalp-pltwrlrbza-ew.a.run.app/tuinhout .
Works fine locally, but the same page gets a 404 when deployed to Vercel.
Client-side routing works fine, but approaching the URL directly on the Vercel deployment doesn't: https://nextjs-azalp-9rpw5y8um-afosto.vercel.app/tuinhout

We redirect this slug to '/collection/[...slug]' as we want to use the URL's existing in our previous setup.
Why does this not work?

// slug: request.nextUrl.pathname (in this example it will be '/tuinhout')
// type: 'collection' (fetched from api or redis database)
const url = request.nextUrl.clone();
url.pathname = `${type}${slug}`;
return NextResponse.rewrite(url);

@javivelasco
Copy link
Member

Hi! We've been working a lot on improving the dev experience and the API for middleware and I've been testing this example with the most recent canary that was just published to make sure it doesn't happen anymore. There seems to be a few duplicates of this issue so I'm going to also share some context that could be useful in those.

First of all, note that Middleware is designed to provide the maximum freedom to users so there are some things that we can't make implicit like, for example, preserving the original locale since we don't know if the user is trying to remove the locale or keep it. This plus the fact that we require absolute URLs for when Middleware is invoked implies that the user must express a rewrite always using a full URL. To make things more convenient we are providing within NextRequest a primitive NextURL that will parse the root path, current locale, etc.

From the OPs code I understand the intention is to preserve the locale so NextResponse.rewrite('/') doesn't even work anymore because the provided URL must be absolute and, if it was, you'd be rewriting always to the default locale without preserving the current request one. The way to achieve what you want is by mutating request.nextUrl.pathname and then stringifying the object. The code would look similar to this:

import { NextRequest, NextResponse } from "next/server";

const CONSTANTS = {
  RH_HOME_PAGE_TEST_COOKIE: 'ab-test-cookie',
  RH_HOME_PAGE_VARIANT_1_ROUTE: 'index-a',
  RH_HOME_PAGE_ORIGINAL_ROUTE: 'index'
}

export function middleware(req: NextRequest) {
  // Home page AB testing
  if (req.nextUrl.pathname === "/") {
    let variant = req.cookies.get(CONSTANTS.RH_HOME_PAGE_TEST_COOKIE);

    // Determine the variant via a random split
    if (!variant) {
      variant = Math.random() >= 0.5 // 50/50 chance
        ? CONSTANTS.RH_HOME_PAGE_VARIANT_1_ROUTE
        : CONSTANTS.RH_HOME_PAGE_ORIGINAL_ROUTE;
    }

    // If variant is not original, redefine route to variant
    req.nextUrl.pathname = variant === CONSTANTS.RH_HOME_PAGE_VARIANT_1_ROUTE
      ? `/${CONSTANTS.RH_HOME_PAGE_VARIANT_1_ROUTE}`
      : "/"
    
    const res = NextResponse.rewrite(req.nextUrl);
    res.cookies.set(CONSTANTS.RH_HOME_PAGE_TEST_COOKIE, variant, {
      expires: new Date(Date.now() + 3600 * 365 * 24 * 1000)
    })

    return res;
  }
}

Note that the rewrite target will be String(req.nextUrl) which will respect the same host, root path, locale, etc since the only thing we are changing is the pathname. This example also updates the cookie API to be the latest.

I've tested this both locally and in production and it works good. Tried out:

  • Getting the page from incognito from to different locales so I get a different version 50% randomly.
  • Navigating to /about from the root page / and then back to root from the about page.
  • The same as before but using a specific locale /es/about and /es.

Everything worked fine in Vercel, Server and Dev modes.
Please reopen if you can still reproduce the issue with the latest canary 🙏

@github-actions
Copy link
Contributor

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 18, 2022
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. Middleware Related to Next.js Middleware
Projects
None yet
Development

No branches or pull requests