Render custom 500/404 error pages on the composable astro/hono middleware() path

.changeset/fix-composable-middleware-500.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes a bug where the advanced routing `astro/hono` / `astro/fetch` `middleware()` handler returned the host framework's default `Internal Server Error` response instead of rendering the custom `500.astro` page when middleware threw. Unmatched requests with a prerendered (or absent) custom 404 page now render the 404 error page instead of failing the same way. Errors surfaced through `next` (the host framework's downstream chain) still propagate to the host's own error handler.

packages/astro/src/core/fetch/index.ts

@@ -72,8 +72,11 @@ const middlewareInstances = new WeakMap<BaseApp<Pipeline>, AstroMiddleware>();
 
 /**
  * Runs Astro's middleware chain for the given state, calling `next` at
- * the bottom of the chain to produce the response. Lazily creates
- * the render context if needed.
+ * the bottom of the chain to produce the response. Lazily creates the
+ * render context if needed. Unmatched routes render the 404 error page;
+ * errors thrown by user middleware are logged and render the 500 error
+ * page; errors surfaced through `next` (the host framework's downstream
+ * chain) propagate to the host instead.
  */
 export function middleware(
 	state: FetchState,
@@ -85,7 +88,7 @@ export function middleware(
 		mw = new AstroMiddleware(app.pipeline);
 		middlewareInstances.set(app, mw);
 	}
-	return mw.handle(state, (s, _ctx) => next(s));
+	return mw.handleWithErrorFallback(app, state, (s, _ctx) => next(s));
 }
 
 const pagesHandlers = new WeakMap<BaseApp<Pipeline>, PagesHandler>();

packages/astro/src/core/middleware/astro-middleware.ts

@@ -1,7 +1,9 @@
 import type { FetchState } from '../fetch/fetch-state.js';
 import type { RewritePayload } from '../../types/public/common.js';
 import type { APIContext } from '../../types/public/context.js';
+import type { BaseApp } from '../app/base.js';
 import { type Pipeline, PipelineFeatures } from '../base-pipeline.js';
+import { ASTRO_ERROR_HEADER } from '../constants.js';
 import { attachCookiesToResponse } from '../cookies/index.js';
 import { applyRewriteToState } from '../rewrites/handler.js';
 import { callMiddleware } from './callMiddleware.js';
@@ -75,6 +77,61 @@ export class AstroMiddleware {
 		return response;
 	}
 
+	/**
+	 * Like `handle`, but mirrors the app-level error handling that
+	 * `AstroHandler` provides on the standard path, the same way
+	 * `PagesHandler.handleWithErrorFallback` does for `pages()`. When no
+	 * route matched it returns a 404 marked with `X-Astro-Error` for the
+	 * app's post-check; when Astro's own middleware chain throws it logs the
+	 * error and renders the custom `500.astro`.
+	 *
+	 * Errors surfaced through `renderRouteCallback` (the host framework's
+	 * `next`, e.g. host middleware mounted below `middleware()`) are
+	 * re-thrown instead, so the host's own error handling still runs rather
+	 * than being swallowed into Astro's 500 page. A sentinel tells the two
+	 * apart.
+	 *
+	 * Used by the composable `astro/fetch` `middleware()` entry point, where
+	 * there is no surrounding `AstroHandler` to supply this fallback.
+	 */
+	async handleWithErrorFallback(
+		app: BaseApp<Pipeline>,
+		state: FetchState,
+		renderRouteCallback: RenderRouteCallback,
+	): Promise<Response> {
+		// `FetchState` falls back to an SSR 404 route when nothing matches, so
+		// routeData is only missing when the custom 404 page is prerendered (or
+		// absent). Returning a marked 404 lets the app's `X-Astro-Error`
+		// post-check render the 404 page a level up, mirroring
+		// `PagesHandler.handleWithErrorFallback`; running user middleware here
+		// would throw on the missing route (no component to load).
+		if (!state.routeData) {
+			return new Response(null, { status: 404, headers: { [ASTRO_ERROR_HEADER]: 'true' } });
+		}
+		let nextError: unknown;
+		try {
+			return await this.handle(state, async (s, ctx) => {
+				try {
+					return await renderRouteCallback(s, ctx);
+				} catch (err) {
+					nextError = err;
+					throw err;
+				}
+			});
+		} catch (err: any) {
+			if (err === nextError) throw err;
+			// User middleware threw: log the stack and render the custom 500
+			// page, the same way `AstroHandler` does on the standard path.
+			app.logger.error(null, err.stack || err.message || String(err));
+			return app.renderError(state.request, {
+				...state.renderOptions,
+				status: 500,
+				error: err,
+				pathname: state.pathname,
+			});
+		}
+	}
+
 	#finalize(state: FetchState, response: Response): Response {
 		// LEGACY: we put cookies on the response object,
 		// where the adapter might be expecting to read it.

