fix: avoid response rewrite inside the dev server (#11477)

* fix: avoid response rewrite inside the dev server

* breakdown logic of reroute and rewrite

.changeset/chilled-impalas-dance.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes an issue where the development server was emitting a 404 status code when the user uses a rewrite that emits a 200 status code.

packages/astro/src/core/constants.ts

@@ -22,6 +22,14 @@ export const ASTRO_VERSION = process.env.PACKAGE_VERSION ?? 'development';
  */
 export const REROUTE_DIRECTIVE_HEADER = 'X-Astro-Reroute';
 
+/**
+ * Header and value that are attached to a Response object when a **user rewrite** occurs.
+ *
+ * This metadata is used to determine the origin of a Response. If a rewrite has occurred, it should be prioritised over other logic.
+ */
+export const REWRITE_DIRECTIVE_HEADER_KEY = 'X-Astro-Rewrite';
+export const REWRITE_DIRECTIVE_HEADER_VALUE = 'yes';
+
 /**
  * The name for the header used to help i18n middleware, which only needs to act on "page" and "fallback" route types.
  */

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

@@ -25,6 +25,8 @@ import {
 	clientAddressSymbol,
 	clientLocalsSymbol,
 	responseSentSymbol,
+	REWRITE_DIRECTIVE_HEADER_KEY,
+	REWRITE_DIRECTIVE_HEADER_VALUE,
 } from './constants.js';
 import { AstroCookies, attachCookiesToResponse } from './cookies/index.js';
 import { getCookiesFromResponse } from './cookies/response.js';
@@ -184,13 +186,12 @@ export class RenderContext {
 					// Signal to the i18n middleware to maybe act on this response
 					response.headers.set(ROUTE_TYPE_HEADER, 'page');
 					// Signal to the error-page-rerouting infra to let this response pass through to avoid loops
-					if (
-						this.routeData.route === '/404' ||
-						this.routeData.route === '/500' ||
-						this.isRewriting
-					) {
+					if (this.routeData.route === '/404' || this.routeData.route === '/500') {
 						response.headers.set(REROUTE_DIRECTIVE_HEADER, 'no');
 					}
+					if (this.isRewriting) {
+						response.headers.set(REWRITE_DIRECTIVE_HEADER_KEY, REWRITE_DIRECTIVE_HEADER_VALUE);
+					}
 					break;
 				}
 				case 'fallback': {
@@ -381,6 +382,7 @@ export class RenderContext {
 	}
 
 	#astroPagePartial?: Omit<AstroGlobal, 'props' | 'self' | 'slots'>;
+
 	/**
 	 * The Astro global is sourced in 3 different phases:
 	 * - **Static**: `.generator` and `.glob` is printed by the compiler, instantiated once per process per astro file
@@ -512,6 +514,7 @@ export class RenderContext {
 	 * So, it is computed and saved here on creation of the first APIContext and reused for later ones.
 	 */
 	#currentLocale: APIContext['currentLocale'];
+
 	computeCurrentLocale() {
 		const {
 			url,
@@ -537,6 +540,7 @@ export class RenderContext {
 	}
 
 	#preferredLocale: APIContext['preferredLocale'];
+
 	computePreferredLocale() {
 		const {
 			pipeline: { i18n },
@@ -547,6 +551,7 @@ export class RenderContext {
 	}
 
 	#preferredLocaleList: APIContext['preferredLocaleList'];
+
 	computePreferredLocaleList() {
 		const {
 			pipeline: { i18n },

packages/astro/src/vite-plugin-astro-server/route.ts

@@ -4,6 +4,7 @@ import {
 	DEFAULT_404_COMPONENT,
 	REROUTE_DIRECTIVE_HEADER,
 	clientLocalsSymbol,
+	REWRITE_DIRECTIVE_HEADER_KEY,
 } from '../core/constants.js';
 import { AstroErrorData, isAstroError } from '../core/errors/index.js';
 import { req } from '../core/messages.js';
@@ -218,8 +219,12 @@ export async function handleRoute({
 	});
 
 	let response;
+	let isReroute = false;
+	let isRewrite = false;
 	try {
 		response = await renderContext.render(mod);
+		isReroute = response.headers.has(REROUTE_DIRECTIVE_HEADER);
+		isRewrite = response.headers.has(REWRITE_DIRECTIVE_HEADER_KEY);
 	} catch (err: any) {
 		const custom500 = getCustom500Route(manifestData);
 		if (!custom500) {
@@ -264,14 +269,25 @@ export async function handleRoute({
 	}
 
 	// We remove the internally-used header before we send the response to the user agent.
-	if (response.headers.has(REROUTE_DIRECTIVE_HEADER)) {
+	if (isReroute) {
+		response.headers.delete(REROUTE_DIRECTIVE_HEADER);
+	}
+	if (isRewrite) {
 		response.headers.delete(REROUTE_DIRECTIVE_HEADER);
 	}
 
 	if (route.type === 'endpoint') {
 		await writeWebResponse(incomingResponse, response);
 		return;
 	}
+
+	// This check is important in case of rewrites.
+	// A route can start with a 404 code, then the rewrite kicks in and can return a 200 status code
+	if (isRewrite) {
+		await writeSSRResult(request, response, incomingResponse);
+		return;
+	}
+
 	// We are in a recursion, and it's possible that this function is called itself with a status code
 	// By default, the status code passed via parameters is computed by the matched route.
 	//

packages/astro/test/rewrite.test.js

@@ -520,6 +520,34 @@ describe('Runtime error in SSR, custom 500', () => {
 	});
 });
 
+describe('Runtime error in dev, custom 500', () => {
+	/** @type {import('./test-utils').Fixture} */
+	let fixture;
+	let devServer;
+
+	before(async () => {
+		fixture = await loadFixture({
+			root: './fixtures/rewrite-i18n-manual-routing/',
+		});
+
+		devServer = await fixture.startDevServer();
+	});
+
+	after(async () => {
+		await devServer.stop();
+	});
+
+	it('should return a status 200 when rewriting from the middleware to the homepage', async () => {
+		const response = await fixture.fetch('/reroute');
+		assert.equal(response.status, 200);
+		const html = await response.text();
+
+		const $ = cheerioLoad(html);
+
+		assert.equal($('h1').text(), 'Expected http status of index page is 200');
+	});
+});
+
 describe('Runtime error in SSR, custom 500', () => {
 	/** @type {import('./test-utils').Fixture} */
 	let fixture;