Allow custom 404 route to handle API route missing methods (#4594)

* Properly allow file uploads in the dev server

* Allow custom 404 route to handle API route missing methods

* Add a changeset

* what was i thinking

* Pass through the pathname

* Move the try/catch out and into handleRequest

* await the result of handleRoute

.changeset/twelve-days-build.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Allow custom 404 route to handle API route missing methods

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

@@ -202,6 +202,13 @@ export class App {
 		});
 
 		if (result.type === 'response') {
+			if(result.response.headers.get('X-Astro-Response') === 'Not-Found') {
+				const fourOhFourRequest = new Request(new URL('/404', request.url));
+				const fourOhFourRouteData = this.match(fourOhFourRequest);
+				if(fourOhFourRouteData) {
+					return this.render(fourOhFourRequest, fourOhFourRouteData);
+				}
+			}
 			return result.response;
 		} else {
 			const body = result.body;

packages/astro/src/runtime/server/endpoint.ts

@@ -18,13 +18,24 @@ function getHandlerFromModule(mod: EndpointHandler, method: string) {
 }
 
 /** Renders an endpoint request to completion, returning the body. */
-export async function renderEndpoint(mod: EndpointHandler, request: Request, params: Params) {
+export async function renderEndpoint(
+	mod: EndpointHandler,
+	request: Request,
+	params: Params
+) {
 	const chosenMethod = request.method?.toLowerCase();
 	const handler = getHandlerFromModule(mod, chosenMethod);
 	if (!handler || typeof handler !== 'function') {
-		throw new Error(
-			`Endpoint handler not found! Expected an exported function for "${chosenMethod}"`
-		);
+		// No handler found, so this should be a 404. Using a custom header
+		// to signal to the renderer that this is an internal 404 that should
+		// be handled by a custom 404 route if possible.
+		let response = new Response(null, {
+			status: 404,
+			headers: {
+				'X-Astro-Response': 'Not-Found'
+			},
+		});
+		return response;
 	}
 
 	if (handler.length > 1) {

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

@@ -1,7 +1,7 @@
 import type http from 'http';
 import mime from 'mime';
 import type * as vite from 'vite';
-import type { AstroConfig, ManifestData } from '../@types/astro';
+import type { AstroConfig, ManifestData, SSRManifest } from '../@types/astro';
 import type { SSROptions } from '../core/render/dev/index';
 
 import { Readable } from 'stream';
@@ -28,13 +28,8 @@ interface AstroPluginOptions {
 	logging: LogOptions;
 }
 
-function truncateString(str: string, n: number) {
-	if (str.length > n) {
-		return str.substring(0, n) + '…';
-	} else {
-		return str;
-	}
-}
+type AsyncReturnType<T extends (...args: any) => Promise<any>> =
+    T extends (...args: any) => Promise<infer R> ? R : any
 
 function writeHtmlResponse(res: http.ServerResponse, statusCode: number, html: string) {
 	res.writeHead(statusCode, {
@@ -180,63 +175,14 @@ export function baseMiddleware(
 	};
 }
 
-/** The main logic to route dev server requests to pages in Astro. */
-async function handleRequest(
+async function matchRoute(
+	pathname: string,
 	routeCache: RouteCache,
 	viteServer: vite.ViteDevServer,
 	logging: LogOptions,
 	manifest: ManifestData,
 	config: AstroConfig,
-	req: http.IncomingMessage,
-	res: http.ServerResponse
 ) {
-	const reqStart = performance.now();
-	const origin = `${viteServer.config.server.https ? 'https' : 'http'}://${req.headers.host}`;
-	const buildingToSSR = config.output === 'server';
-	// Ignore `.html` extensions and `index.html` in request URLS to ensure that
-	// routing behavior matches production builds. This supports both file and directory
-	// build formats, and is necessary based on how the manifest tracks build targets.
-	const url = new URL(origin + req.url?.replace(/(index)?\.html$/, ''));
-	const pathname = decodeURI(url.pathname);
-
-	// Add config.base back to url before passing it to SSR
-	url.pathname = config.base.substring(0, config.base.length - 1) + url.pathname;
-
-	// HACK! @astrojs/image uses query params for the injected route in `dev`
-	if (!buildingToSSR && pathname !== '/_image') {
-		// Prevent user from depending on search params when not doing SSR.
-		// NOTE: Create an array copy here because deleting-while-iterating
-		// creates bugs where not all search params are removed.
-		const allSearchParams = Array.from(url.searchParams);
-		for (const [key] of allSearchParams) {
-			url.searchParams.delete(key);
-		}
-	}
-
-	let body: ArrayBuffer | undefined = undefined;
-	if (!(req.method === 'GET' || req.method === 'HEAD')) {
-		let bytes: Uint8Array[] = [];
-		await new Promise((resolve) => {
-			req.on('data', (part) => {
-				bytes.push(part);
-			});
-			req.on('end', resolve);
-		});
-		body = Buffer.concat(bytes);
-	}
-
-	// Headers are only available when using SSR.
-	const request = createRequest({
-		url,
-		headers: buildingToSSR ? req.headers : new Headers(),
-		method: req.method,
-		body,
-		logging,
-		ssr: buildingToSSR,
-		clientAddress: buildingToSSR ? req.socket.remoteAddress : undefined,
-	});
-
-	async function matchRoute() {
 	const matches = matchAllRoutes(pathname, manifest);
 
 	for await (const maybeRoute of matches) {
@@ -289,18 +235,118 @@ async function handleRequest(
 	}
 
 	return undefined;
+}
+
+/** The main logic to route dev server requests to pages in Astro. */
+async function handleRequest(
+	routeCache: RouteCache,
+	viteServer: vite.ViteDevServer,
+	logging: LogOptions,
+	manifest: ManifestData,
+	config: AstroConfig,
+	req: http.IncomingMessage,
+	res: http.ServerResponse
+) {
+	const origin = `${viteServer.config.server.https ? 'https' : 'http'}://${req.headers.host}`;
+	const buildingToSSR = config.output === 'server';
+	// Ignore `.html` extensions and `index.html` in request URLS to ensure that
+	// routing behavior matches production builds. This supports both file and directory
+	// build formats, and is necessary based on how the manifest tracks build targets.
+	const url = new URL(origin + req.url?.replace(/(index)?\.html$/, ''));
+	const pathname = decodeURI(url.pathname);
+
+	// Add config.base back to url before passing it to SSR
+	url.pathname = config.base.substring(0, config.base.length - 1) + url.pathname;
+
+	// HACK! @astrojs/image uses query params for the injected route in `dev`
+	if (!buildingToSSR && pathname !== '/_image') {
+		// Prevent user from depending on search params when not doing SSR.
+		// NOTE: Create an array copy here because deleting-while-iterating
+		// creates bugs where not all search params are removed.
+		const allSearchParams = Array.from(url.searchParams);
+		for (const [key] of allSearchParams) {
+			url.searchParams.delete(key);
+		}
+	}
+
+	let body: ArrayBuffer | undefined = undefined;
+	if (!(req.method === 'GET' || req.method === 'HEAD')) {
+		let bytes: Uint8Array[] = [];
+		await new Promise((resolve) => {
+			req.on('data', part => {
+				bytes.push(part);
+			});
+			req.on('end', resolve);
+		});
+		body = Buffer.concat(bytes);
 	}
 
 	let filePath: URL | undefined;
 	try {
-		const matchedRoute = await matchRoute();
+		const matchedRoute = await matchRoute(
+			pathname,
+			routeCache,
+			viteServer,
+			logging,
+			manifest,
+			config
+		);
+		filePath = matchedRoute?.filePath;
 	
+		return await handleRoute(
+			matchedRoute,
+			url,
+			pathname,
+			body,
+			origin,
+			routeCache,
+			viteServer,
+			manifest,
+			logging,
+			config,
+			req,
+			res
+		);
+	} catch(_err) {
+		const err = fixViteErrorMessage(_err, viteServer, filePath);
+		const errorWithMetadata = collectErrorMetadata(err);
+		error(logging, null, msg.formatErrorMessage(errorWithMetadata));
+		handle500Response(viteServer, origin, req, res, errorWithMetadata);
+	}
+}
+
+async function handleRoute(
+	matchedRoute: AsyncReturnType<typeof matchRoute>,
+	url: URL,
+	pathname: string,
+	body: ArrayBuffer | undefined,
+	origin: string,
+	routeCache: RouteCache,
+	viteServer: vite.ViteDevServer,
+	manifest: ManifestData,
+	logging: LogOptions,
+	config: AstroConfig,
+	req: http.IncomingMessage,
+	res: http.ServerResponse
+): Promise<void> {
 	if (!matchedRoute) {
 		return handle404Response(origin, config, req, res);
 	}
 
+	const filePath: URL | undefined = matchedRoute.filePath;
 	const { route, preloadedComponent, mod } = matchedRoute;
-		filePath = matchedRoute.filePath;
+	const buildingToSSR = config.output === 'server';
+
+	// Headers are only available when using SSR.
+	const request = createRequest({
+		url,
+		headers: buildingToSSR ? req.headers : new Headers(),
+		method: req.method,
+		body,
+		logging,
+		ssr: buildingToSSR,
+		clientAddress: buildingToSSR ? req.socket.remoteAddress : undefined,
+	});
 
 	// attempt to get static paths
 	// if this fails, we have a bad URL match!
@@ -330,6 +376,30 @@ async function handleRequest(
 	if (route.type === 'endpoint') {
 		const result = await callEndpoint(options);
 		if (result.type === 'response') {
+			if(result.response.headers.get('X-Astro-Response') === 'Not-Found') {
+				const fourOhFourRoute = await matchRoute(
+					'/404',
+					routeCache,
+					viteServer,
+					logging,
+					manifest,
+					config
+				);
+				return handleRoute(
+					fourOhFourRoute,
+					new URL('/404', url),
+					'/404',
+					body,
+					origin,
+					routeCache,
+					viteServer,
+					manifest,
+					logging,
+					config,
+					req,
+					res
+				);
+			}
 			await writeWebResponse(res, result.response);
 		} else {
 			let contentType = 'text/plain';
@@ -348,12 +418,6 @@ async function handleRequest(
 		const result = await ssr(preloadedComponent, options);
 		return await writeSSRResult(result, res);
 	}
-	} catch (_err) {
-		const err = fixViteErrorMessage(_err, viteServer, filePath);
-		const errorWithMetadata = collectErrorMetadata(err);
-		error(logging, null, msg.formatErrorMessage(errorWithMetadata));
-		handle500Response(viteServer, origin, req, res, errorWithMetadata);
-	}
 }
 
 export default function createPlugin({ config, logging }: AstroPluginOptions): vite.Plugin {

packages/astro/test/dev-routing.test.js

@@ -285,6 +285,10 @@ describe('Development Routing', () => {
 			devServer = await fixture.startDevServer();
 		});
 
+		after(async () => {
+			await devServer.stop();
+		});
+
 		it('200 when loading /index.html', async () => {
 			const response = await fixture.fetch('/index.html');
 			expect(response.status).to.equal(200);

packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/api/route.js

@@ -0,0 +1,6 @@
+
+export function post() {
+	return {
+		body: JSON.stringify({ ok: true })
+	};
+}

packages/astro/test/ssr-404-500-pages.test.js

@@ -13,6 +13,31 @@ describe('404 and 500 pages', () => {
 			output: 'server',
 			adapter: testAdapter(),
 		});
+	});
+
+	describe('Development', () => {
+		/** @type {import('./test-utils').DevServer} */
+		let devServer;
+		before(async () => {
+			devServer = await fixture.startDevServer();
+		});
+
+		after(async () => {
+			await devServer.stop();
+		});
+
+		it('Returns 404 when hitting an API route with the wrong method', async () => {
+			let res = await fixture.fetch('/api/route', {
+				method: 'PUT'
+			});
+			let html = await res.text();
+			let $ = cheerio.load(html);
+			expect($('h1').text()).to.equal(`Something went horribly wrong!`);
+		});
+	});
+	
+	describe('Production', () => {
+		before(async () => {
 			await fixture.build({});
 		});
 
@@ -46,4 +71,17 @@ describe('404 and 500 pages', () => {
 			const $ = cheerio.load(html);
 			expect($('h1').text()).to.equal('This is an error page');
 		});
+
+		it('Returns 404 when hitting an API route with the wrong method', async () => {
+			const app = await fixture.loadTestAdapterApp();
+			const request = new Request('http://example.com/api/route', {
+				method: 'PUT'
+			});
+			const response = await app.render(request);
+			expect(response.status).to.equal(404);
+			const html = await response.text();
+			const $ = cheerio.load(html);
+			expect($('h1').text()).to.equal(`Something went horribly wrong!`);
+		});
+	});
 });