summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Emanuele Stoppa <my.burning@gmail.com> 2024-06-24 13:21:27 +0100
committerGravatar GitHub <noreply@github.com> 2024-06-24 13:21:27 +0100
commit44c61ddfd85f1c23f8cec8caeaa5e25897121996 (patch)
treeff339c744af08b5c9b1a678c2f7acc364caf5a36
parentedd35d3c6996e878c0011d811014bdcd1fa7c4c4 (diff)
downloadastro-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
-rw-r--r--.changeset/curvy-emus-mate.md5
-rw-r--r--packages/astro/src/core/cookies/response.ts4
-rw-r--r--packages/astro/src/core/render-context.ts5
-rw-r--r--packages/astro/src/runtime/server/render/page.ts17
-rw-r--r--packages/astro/test/custom-404-implicit-rerouting.test.js2
-rw-r--r--packages/astro/test/fixtures/rewrite-custom-404/astro.config.mjs (renamed from packages/astro/test/fixtures/rewrite-404-invalid/astro.config.mjs)0
-rw-r--r--packages/astro/test/fixtures/rewrite-custom-404/package.json (renamed from packages/astro/test/fixtures/rewrite-404-invalid/package.json)2
-rw-r--r--packages/astro/test/fixtures/rewrite-custom-404/src/middleware.js10
-rw-r--r--packages/astro/test/fixtures/rewrite-custom-404/src/pages/404.astro13
-rw-r--r--packages/astro/test/fixtures/rewrite-custom-404/src/pages/500.astro13
-rw-r--r--packages/astro/test/fixtures/rewrite-custom-404/src/pages/about-2.astro3
-rw-r--r--packages/astro/test/fixtures/rewrite-custom-404/src/pages/about.astro (renamed from packages/astro/test/fixtures/rewrite-404-invalid/src/pages/rewrite-404.astro)0
-rw-r--r--packages/astro/test/rewrite.test.js34
-rw-r--r--pnpm-lock.yaml2
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:*