Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/soft-things-deny.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes a bug where the custom 404 (or 500) page was not rendered when a middleware rewrite targeted a route that returned an empty 404/500 response, and a blank page was returned instead
24 changes: 24 additions & 0 deletions packages/astro/src/core/errors/default-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { provideSession } from '../session/handler.js';
import { validateHost } from '../app/validate-headers.js';
import { getErrorRoutePath } from '../../i18n/error-routes.js';
import { getOutputFilename } from '../output-filename.js';
import { REROUTABLE_STATUS_CODES } from '../constants.js';
import type { ErrorHandler } from './handler.js';

type ErrorPagePath =
Expand Down Expand Up @@ -127,6 +128,29 @@ export class DefaultErrorHandler implements ErrorHandler {
errorState,
this.#pagesHandler.handle.bind(this.#pagesHandler),
);
// A middleware rewrite (`ctx.rewrite()` / `next(payload)`) issued while
// rendering the error page swaps the state's routeData away from the
// error route, so the rewrite target renders instead of 404/500.astro.
// If that hijacked render produced another empty reroutable error
// response, we'd return a blank page — retry rendering the error page
// without middleware instead (same fallback used when middleware throws).
// A rewrite that produced a real body is left untouched, so middleware
// that intentionally rewrites error renders keeps working.
if (
skipMiddleware === false &&
errorState.routeData !== errorRouteData &&
response.body === null &&
REROUTABLE_STATUS_CODES.includes(response.status)
) {
return this.renderError(request, {
...resolvedRenderOptions,
status,
error,
response: originalResponse,
skipMiddleware: true,
pathname: resolvedPathname,
});
}
const newResponse = mergeResponses(response, originalResponse);
prepareResponse(newResponse, resolvedRenderOptions);
return newResponse;
Expand Down
20 changes: 20 additions & 0 deletions packages/astro/src/core/errors/dev-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { RouteData } from '../../types/public/index.js';
import { AstroMiddleware } from '../middleware/astro-middleware.js';
import { PagesHandler } from '../pages/handler.js';
import { getCustom404Route, getCustom500Route } from '../routing/helpers.js';
import { REROUTABLE_STATUS_CODES } from '../constants.js';
import { type AstroError, isAstroError } from './index.js';
import { MiddlewareNoDataOrNextCalled, MiddlewareNotAResponse } from './errors-data.js';
import type { ErrorHandler } from './handler.js';
Expand Down Expand Up @@ -77,6 +78,25 @@ export class DevErrorHandler implements ErrorHandler {
this.#pagesHandler.handle.bind(this.#pagesHandler),
);

// A middleware rewrite issued while rendering the error page swaps the
// state's routeData away from the error route. If that hijacked render
// produced another empty reroutable error response, retry rendering the
// error page without middleware (same fallback used when middleware throws).
if (
skipMiddleware === false &&
errorState.routeData !== routeData &&
response.body === null &&
REROUTABLE_STATUS_CODES.includes(response.status)
) {
return this.renderError(request, {
...resolvedRenderOptions,
status,
error,
skipMiddleware: true,
pathname: resolvedPathname,
});
}

if (error) {
// Log useful information that the custom 500 page may not display unlike the default error overlay
app.logger.error('router', (error as AstroError).stack || (error as AstroError).message);
Expand Down
65 changes: 65 additions & 0 deletions packages/astro/test/units/routing/rewrite-app.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,71 @@ describe('Rewrites via App - body-used rewrite returns 500', () => {
});
});

describe('Rewrites via App - middleware rewrite to a route returning an empty 404 (#17281)', () => {
const custom404 = createComponent(() => render`<h1>Custom 404</h1>`);
const empty404Page = createComponent(() => new Response(null, { status: 404 }));

const middleware = async (context: APIContext, next: MiddlewareNext) => {
if (context.url.pathname === '/rewrite') {
return context.rewrite('/unexisting-page');
}
return next();
};

const app = createTestApp(
[
createPage(custom404, { route: '/404', component: '404.astro' }),
createPage(empty404Page, {
route: '/[...slug]',
segments: [[spreadPart('...slug')]],
pathname: undefined,
}),
],
{ middleware: () => ({ onRequest: middleware }) },
);

it('renders the custom 404 page when middleware rewrites to a route that returns an empty 404', async () => {
const res = await app.render(new Request('http://example.com/rewrite'));
assert.equal(res.status, 404);
const $ = cheerio.load(await res.text());
assert.equal($('h1').text(), 'Custom 404');
});

it('renders the custom 404 page when the 404-returning route is hit directly', async () => {
const res = await app.render(new Request('http://example.com/unexisting-page'));
assert.equal(res.status, 404);
const $ = cheerio.load(await res.text());
assert.equal($('h1').text(), 'Custom 404');
});
});

describe('Rewrites via App - middleware rewrites to a real page after next() returns 404', () => {
const empty404Page = createComponent(() => new Response(null, { status: 404 }));

const middleware = async (context: APIContext, next: MiddlewareNext) => {
const response = await next();
if (response.status === 404 && context.url.pathname === '/soft-404') {
return context.rewrite('/');
}
return response;
};

const app = createTestApp(
[
createPage(indexPage, { route: '/', isIndex: true }),
createPage(empty404Page, { route: '/soft-404' }),
],
{ middleware: () => ({ onRequest: middleware }) },
);

it('renders the rewrite target when middleware rewrites on a 404 response', async () => {
const res = await app.render(new Request('http://example.com/soft-404'));
assert.equal(res.status, 200);
const $ = cheerio.load(await res.text());
assert.equal($('h1').text(), 'Index');
});
});

describe('Rewrites via App - routePattern updated after sequence rewrite', () => {
const patternPage = createComponent((result: any, props: any, slots: any) => {
const Astro = result.createAstro(props, slots);
Expand Down
Loading