summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat (limited to '')
-rw-r--r--.changeset/shaggy-onions-try.md5
-rw-r--r--packages/astro/src/core/app/index.ts19
-rw-r--r--packages/astro/src/core/pipeline.ts6
-rw-r--r--packages/astro/test/fixtures/middleware space/src/middleware.js2
-rw-r--r--packages/astro/test/fixtures/middleware space/src/pages/500.astro1
-rw-r--r--packages/astro/test/fixtures/middleware space/src/pages/throw.astro0
-rw-r--r--packages/astro/test/middleware.test.js27
7 files changed, 45 insertions, 15 deletions
diff --git a/.changeset/shaggy-onions-try.md b/.changeset/shaggy-onions-try.md
new file mode 100644
index 000000000..3ae58f2d8
--- /dev/null
+++ b/.changeset/shaggy-onions-try.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fix an issue where `500.astro` did not render when the middleware threw an error.
diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts
index 9cf01f82d..6475718ac 100644
--- a/packages/astro/src/core/app/index.ts
+++ b/packages/astro/src/core/app/index.ts
@@ -41,6 +41,10 @@ export interface RenderErrorOptions {
routeData?: RouteData;
response?: Response;
status: 404 | 500;
+ /**
+ * Whether to skip onRequest() while rendering the error page. Defaults to false.
+ */
+ skipMiddleware?: boolean;
}
export class App {
@@ -252,7 +256,7 @@ export class App {
* If it is a known error code, try sending the according page (e.g. 404.astro / 500.astro).
* This also handles pre-rendered /404 or /500 routes
*/
- async #renderError(request: Request, { status, response: originalResponse }: RenderErrorOptions) {
+ async #renderError(request: Request, { status, response: originalResponse, skipMiddleware = false }: RenderErrorOptions): Promise<Response> {
const errorRouteData = matchRoute('/' + status, this.#manifestData);
const url = new URL(request.url);
if (errorRouteData) {
@@ -280,12 +284,21 @@ export class App {
status
);
const page = (await mod.page()) as any;
- if (mod.onRequest) {
+ if (skipMiddleware === false && mod.onRequest) {
this.#pipeline.setMiddlewareFunction(mod.onRequest as MiddlewareEndpointHandler);
}
+ if (skipMiddleware) {
+ // make sure middleware set by other requests is cleared out
+ this.#pipeline.unsetMiddlewareFunction();
+ }
const response = await this.#pipeline.renderRoute(newRenderContext, page);
return this.#mergeResponses(response, originalResponse);
- } catch {}
+ } catch {
+ // Middleware may be the cause of the error, so we try rendering 404/500.astro without it.
+ if (skipMiddleware === false && mod.onRequest) {
+ return this.#renderError(request, { status, response: originalResponse, skipMiddleware: true });
+ }
+ }
}
const response = this.#mergeResponses(new Response(null, { status }), originalResponse);
diff --git a/packages/astro/src/core/pipeline.ts b/packages/astro/src/core/pipeline.ts
index 438ff275d..dd1e66a52 100644
--- a/packages/astro/src/core/pipeline.ts
+++ b/packages/astro/src/core/pipeline.ts
@@ -56,6 +56,12 @@ export class Pipeline {
}
/**
+ * Removes the current middleware function. Subsequent requests won't trigger any middleware.
+ */
+ unsetMiddlewareFunction() {
+ this.#onRequest = undefined;
+ }
+ /**
* Returns the current environment
*/
getEnvironment(): Readonly<Environment> {
diff --git a/packages/astro/test/fixtures/middleware space/src/middleware.js b/packages/astro/test/fixtures/middleware space/src/middleware.js
index b2e5c7e2d..a889569c4 100644
--- a/packages/astro/test/fixtures/middleware space/src/middleware.js
+++ b/packages/astro/test/fixtures/middleware space/src/middleware.js
@@ -18,6 +18,8 @@ const first = defineMiddleware(async (context, next) => {
return new Response(JSON.stringify(object), {
headers: response.headers,
});
+ } else if (context.url.pathname === '/throw') {
+ throw new Error;
} else if (context.url.pathname === '/clone') {
const response = await next();
const newResponse = response.clone();
diff --git a/packages/astro/test/fixtures/middleware space/src/pages/500.astro b/packages/astro/test/fixtures/middleware space/src/pages/500.astro
new file mode 100644
index 000000000..fc6b2588b
--- /dev/null
+++ b/packages/astro/test/fixtures/middleware space/src/pages/500.astro
@@ -0,0 +1 @@
+<h1>There was an error rendering the page.</h1> \ No newline at end of file
diff --git a/packages/astro/test/fixtures/middleware space/src/pages/throw.astro b/packages/astro/test/fixtures/middleware space/src/pages/throw.astro
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/packages/astro/test/fixtures/middleware space/src/pages/throw.astro
diff --git a/packages/astro/test/middleware.test.js b/packages/astro/test/middleware.test.js
index d3cb83310..b882fcc84 100644
--- a/packages/astro/test/middleware.test.js
+++ b/packages/astro/test/middleware.test.js
@@ -119,6 +119,8 @@ describe('Middleware API in PROD mode, SSR', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;
let middlewarePath;
+ /** @type {import('../src/core/app/index').App} */
+ let app;
before(async () => {
fixture = await loadFixture({
@@ -127,10 +129,10 @@ describe('Middleware API in PROD mode, SSR', () => {
adapter: testAdapter({}),
});
await fixture.build();
+ app = await fixture.loadTestAdapterApp();
});
it('should render locals data', async () => {
- const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/');
const response = await app.render(request);
const html = await response.text();
@@ -139,7 +141,6 @@ describe('Middleware API in PROD mode, SSR', () => {
});
it('should change locals data based on URL', async () => {
- const app = await fixture.loadTestAdapterApp();
let response = await app.render(new Request('http://example.com/'));
let html = await response.text();
let $ = cheerio.load(html);
@@ -152,14 +153,12 @@ describe('Middleware API in PROD mode, SSR', () => {
});
it('should successfully redirect to another page', async () => {
- const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/redirect');
const response = await app.render(request);
expect(response.status).to.equal(302);
});
it('should call a second middleware', async () => {
- const app = await fixture.loadTestAdapterApp();
const response = await app.render(new Request('http://example.com/second'));
const html = await response.text();
const $ = cheerio.load(html);
@@ -167,7 +166,6 @@ describe('Middleware API in PROD mode, SSR', () => {
});
it('should successfully create a new response', async () => {
- const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/rewrite');
const response = await app.render(request);
const html = await response.text();
@@ -177,14 +175,12 @@ describe('Middleware API in PROD mode, SSR', () => {
});
it('should return a new response that is a 500', async () => {
- const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/broken-500');
const response = await app.render(request);
expect(response.status).to.equal(500);
});
it('should successfully render a page if the middleware calls only next() and returns nothing', async () => {
- const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/not-interested');
const response = await app.render(request);
const html = await response.text();
@@ -192,8 +188,7 @@ describe('Middleware API in PROD mode, SSR', () => {
expect($('p').html()).to.equal('Not interested');
});
- it("should throws an error when the middleware doesn't call next or doesn't return a response", async () => {
- const app = await fixture.loadTestAdapterApp();
+ it("should throw an error when the middleware doesn't call next or doesn't return a response", async () => {
const request = new Request('http://example.com/does-nothing');
const response = await app.render(request);
const html = await response.text();
@@ -202,7 +197,6 @@ describe('Middleware API in PROD mode, SSR', () => {
});
it('should correctly work for API endpoints that return a Response object', async () => {
- const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/api/endpoint');
const response = await app.render(request);
expect(response.status).to.equal(200);
@@ -210,7 +204,6 @@ describe('Middleware API in PROD mode, SSR', () => {
});
it('should correctly manipulate the response coming from API endpoints (not simple)', async () => {
- const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/api/endpoint');
const response = await app.render(request);
const text = await response.text();
@@ -218,7 +211,6 @@ describe('Middleware API in PROD mode, SSR', () => {
});
it('should correctly call the middleware function for 404', async () => {
- const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/funky-url');
const routeData = app.match(request, { matchNotFound: true });
const response = await app.render(request, routeData);
@@ -227,6 +219,17 @@ describe('Middleware API in PROD mode, SSR', () => {
expect(text.includes('bar')).to.be.true;
});
+ it('should render 500.astro when the middleware throws an error', async () => {
+ const request = new Request('http://example.com/throw');
+ const routeData = app.match(request, { matchNotFound: true });
+
+ const response = await app.render(request, routeData);
+ expect(response).to.deep.include({ status: 500 });
+
+ const text = await response.text();
+ expect(text).to.include("<h1>There was an error rendering the page.</h1>")
+ });
+
it('the integration should receive the path to the middleware', async () => {
fixture = await loadFixture({
root: './fixtures/middleware space/',
generated by cgit v1.2.3 (git 2.46.0) at 2025-08-18 18:13:14 +0000