fix: Ensure middleware rewrite status code is properly propagated in cache interceptor

Author: sommeeeer

packages/open-next/src/adapters/middleware.ts

@@ -125,7 +125,13 @@ const defaultHandler = async (
         }
       }
 
-      result.headers[INTERNAL_EVENT_REQUEST_ID] = requestId;
+      if (process.env.OPEN_NEXT_REQUEST_ID_HEADER || globalThis.openNextDebug) {
+        result.headers[INTERNAL_EVENT_REQUEST_ID] = requestId;
+      }
+
+      // Can safely delete this header here before returning as it is only used for the cache tags internally
+      delete result.headers["x-next-cache-tags"];
+
       debug("Middleware response", result);
       return result;
     },

packages/open-next/src/core/requestHandler.ts

@@ -63,7 +63,7 @@ export async function openNextHandler(
       }
       debug("internalEvent", internalEvent);
 
-      // These 2 will get overwritten by the routing handler if not using an external middleware
+      // These 3 will get overwritten by the routing handler if not using an external middleware
       const internalHeaders = {
         initialPath:
           initialHeaders[INTERNAL_HEADER_INITIAL_URL] ?? internalEvent.rawPath,

packages/open-next/src/core/routing/cacheInterceptor.ts

@@ -132,8 +132,12 @@ async function generateResult(
   );
   return {
     type: "core",
-    // sometimes other status codes can be cached, like 404. For these cases, we should return the correct status code
-    statusCode: cachedValue.meta?.status ?? 200,
+    // Sometimes other status codes can be cached, like 404. For these cases, we should return the correct status code
+    // Also set the status code to the rewriteStatusCode if defined
+    // This can happen in handleMiddleware in routingHandler.
+    // `NextResponse.rewrite(url, { status: xxx})
+    // The rewrite status code should take precedence over the cached one
+    statusCode: event.rewriteStatusCode ?? cachedValue.meta?.status ?? 200,
     body: toReadableStream(body, false),
     isBase64Encoded: false,
     headers: {
@@ -287,7 +291,8 @@ export async function cacheInterceptor(
 
           return {
             type: "core",
-            statusCode: cachedData.value.meta?.status ?? 200,
+            statusCode:
+              event.rewriteStatusCode ?? cachedData.value.meta?.status ?? 200,
             body: toReadableStream(cachedData.value.body, isBinary),
             headers: {
               ...cacheControl,

packages/open-next/src/core/routing/middleware.ts

@@ -198,6 +198,7 @@ export async function handleMiddleware(
     cookies: internalEvent.cookies,
     remoteAddress: internalEvent.remoteAddress,
     isExternalRewrite,
-    rewriteStatusCode: statusCode,
+    rewriteStatusCode:
+      rewriteUrl && !isExternalRewrite ? statusCode : undefined,
   } satisfies MiddlewareEvent;
 }

packages/open-next/src/types/open-next.ts

@@ -30,6 +30,8 @@ export type InternalEvent = {
   readonly query: Record<string, string | string[]>;
   readonly cookies: Record<string, string>;
   readonly remoteAddress: string;
+  // Used for NextResponse.rewrite's status code in middleware
+  readonly rewriteStatusCode?: number;
 } & BaseEventOrResult<"core">;
 
 export type InternalResult = {

packages/tests-e2e/tests/appRouter/middleware.rewrite.test.ts

@@ -23,21 +23,21 @@ test("Middleware Rewrite", async ({ page }) => {
 });
 
 test("Middleware Rewrite External Image", async ({ page }) => {
-  await page.goto("/rewrite-external");
   page.on("response", async (response) => {
     expect(response.status()).toBe(200);
     expect(response.headers()["content-type"]).toBe("image/png");
     expect(response.headers()["cache-control"]).toBe("max-age=600");
     const bodyBuffer = await response.body();
     expect(validateMd5(bodyBuffer, OPENNEXT_PNG_MD5)).toBe(true);
   });
+  await page.goto("/rewrite-external");
 });
 
 test("Middleware Rewrite Status Code", async ({ page }) => {
-  await page.goto("/rewrite-status-code");
-  const el = page.getByText("Rewritten Destination", { exact: true });
-  await expect(el).toBeVisible();
   page.on("response", async (response) => {
     expect(response.status()).toBe(403);
   });
+  await page.goto("/rewrite-status-code");
+  const el = page.getByText("Rewritten Destination", { exact: true });
+  await expect(el).toBeVisible();
 });

packages/tests-unit/tests/core/routing/cacheInterceptor.test.ts

@@ -45,6 +45,7 @@ function createEvent(event: PartialEvent): InternalEvent {
     query: convertFromQueryString(qs ?? ""),
     cookies: event.cookies ?? {},
     remoteAddress: event.remoteAddress ?? "::1",
+    rewriteStatusCode: event.rewriteStatusCode,
   };
 }
 
@@ -452,4 +453,72 @@ describe("cacheInterceptor", () => {
       }),
     );
   });
+
+  it("should return the rewrite status code when there is active cache", async () => {
+    const event = createEvent({
+      url: "/albums",
+      rewriteStatusCode: 403,
+    });
+    incrementalCache.get.mockResolvedValueOnce({
+      value: {
+        type: "app",
+        html: "Hello, world!",
+      },
+    });
+
+    const result = await cacheInterceptor(event);
+    expect(result.statusCode).toBe(403);
+  });
+
+  it("should return the rewriteStatusCode if there is a cached status code", async () => {
+    const event = createEvent({
+      url: "/albums",
+      rewriteStatusCode: 203,
+    });
+    incrementalCache.get.mockResolvedValueOnce({
+      value: {
+        type: "app",
+        html: "Hello, world!",
+        meta: {
+          status: 404,
+        },
+      },
+    });
+
+    const result = await cacheInterceptor(event);
+    expect(result.statusCode).toBe(203);
+  });
+
+  it("should return the cached status code if there is one", async () => {
+    const event = createEvent({
+      url: "/albums",
+    });
+    incrementalCache.get.mockResolvedValueOnce({
+      value: {
+        type: "app",
+        html: "Hello, world!",
+        meta: {
+          status: 405,
+        },
+      },
+    });
+
+    const result = await cacheInterceptor(event);
+    expect(result.statusCode).toBe(405);
+  });
+
+  it("should return 200 if there is no cached status code, nor a rewriteStatusCode", async () => {
+    const event = createEvent({
+      url: "/albums",
+    });
+    incrementalCache.get.mockResolvedValueOnce({
+      value: {
+        type: "app",
+        html: "Hello, world!",
+      },
+    });
+
+    const result = await cacheInterceptor(event);
+    expect(result.statusCode).toBe(200);
+  });
 });