diff options
author | 2024-06-24 13:21:27 +0100 | |
---|---|---|
committer | 2024-06-24 13:21:27 +0100 | |
commit | 44c61ddfd85f1c23f8cec8caeaa5e25897121996 (patch) | |
tree | ff339c744af08b5c9b1a678c2f7acc364caf5a36 | |
parent | edd35d3c6996e878c0011d811014bdcd1fa7c4c4 (diff) | |
download | astro-44c61ddfd85f1c23f8cec8caeaa5e25897121996.tar.gz astro-44c61ddfd85f1c23f8cec8caeaa5e25897121996.tar.zst astro-44c61ddfd85f1c23f8cec8caeaa5e25897121996.zip |
fix(routing): return correct status code for `500.astro` and `404.astro` (#11308)
* fix(routing): return correct status code for `500.astro` and `404.astro`
* changeset
* fix regression
* use `route` instead
14 files changed, 100 insertions, 10 deletions
diff --git a/.changeset/curvy-emus-mate.md b/.changeset/curvy-emus-mate.md new file mode 100644 index 000000000..3006f1e8c --- /dev/null +++ b/.changeset/curvy-emus-mate.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes an issue where custom `404.astro` and `500.astro` were not returning the correct status code when rendered inside a rewriting cycle. diff --git a/packages/astro/src/core/cookies/response.ts b/packages/astro/src/core/cookies/response.ts index 26f032fdd..288bb3e93 100644 --- a/packages/astro/src/core/cookies/response.ts +++ b/packages/astro/src/core/cookies/response.ts @@ -10,7 +10,7 @@ export function responseHasCookies(response: Response): boolean { return Reflect.has(response, astroCookiesSymbol); } -export function getFromResponse(response: Response): AstroCookies | undefined { +export function getCookiesFromResponse(response: Response): AstroCookies | undefined { let cookies = Reflect.get(response, astroCookiesSymbol); if (cookies != null) { return cookies as AstroCookies; @@ -20,7 +20,7 @@ export function getFromResponse(response: Response): AstroCookies | undefined { } export function* getSetCookiesFromResponse(response: Response): Generator<string, string[]> { - const cookies = getFromResponse(response); + const cookies = getCookiesFromResponse(response); if (!cookies) { return []; } diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts index 413922042..e6ac42364 100644 --- a/packages/astro/src/core/render-context.ts +++ b/packages/astro/src/core/render-context.ts @@ -27,7 +27,7 @@ import { responseSentSymbol, } from './constants.js'; import { AstroCookies, attachCookiesToResponse } from './cookies/index.js'; -import { getFromResponse } from './cookies/response.js'; +import { getCookiesFromResponse } from './cookies/response.js'; import { AstroError, AstroErrorData } from './errors/index.js'; import { callMiddleware } from './middleware/callMiddleware.js'; import { sequence } from './middleware/index.js'; @@ -136,6 +136,7 @@ export class RenderContext { const lastNext = async (ctx: APIContext, payload?: RewritePayload) => { if (payload) { if (this.pipeline.manifest.rewritingEnabled) { + pipeline.logger.debug('router', 'Called rewriting to:', payload); // we intentionally let the error bubble up const [routeData, component] = await pipeline.tryRewrite( payload, @@ -197,7 +198,7 @@ export class RenderContext { } // We need to merge the cookies from the response back into this.cookies // because they may need to be passed along from a rewrite. - const responseCookies = getFromResponse(response); + const responseCookies = getCookiesFromResponse(response); if (responseCookies) { cookies.merge(responseCookies); } diff --git a/packages/astro/src/runtime/server/render/page.ts b/packages/astro/src/runtime/server/render/page.ts index cc6461ce6..9d84fdd5a 100644 --- a/packages/astro/src/runtime/server/render/page.ts +++ b/packages/astro/src/runtime/server/render/page.ts @@ -1,4 +1,4 @@ -import type { RouteData, SSRResult } from '../../../@types/astro.js'; +import type {AstroConfig, RouteData, SSRResult} from '../../../@types/astro.js'; import { type NonAstroPageComponent, renderComponentToString } from './component.js'; import type { AstroComponentFactory } from './index.js'; @@ -85,6 +85,17 @@ export async function renderPage( if (route?.component.endsWith('.md')) { headers.set('Content-Type', 'text/html; charset=utf-8'); } - const response = new Response(body, { ...init, headers }); - return response; + let status = init.status; + // Custom 404.astro and 500.astro are particular routes that must return a fixed status code + if (route?.route === "/404") { + status = 404 + } else if (route?.route === "/500") { + status = 500 + } + if (status) { + return new Response(body, { ...init, headers, status }); + } else { + return new Response(body, { ...init, headers }); + + } } diff --git a/packages/astro/test/custom-404-implicit-rerouting.test.js b/packages/astro/test/custom-404-implicit-rerouting.test.js index 7e2ed30c8..06eea21d9 100644 --- a/packages/astro/test/custom-404-implicit-rerouting.test.js +++ b/packages/astro/test/custom-404-implicit-rerouting.test.js @@ -58,7 +58,7 @@ for (const caseNumber of [1, 2, 3, 4, 5]) { }); // IMPORTANT: never skip - it('prod server stays responsive', { timeout: 1000 }, async () => { + it('prod server stays responsive for case number ' + caseNumber, { timeout: 1000 }, async () => { const response = await app.render(new Request('https://example.com/alvsibdlvjks')); assert.equal(response.status, 404); }); diff --git a/packages/astro/test/fixtures/rewrite-404-invalid/astro.config.mjs b/packages/astro/test/fixtures/rewrite-custom-404/astro.config.mjs index af13ef19b..af13ef19b 100644 --- a/packages/astro/test/fixtures/rewrite-404-invalid/astro.config.mjs +++ b/packages/astro/test/fixtures/rewrite-custom-404/astro.config.mjs diff --git a/packages/astro/test/fixtures/rewrite-404-invalid/package.json b/packages/astro/test/fixtures/rewrite-custom-404/package.json index 994743ef2..128321457 100644 --- a/packages/astro/test/fixtures/rewrite-404-invalid/package.json +++ b/packages/astro/test/fixtures/rewrite-custom-404/package.json @@ -1,5 +1,5 @@ { - "name": "@test/rewrite-404-invalid", + "name": "@test/rewrite-custom-404", "version": "0.0.0", "private": true, "dependencies": { diff --git a/packages/astro/test/fixtures/rewrite-custom-404/src/middleware.js b/packages/astro/test/fixtures/rewrite-custom-404/src/middleware.js new file mode 100644 index 000000000..4ca4affe1 --- /dev/null +++ b/packages/astro/test/fixtures/rewrite-custom-404/src/middleware.js @@ -0,0 +1,10 @@ + + +export const onRequest = async (context, next) => { + if (context.url.pathname.startsWith("/404") || context.url.pathname.startsWith("/500")) { + context.locals = { + interjected: "Interjected" + } + } + return await next(); +} diff --git a/packages/astro/test/fixtures/rewrite-custom-404/src/pages/404.astro b/packages/astro/test/fixtures/rewrite-custom-404/src/pages/404.astro new file mode 100644 index 000000000..e4a533a61 --- /dev/null +++ b/packages/astro/test/fixtures/rewrite-custom-404/src/pages/404.astro @@ -0,0 +1,13 @@ +--- +const interjected = Astro.locals.interjected; +--- + +<html> +<head> + <title>Custom error</title> +</head> +<body> +<h1>Custom error</h1> +<p>{interjected}</p> +</body> +</html> diff --git a/packages/astro/test/fixtures/rewrite-custom-404/src/pages/500.astro b/packages/astro/test/fixtures/rewrite-custom-404/src/pages/500.astro new file mode 100644 index 000000000..e4a533a61 --- /dev/null +++ b/packages/astro/test/fixtures/rewrite-custom-404/src/pages/500.astro @@ -0,0 +1,13 @@ +--- +const interjected = Astro.locals.interjected; +--- + +<html> +<head> + <title>Custom error</title> +</head> +<body> +<h1>Custom error</h1> +<p>{interjected}</p> +</body> +</html> diff --git a/packages/astro/test/fixtures/rewrite-custom-404/src/pages/about-2.astro b/packages/astro/test/fixtures/rewrite-custom-404/src/pages/about-2.astro new file mode 100644 index 000000000..b74006b8d --- /dev/null +++ b/packages/astro/test/fixtures/rewrite-custom-404/src/pages/about-2.astro @@ -0,0 +1,3 @@ +--- +return Astro.rewrite("/500") +--- diff --git a/packages/astro/test/fixtures/rewrite-404-invalid/src/pages/rewrite-404.astro b/packages/astro/test/fixtures/rewrite-custom-404/src/pages/about.astro index 536eb3501..536eb3501 100644 --- a/packages/astro/test/fixtures/rewrite-404-invalid/src/pages/rewrite-404.astro +++ b/packages/astro/test/fixtures/rewrite-custom-404/src/pages/about.astro diff --git a/packages/astro/test/rewrite.test.js b/packages/astro/test/rewrite.test.js index 048950a35..844179535 100644 --- a/packages/astro/test/rewrite.test.js +++ b/packages/astro/test/rewrite.test.js @@ -390,6 +390,40 @@ describe('Middleware', () => { }); }); + +describe('Middleware with custom 404.astro and 500.astro', () => { + /** @type {import('./test-utils').Fixture} */ + let fixture; + let devServer; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/rewrite-custom-404/', + }); + devServer = await fixture.startDevServer(); + }); + + after(async () => { + await devServer.stop(); + }); + + it('The `next()` function should return a Response with status code 404', async () => { + const html = await fixture.fetch('/about').then((res) => res.text()); + const $ = cheerioLoad(html); + + assert.equal($('h1').text(), 'Custom error'); + assert.equal($('p').text(), 'Interjected'); + }); + + it('The `next()` function should return a Response with status code 500', async () => { + const html = await fixture.fetch('/about-2').then((res) => res.text()); + const $ = cheerioLoad(html); + + assert.equal($('h1').text(), 'Custom error'); + assert.equal($('p').text(), 'Interjected'); + }); +}); + describe('Runtime error, default 500', () => { /** @type {import('./test-utils').Fixture} */ let fixture; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 65dea2754..b2a73463c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -3523,7 +3523,7 @@ importers: specifier: workspace:* version: link:../../.. - packages/astro/test/fixtures/rewrite-404-invalid: + packages/astro/test/fixtures/rewrite-custom-404: dependencies: astro: specifier: workspace:* |