aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Emanuele Stoppa <my.burning@gmail.com> 2024-11-13 13:34:35 +0000
committerGravatar GitHub <noreply@github.com> 2024-11-13 13:34:35 +0000
commitacac0af53466f8a381ccdac29ed2ad735d7b4e79 (patch)
treee8dffdc6ceee3a97d6bf5ce06196f8783368430f
parente723e9e8ea21531b5200b22a14aca333b3a0580d (diff)
downloadastro-acac0af53466f8a381ccdac29ed2ad735d7b4e79.tar.gz
astro-acac0af53466f8a381ccdac29ed2ad735d7b4e79.tar.zst
astro-acac0af53466f8a381ccdac29ed2ad735d7b4e79.zip
fix(routing): middleware in dev (#12420)
-rw-r--r--.changeset/spicy-ties-matter.md5
-rw-r--r--packages/astro/src/core/constants.ts5
-rw-r--r--packages/astro/src/core/middleware/noop-middleware.ts6
-rw-r--r--packages/astro/src/vite-plugin-astro-server/route.ts19
-rw-r--r--packages/astro/test/fixtures/middleware space/src/middleware.js11
-rw-r--r--packages/astro/test/middleware.test.js15
6 files changed, 52 insertions, 9 deletions
diff --git a/.changeset/spicy-ties-matter.md b/.changeset/spicy-ties-matter.md
new file mode 100644
index 000000000..48173596a
--- /dev/null
+++ b/.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`.
diff --git a/packages/astro/src/core/constants.ts b/packages/astro/src/core/constants.ts
index 6183e1557..dc09a1f69 100644
--- a/packages/astro/src/core/constants.ts
+++ b/packages/astro/src/core/constants.ts
@@ -32,6 +32,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.
*/
export const ROUTE_TYPE_HEADER = 'X-Astro-Route-Type';
diff --git a/packages/astro/src/core/middleware/noop-middleware.ts b/packages/astro/src/core/middleware/noop-middleware.ts
index bf5f10d89..b141285f6 100644
--- a/packages/astro/src/core/middleware/noop-middleware.ts
+++ b/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();
+};
diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts
index 9f6d434b5..8de52d158 100644
--- a/packages/astro/src/vite-plugin-astro-server/route.ts
+++ b/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;
+}
diff --git a/packages/astro/test/fixtures/middleware space/src/middleware.js b/packages/astro/test/fixtures/middleware space/src/middleware.js
index 631099455..185fc3116 100644
--- a/packages/astro/test/fixtures/middleware space/src/middleware.js
+++ b/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);
diff --git a/packages/astro/test/middleware.test.js b/packages/astro/test/middleware.test.js
index 612937d80..2bcfe8d57 100644
--- a/packages/astro/test/middleware.test.js
+++ b/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);