diff options
4 files changed, 54 insertions, 50 deletions
diff --git a/.changeset/sharp-elephants-clap.md b/.changeset/sharp-elephants-clap.md new file mode 100644 index 000000000..b1bf39bfa --- /dev/null +++ b/.changeset/sharp-elephants-clap.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Fixes a regression where constructing and returning 404 responses from a middleware resulted in the dev server getting stuck in a loop. diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index c793151a4..322864148 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -287,7 +287,7 @@ export async function handleRoute({ } if (response.status === 404 && response.headers.get(REROUTE_DIRECTIVE_HEADER) !== 'no') { const fourOhFourRoute = await matchRoute('/404', manifestData, pipeline); - if (options) + if (options && options.route !== fourOhFourRoute?.route) return handleRoute({ ...options, matchedRoute: fourOhFourRoute, diff --git a/packages/astro/test/custom-404-implicit-rerouting.test.js b/packages/astro/test/custom-404-implicit-rerouting.test.js index 4986b34c4..5dcda1329 100644 --- a/packages/astro/test/custom-404-implicit-rerouting.test.js +++ b/packages/astro/test/custom-404-implicit-rerouting.test.js @@ -1,69 +1,67 @@ import assert from 'node:assert/strict'; import { after, before, describe, it } from 'node:test'; import { loadFixture } from './test-utils.js'; +import testAdapter from './test-adapter.js'; -for (const caseNumber of [1, 2, 3, 4]) { +for (const caseNumber of [1, 2, 3, 4, 5]) { describe(`Custom 404 with implicit rerouting - Case #${caseNumber}`, () => { - /** @type Awaited<ReturnType<typeof loadFixture>> */ + /** @type {import('./test-utils.js').Fixture} */ let fixture; - /** @type Awaited<ReturnType<typeof fixture['startDevServer']>> */ - let devServer; before(async () => { fixture = await loadFixture({ + output: 'server', root: `./fixtures/custom-404-loop-case-${caseNumber}/`, site: 'http://example.com', + adapter: testAdapter() }); - - devServer = await fixture.startDevServer(); }); - // sanity check - it.skip( - 'dev server handles normal requests', - { - todo: 'To re-enabled after we understand why this fails when the test suite is run in parallel', - }, - async () => { - const resPromise = fixture.fetch('/'); - const result = await withTimeout(resPromise, 1000); - assert.notEqual(result, timeout); - assert.equal(result.status, 200); - } - ); + describe("dev server", () => { + /** @type {import('./test-utils.js').DevServer} */ + let devServer; - it.skip( - 'dev server stays responsive', - { - todo: 'To re-enabled after we understand why this fails when the test suite is run in parallel', - }, - async () => { - const resPromise = fixture.fetch('/alvsibdlvjks'); - const result = await withTimeout(resPromise, 1000); - assert.notEqual(result, timeout); - assert.equal(result.status, 404); - } - ); + before(async () => { + await fixture.build() + devServer = await fixture.startDevServer(); + }); - after(async () => { - await devServer.stop(); + // sanity check + it('dev server handles normal requests', { timeout: 1000 }, async () => { + const response = await fixture.fetch('/'); + assert.equal(response.status, 200); + }); + + // IMPORTANT: never skip + it('dev server stays responsive', { timeout: 1000 }, async () => { + const response = await fixture.fetch('/alvsibdlvjks'); + assert.equal(response.status, 404); + }); + + after(async () => { + await devServer.stop(); + }); }); - }); -} + + describe("prod server", () => { + /** @type {import('./test-utils.js').App} */ + let app; -/***** UTILITY FUNCTIONS *****/ - -const timeout = Symbol('timeout'); - -/** @template Res */ -function withTimeout( - /** @type Promise<Res> */ - responsePromise, - /** @type number */ - timeLimit -) { - /** @type Promise<typeof timeout> */ - const timeoutPromise = new Promise((resolve) => setTimeout(() => resolve(timeout), timeLimit)); + before(async () => { + app = await fixture.loadTestAdapterApp(); + }); - return Promise.race([responsePromise, timeoutPromise]); + // sanity check + it('prod server handles normal requests', { timeout: 1000 }, async () => { + const response = await app.render(new Request('https://example.com/')); + assert.equal(response.status, 200); + }); + + // IMPORTANT: never skip + it('prod server stays responsive', { 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/custom-404-loop-case-5/src/pages/index.astro b/packages/astro/test/fixtures/custom-404-loop-case-5/src/pages/index.astro new file mode 100644 index 000000000..a65c81fdb --- /dev/null +++ b/packages/astro/test/fixtures/custom-404-loop-case-5/src/pages/index.astro @@ -0,0 +1 @@ +<p>all good here... or is it?</p>
\ No newline at end of file |