chore: gracefully handle 404s from pages router's dynamic pages + tests (#18618)

* restore pages/_error * set custom header in pages/_error * handle it in middleware * add test * remove logs * better test description
2025-01-13 15:46:56 +05:30
parent b3ed6e39e3
commit 0befdde5d7
3 changed files with 135 additions and 0 deletions
@@ -22,6 +22,11 @@ const safeGet = async <T = any>(key: string): Promise<T | undefined> => {
 const middleware = async (req: NextRequest): Promise<NextResponse<unknown>> => {
  const url = req.nextUrl;
  const requestHeaders = new Headers(req.headers);
+  const response = NextResponse.next();
+
+  if (response.headers.get("x-pages-router-error") === "true") {
+    return NextResponse.rewrite(new URL("/_not-found", req.url));
+  }

  requestHeaders.set("x-url", req.url);

@@ -0,0 +1,115 @@
+/**
+ * Typescript class based component for custom-error
+ * @link https://nextjs.org/docs/advanced-features/custom-error-page
+ */
+import type { NextPage, NextPageContext } from "next";
+import type { ErrorProps } from "next/error";
+import NextError from "next/error";
+import React from "react";
+
+import { getErrorFromUnknown } from "@calcom/lib/errors";
+import { HttpError } from "@calcom/lib/http-error";
+import logger from "@calcom/lib/logger";
+import { redactError } from "@calcom/lib/redactError";
+
+import { ErrorPage } from "@components/error/error-page";
+
+// Adds HttpException to the list of possible error types.
+type AugmentedError = (NonNullable<NextPageContext["err"]> & HttpError) | null;
+type CustomErrorProps = {
+  err?: AugmentedError;
+  message?: string;
+  hasGetInitialPropsRun?: boolean;
+} & Omit<ErrorProps, "err">;
+
+type AugmentedNextPageContext = Omit<NextPageContext, "err"> & {
+  err: AugmentedError;
+};
+
+const log = logger.getSubLogger({ prefix: ["[error]"] });
+
+const CustomError: NextPage<CustomErrorProps> = (props) => {
+  const { statusCode, err, message, hasGetInitialPropsRun } = props;
+
+  if (!hasGetInitialPropsRun && err) {
+    // getInitialProps is not called in case of
+    // https://github.com/vercel/next.js/issues/8592. As a workaround, we pass
+    // err via _app.tsx so it can be captured
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    const e = getErrorFromUnknown(err);
+    // can be captured here
+    // e.g. Sentry.captureException(e);
+  }
+  return <ErrorPage statusCode={statusCode} error={err} message={message} />;
+};
+
+/**
+ * Partially adapted from the example in
+ * https://github.com/vercel/next.js/tree/canary/examples/with-sentry
+ */
+CustomError.getInitialProps = async (ctx: AugmentedNextPageContext) => {
+  const { res, err, asPath } = ctx;
+  const errorInitialProps = (await NextError.getInitialProps({
+    res,
+    err,
+  } as NextPageContext)) as CustomErrorProps;
+
+  // Workaround for https://github.com/vercel/next.js/issues/8592, mark when
+  // getInitialProps has run
+  errorInitialProps.hasGetInitialPropsRun = true;
+
+  // If a HttpError message, let's override defaults
+  if (err instanceof HttpError) {
+    const redactedError = redactError(err);
+    errorInitialProps.statusCode = err.statusCode;
+    errorInitialProps.title = redactedError.name;
+    errorInitialProps.message = redactedError.message;
+    errorInitialProps.err = {
+      ...redactedError,
+      url: err.url,
+      statusCode: err.statusCode,
+      cause: err.cause,
+      method: err.method,
+    };
+  }
+
+  if (res) {
+    // Running on the server, the response object is available.
+    //
+    // Next.js will pass an err on the server if a page's `getInitialProps`
+    // threw or returned a Promise that rejected
+
+    // Overrides http status code if present in errorInitialProps
+    res.statusCode = errorInitialProps.statusCode;
+
+    log.debug(`server side logged this: ${err?.toString() ?? JSON.stringify(err)}`);
+    log.info("return props, ", errorInitialProps);
+
+    res.setHeader("x-pages-router-error", "true");
+
+    return errorInitialProps;
+  } else {
+    // Running on the client (browser).
+    //
+    // Next.js will provide an err if:
+    //
+    //  - a page's `getInitialProps` threw or returned a Promise that rejected
+    //  - an exception was thrown somewhere in the React lifecycle (render,
+    //    componentDidMount, etc) that was caught by Next.js's React Error
+    //    Boundary. Read more about what types of exceptions are caught by Error
+    //    Boundaries: https://reactjs.org/docs/error-boundaries.html
+    if (err) {
+      log.info("client side logged this", err);
+      return errorInitialProps;
+    }
+  }
+
+  // If this point is reached, getInitialProps was called without any
+  // information about what the error might be. This is unexpected and may
+  // indicate a bug introduced in Next.js
+  new Error(`_error.tsx getInitialProps missing data at path: ${asPath}`);
+
+  return errorInitialProps;
+};
+
+export default CustomError;
@@ -0,0 +1,15 @@
+import { expectPageToBeNotFound } from "playwright/lib/testUtils";
+
+import { test } from "./lib/fixtures";
+
+test.describe("App Router - error handling", () => {
+  test("404s must render App Router's not-found page regardless of whether pages router or app router is used", async ({
+    page,
+  }) => {
+    await expectPageToBeNotFound({ page, url: "/123491234" });
+    await expectPageToBeNotFound({ page, url: "/team/123491234" });
+    await expectPageToBeNotFound({ page, url: "/org/123491234" });
+    await expectPageToBeNotFound({ page, url: "/insights/123491234" });
+    await expectPageToBeNotFound({ page, url: "/login/123491234" });
+  });
+});