packages/astro/test/units/fetch/index.test.ts

@@ -259,6 +259,87 @@ describe('middleware()', () => {
 		assert.equal(response.headers.get('x-user-middleware'), 'true');
 		assert.equal(await response.text(), 'page');
 	});
+
+	it('renders the custom 500 page when user middleware throws', async () => {
+		const errorPage = createComponent((_result: any, _props: any, _slots: any) => {
+			return render`<h1>my custom 500</h1>`;
+		});
+		const app = createTestApp(
+			[createPage(simplePage, { route: '/' }), createPage(errorPage, { route: '/500' })],
+			{
+				middleware: async () => ({
+					onRequest: async () => {
+						throw new Error('boom from middleware');
+					},
+				}),
+			},
+		);
+		const request = stampApp(new Request('http://example.com/'), app);
+		const state = new FetchState(request);
+
+		const response = await middleware(state, async () => new Response('page'));
+
+		assert.equal(response.status, 500);
+		const text = await response.text();
+		assert.match(text, /<h1>my custom 500<\/h1>/);
+	});
+
+	it('re-throws errors from the next callback instead of rendering the 500 page', async () => {
+		// A throw from `next` originates downstream of Astro's middleware (the
+		// host framework's chain), so it must propagate to the host's own error
+		// handling rather than be swallowed into Astro's 500 page.
+		const errorPage = createComponent((_result: any, _props: any, _slots: any) => {
+			return render`<h1>my custom 500</h1>`;
+		});
+		const app = createTestApp([
+			createPage(simplePage, { route: '/' }),
+			createPage(errorPage, { route: '/500' }),
+		]);
+		const request = stampApp(new Request('http://example.com/'), app);
+		const state = new FetchState(request);
+
+		const downstreamError = new Error('boom from next');
+		await assert.rejects(
+			middleware(state, async () => {
+				throw downstreamError;
+			}),
+			(err: unknown) => err === downstreamError,
+		);
+	});
+
+	it('returns a marked 404 without running middleware when the custom 404 route is prerendered', async () => {
+		const notFoundPage = createComponent((_result: any, _props: any, _slots: any) => {
+			return render`<h1>Not Found</h1>`;
+		});
+		const app = createTestApp(
+			[
+				createPage(simplePage, { route: '/' }),
+				createPage(notFoundPage, { route: '/404', prerender: true }),
+			],
+			{
+				middleware: async () => ({
+					onRequest: async (_ctx: any, next: any) => {
+						const response = await next();
+						response.headers.set('x-user-middleware', 'true');
+						return response;
+					},
+				}),
+			},
+		);
+		const request = stampApp(new Request('http://example.com/does-not-exist'), app);
+		const state = new FetchState(request);
+
+		let nextCalled = false;
+		const response = await middleware(state, async () => {
+			nextCalled = true;
+			return new Response('page');
+		});
+
+		assert.equal(response.status, 404);
+		assert.equal(response.headers.get('X-Astro-Error'), 'true');
+		assert.equal(response.headers.get('x-user-middleware'), null, 'user middleware should not run');
+		assert.equal(nextCalled, false, 'next should not be called for an unmatched route');
+	});
 });
 
 // #endregion