fix(routing): middleware in dev (#12420)

Author: ematipico

.changeset/spicy-ties-matter.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes an issue where the dev server returns a 404 status code when a user middleware returns a valid `Response`.

packages/astro/src/core/constants.ts

@@ -31,6 +31,11 @@ export const REWRITE_DIRECTIVE_HEADER_KEY = 'X-Astro-Rewrite';
 
 export const REWRITE_DIRECTIVE_HEADER_VALUE = 'yes';
 
+/**
+ * This header is set by the no-op Astro middleware.
+ */
+export const NOOP_MIDDLEWARE_HEADER = 'X-Astro-Noop';
+
 /**
  * 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/middleware/noop-middleware.ts

@@ -1,3 +1,7 @@
 import type { MiddlewareHandler } from '../../@types/astro.js';
+import { NOOP_MIDDLEWARE_HEADER } from '../constants.js';
 
-export const NOOP_MIDDLEWARE_FN: MiddlewareHandler = (_, next) => next();
+export const NOOP_MIDDLEWARE_FN: MiddlewareHandler = (ctx, next) => {
+	ctx.request.headers.set(NOOP_MIDDLEWARE_HEADER, 'true');
+	return next();
+};

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

@@ -2,6 +2,7 @@ import type http from 'node:http';
 import type { ComponentInstance, ManifestData, RouteData } from '../@types/astro.js';
 import {
 	DEFAULT_404_COMPONENT,
+	NOOP_MIDDLEWARE_HEADER,
 	REROUTE_DIRECTIVE_HEADER,
 	REWRITE_DIRECTIVE_HEADER_KEY,
 	clientLocalsSymbol,
@@ -191,16 +192,11 @@ export async function handleRoute({
 
 	mod = preloadedComponent;
 
-	const isDefaultPrerendered404 =
-		matchedRoute.route.route === '/404' &&
-		matchedRoute.route.prerender &&
-		matchedRoute.route.component === DEFAULT_404_COMPONENT;
-
 	renderContext = await RenderContext.create({
 		locals,
 		pipeline,
 		pathname,
-		middleware: isDefaultPrerendered404 ? undefined : middleware,
+		middleware: isDefaultPrerendered404(matchedRoute.route) ? undefined : middleware,
 		request,
 		routeData: route,
 	});
@@ -213,10 +209,15 @@ export async function handleRoute({
 		response = await renderContext.render(mod);
 		isReroute = response.headers.has(REROUTE_DIRECTIVE_HEADER);
 		isRewrite = response.headers.has(REWRITE_DIRECTIVE_HEADER_KEY);
+		const statusCodedMatched = getStatusByMatchedRoute(matchedRoute);
 		statusCode = isRewrite
 			? // Ignore `matchedRoute` status for rewrites
 				response.status
-			: (getStatusByMatchedRoute(matchedRoute) ?? response.status);
+			: // Our internal noop middleware sets a particular header. If the header isn't present, it means that the user have
+				// their own middleware, so we need to return what the user returns.
+				!response.headers.has(NOOP_MIDDLEWARE_HEADER) && !isReroute
+				? response.status
+				: (statusCodedMatched ?? response.status);
 	} catch (err: any) {
 		const custom500 = getCustom500Route(manifestData);
 		if (!custom500) {
@@ -308,3 +309,7 @@ function getStatusByMatchedRoute(matchedRoute?: MatchedRoute) {
 	if (matchedRoute?.route.route === '/500') return 500;
 	return undefined;
 }
+
+function isDefaultPrerendered404(route: RouteData) {
+	return route.route === '/404' && route.prerender && route.component === DEFAULT_404_COMPONENT;
+}

packages/astro/test/fixtures/middleware space/src/middleware.js

@@ -74,4 +74,13 @@ const third = defineMiddleware(async (context, next) => {
 	return next();
 });
 
-export const onRequest = sequence(first, second, third);
+const fourth = defineMiddleware((context, next) => {
+	if (context.request.url.includes('/no-route-but-200')) {
+		return new Response("It's OK!", {
+			status: 200
+		});
+	}
+	return next()
+})
+
+export const onRequest = sequence(first, second, third, fourth);

packages/astro/test/middleware.test.js

@@ -70,6 +70,13 @@ describe('Middleware in DEV mode', () => {
 		assert.equal($('title').html(), 'MiddlewareNoDataOrNextCalled');
 	});
 
+	it('should return 200 if the middleware returns a 200 Response', async () => {
+		const response = await fixture.fetch('/no-route-but-200');
+		assert.equal(response.status, 200);
+		const html = await response.text();
+		assert.match(html, /It's OK!/);
+	});
+
 	it('should allow setting cookies', async () => {
 		const res = await fixture.fetch('/');
 		assert.equal(res.headers.get('set-cookie'), 'foo=bar');
@@ -239,6 +246,14 @@ describe('Middleware API in PROD mode, SSR', () => {
 		assert.notEqual($('title').html(), 'MiddlewareNoDataReturned');
 	});
 
+	it('should return 200 if the middleware returns a 200 Response', async () => {
+		const request = new Request('http://example.com/no-route-but-200');
+		const response = await app.render(request);
+		assert.equal(response.status, 200);
+		const html = await response.text();
+		assert.match(html, /It's OK!/);
+	});
+
 	it('should correctly work for API endpoints that return a Response object', async () => {
 		const request = new Request('http://example.com/api/endpoint');
 		const response = await app.render(request);