diff --git a/.changeset/spicy-ties-matter.md b/.changeset/spicy-ties-matter.md new file mode 100644 index 000000000000..48173596af10 --- /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 6183e155721c..dc09a1f69474 100644 --- a/packages/astro/src/core/constants.ts +++ b/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. */ diff --git a/packages/astro/src/core/middleware/noop-middleware.ts b/packages/astro/src/core/middleware/noop-middleware.ts index bf5f10d89054..b141285f699c 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 9f6d434b55e8..8de52d158bb0 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 6310994554ea..185fc31168e7 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 612937d80433..2bcfe8d57d41 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);