fix(routing): return correct status code for 500.astro and 404.astro (#11308)

* fix(routing): return correct status code for `500.astro` and `404.astro`

* changeset

* fix regression

* use `route` instead

.changeset/curvy-emus-mate.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes an issue where custom `404.astro` and `500.astro` were not returning the correct status code when rendered inside a rewriting cycle.

packages/astro/src/core/cookies/response.ts

@@ -10,7 +10,7 @@ export function responseHasCookies(response: Response): boolean {
 	return Reflect.has(response, astroCookiesSymbol);
 }
 
-export function getFromResponse(response: Response): AstroCookies | undefined {
+export function getCookiesFromResponse(response: Response): AstroCookies | undefined {
 	let cookies = Reflect.get(response, astroCookiesSymbol);
 	if (cookies != null) {
 		return cookies as AstroCookies;
@@ -20,7 +20,7 @@ export function getFromResponse(response: Response): AstroCookies | undefined {
 }
 
 export function* getSetCookiesFromResponse(response: Response): Generator<string, string[]> {
-	const cookies = getFromResponse(response);
+	const cookies = getCookiesFromResponse(response);
 	if (!cookies) {
 		return [];
 	}

packages/astro/src/core/render-context.ts

@@ -27,7 +27,7 @@ import {
 	responseSentSymbol,
 } from './constants.js';
 import { AstroCookies, attachCookiesToResponse } from './cookies/index.js';
-import { getFromResponse } from './cookies/response.js';
+import { getCookiesFromResponse } from './cookies/response.js';
 import { AstroError, AstroErrorData } from './errors/index.js';
 import { callMiddleware } from './middleware/callMiddleware.js';
 import { sequence } from './middleware/index.js';
@@ -136,6 +136,7 @@ export class RenderContext {
 		const lastNext = async (ctx: APIContext, payload?: RewritePayload) => {
 			if (payload) {
 				if (this.pipeline.manifest.rewritingEnabled) {
+					pipeline.logger.debug('router', 'Called rewriting to:', payload);
 					// we intentionally let the error bubble up
 					const [routeData, component] = await pipeline.tryRewrite(
 						payload,
@@ -197,7 +198,7 @@ export class RenderContext {
 			}
 			// We need to merge the cookies from the response back into this.cookies
 			// because they may need to be passed along from a rewrite.
-			const responseCookies = getFromResponse(response);
+			const responseCookies = getCookiesFromResponse(response);
 			if (responseCookies) {
 				cookies.merge(responseCookies);
 			}

packages/astro/src/runtime/server/render/page.ts

@@ -1,4 +1,4 @@
-import type { RouteData, SSRResult } from '../../../@types/astro.js';
+import type {AstroConfig, RouteData, SSRResult} from '../../../@types/astro.js';
 import { type NonAstroPageComponent, renderComponentToString } from './component.js';
 import type { AstroComponentFactory } from './index.js';
 
@@ -85,6 +85,17 @@ export async function renderPage(
 	if (route?.component.endsWith('.md')) {
 		headers.set('Content-Type', 'text/html; charset=utf-8');
 	}
-	const response = new Response(body, { ...init, headers });
-	return response;
+	let status = init.status;
+	// Custom 404.astro and 500.astro are particular routes that must return a fixed status code
+	if (route?.route === "/404") {
+		status = 404
+	} else if (route?.route === "/500") {
+		status = 500
+	}
+	if (status) {
+		return new Response(body, { ...init, headers, status });
+	} else {
+		return new Response(body, { ...init, headers });
+		
+	}
 }

packages/astro/test/custom-404-implicit-rerouting.test.js

@@ -58,7 +58,7 @@ for (const caseNumber of [1, 2, 3, 4, 5]) {
 			});
 
 			// IMPORTANT: never skip
-			it('prod server stays responsive', { timeout: 1000 }, async () => {
+			it('prod server stays responsive for case number ' + caseNumber, { timeout: 1000 }, async () => {
 				const response = await app.render(new Request('https://example.com/alvsibdlvjks'));
 				assert.equal(response.status, 404);
 			});

packages/astro/test/fixtures/rewrite-custom-404/package.json

@@ -1,5 +1,5 @@
 {
-  "name": "@test/rewrite-404-invalid",
+  "name": "@test/rewrite-custom-404",
   "version": "0.0.0",
   "private": true,
   "dependencies": {

packages/astro/test/fixtures/rewrite-custom-404/src/middleware.js

@@ -0,0 +1,10 @@
+
+
+export const onRequest = async (context, next) => {
+		if (context.url.pathname.startsWith("/404") || context.url.pathname.startsWith("/500")) {
+			context.locals = {
+				interjected: "Interjected"
+			}
+		}
+		return  await next();
+}

packages/astro/test/fixtures/rewrite-custom-404/src/pages/404.astro

@@ -0,0 +1,13 @@
+---
+const interjected = Astro.locals.interjected;
+---
+
+<html>
+<head>
+	<title>Custom error</title>
+</head>
+<body>
+<h1>Custom error</h1>
+<p>{interjected}</p>
+</body>
+</html>

packages/astro/test/fixtures/rewrite-custom-404/src/pages/500.astro

@@ -0,0 +1,13 @@
+---
+const interjected = Astro.locals.interjected;
+---
+
+<html>
+<head>
+	<title>Custom error</title>
+</head>
+<body>
+<h1>Custom error</h1>
+<p>{interjected}</p>
+</body>
+</html>

packages/astro/test/fixtures/rewrite-custom-404/src/pages/about-2.astro

@@ -0,0 +1,3 @@
+---
+return Astro.rewrite("/500")
+---

packages/astro/test/rewrite.test.js

@@ -390,6 +390,40 @@ describe('Middleware', () => {
 	});
 });
 
+
+describe('Middleware with custom 404.astro and 500.astro', () => {
+	/** @type {import('./test-utils').Fixture} */
+	let fixture;
+	let devServer;
+
+	before(async () => {
+		fixture = await loadFixture({
+			root: './fixtures/rewrite-custom-404/',
+		});
+		devServer = await fixture.startDevServer();
+	});
+
+	after(async () => {
+		await devServer.stop();
+	});
+
+	it('The `next()` function should return a Response with status code 404', async () => {
+		const html = await fixture.fetch('/about').then((res) => res.text());
+		const $ = cheerioLoad(html);
+
+		assert.equal($('h1').text(), 'Custom error');
+		assert.equal($('p').text(), 'Interjected');
+	});
+
+	it('The `next()` function should return a Response with status code 500', async () => {
+		const html = await fixture.fetch('/about-2').then((res) => res.text());
+		const $ = cheerioLoad(html);
+
+		assert.equal($('h1').text(), 'Custom error');
+		assert.equal($('p').text(), 'Interjected');
+	});
+});
+
 describe('Runtime error, default 500', () => {
 	/** @type {import('./test-utils').Fixture} */
 	let fixture;

pnpm-lock.yaml

@@ -3523,7 +3523,7 @@ importers:
         specifier: workspace:*
         version: link:../../..
 
-  packages/astro/test/fixtures/rewrite-404-invalid:
+  packages/astro/test/fixtures/rewrite-custom-404:
     dependencies:
       astro:
         specifier: workspace:*