From 6d2d0e279dd51e04099c86c4d900e2dd1d5fa837 Mon Sep 17 00:00:00 2001 From: Arsh <69170106+lilnasy@users.noreply.github.com> Date: Fri, 15 Dec 2023 17:15:04 +0000 Subject: [PATCH] fix(error pages): account for trailingSlash (#9126) * account for trailingSlash * add changeset * add tests * update lock file --- .changeset/old-pandas-travel.md | 5 ++ packages/astro/src/core/app/index.ts | 3 +- packages/astro/test/custom-404-server.test.js | 45 ------------ ...-404.test.js => custom-404-static.test.js} | 13 +++- .../custom-404-server/astro.config.mjs | 8 --- .../fixtures/custom-404-server/package.json | 8 --- .../custom-404-server/src/pages/[slug].astro | 6 -- .../astro.config.mjs | 0 .../package.json | 0 .../src/pages/404.astro | 0 .../src/pages/index.astro | 0 .../fixtures/custom-404/src/pages/404.astro | 13 ---- .../fixtures/custom-404/src/pages/index.astro | 11 --- .../package.json | 0 .../src/content/pages/index.md | 0 .../src/pages/404.astro | 0 .../src/pages/500.astro | 0 .../src/pages/api/route.js | 0 .../src/pages/blog/[...ssrPath].astro | 0 .../src/pages/causes-404.astro | 0 .../src/pages/causes-error.astro | 0 .../src/styles/main.css | 0 .../test/fixtures/status-code/package.json | 8 --- .../fixtures/status-code/src/pages/404.astro | 8 --- .../status-code/src/pages/index.astro | 8 --- ...-pages.test.js => ssr-error-pages.test.js} | 72 ++++++++++++++++--- packages/astro/test/status-page.test.js | 19 ----- pnpm-lock.yaml | 26 ++----- 28 files changed, 87 insertions(+), 166 deletions(-) create mode 100644 .changeset/old-pandas-travel.md delete mode 100644 packages/astro/test/custom-404-server.test.js rename packages/astro/test/{custom-404.test.js => custom-404-static.test.js} (78%) delete mode 100644 packages/astro/test/fixtures/custom-404-server/astro.config.mjs delete mode 100644 packages/astro/test/fixtures/custom-404-server/package.json delete mode 100644 packages/astro/test/fixtures/custom-404-server/src/pages/[slug].astro rename packages/astro/test/fixtures/{custom-404 => custom-404-static}/astro.config.mjs (100%) rename packages/astro/test/fixtures/{custom-404 => custom-404-static}/package.json (100%) rename packages/astro/test/fixtures/{custom-404-server => custom-404-static}/src/pages/404.astro (100%) rename packages/astro/test/fixtures/{custom-404-server => custom-404-static}/src/pages/index.astro (100%) delete mode 100644 packages/astro/test/fixtures/custom-404/src/pages/404.astro delete mode 100644 packages/astro/test/fixtures/custom-404/src/pages/index.astro rename packages/astro/test/fixtures/{ssr-api-route-custom-404 => ssr-error-pages}/package.json (100%) rename packages/astro/test/fixtures/{ssr-api-route-custom-404 => ssr-error-pages}/src/content/pages/index.md (100%) rename packages/astro/test/fixtures/{ssr-api-route-custom-404 => ssr-error-pages}/src/pages/404.astro (100%) rename packages/astro/test/fixtures/{ssr-api-route-custom-404 => ssr-error-pages}/src/pages/500.astro (100%) rename packages/astro/test/fixtures/{ssr-api-route-custom-404 => ssr-error-pages}/src/pages/api/route.js (100%) rename packages/astro/test/fixtures/{ssr-api-route-custom-404 => ssr-error-pages}/src/pages/blog/[...ssrPath].astro (100%) rename packages/astro/test/fixtures/{ssr-api-route-custom-404 => ssr-error-pages}/src/pages/causes-404.astro (100%) rename packages/astro/test/fixtures/{ssr-api-route-custom-404 => ssr-error-pages}/src/pages/causes-error.astro (100%) rename packages/astro/test/fixtures/{ssr-api-route-custom-404 => ssr-error-pages}/src/styles/main.css (100%) delete mode 100644 packages/astro/test/fixtures/status-code/package.json delete mode 100644 packages/astro/test/fixtures/status-code/src/pages/404.astro delete mode 100644 packages/astro/test/fixtures/status-code/src/pages/index.astro rename packages/astro/test/{ssr-404-500-pages.test.js => ssr-error-pages.test.js} (64%) delete mode 100644 packages/astro/test/status-page.test.js diff --git a/.changeset/old-pandas-travel.md b/.changeset/old-pandas-travel.md new file mode 100644 index 000000000000..bd1844066901 --- /dev/null +++ b/.changeset/old-pandas-travel.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes an issue where error pages were not shown when trailingSlash was set to "always". diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index b1689bac6aa3..4e446bf9cf28 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -333,7 +333,8 @@ export class App { request: Request, { status, response: originalResponse, skipMiddleware = false }: RenderErrorOptions ): Promise { - const errorRouteData = matchRoute('/' + status, this.#manifestData); + const errorRoutePath = `/${status}${this.#manifest.trailingSlash === "always" ? '/' : ''}`; + const errorRouteData = matchRoute(errorRoutePath, this.#manifestData); const url = new URL(request.url); if (errorRouteData) { if (errorRouteData.prerender) { diff --git a/packages/astro/test/custom-404-server.test.js b/packages/astro/test/custom-404-server.test.js deleted file mode 100644 index 0c824546b2c8..000000000000 --- a/packages/astro/test/custom-404-server.test.js +++ /dev/null @@ -1,45 +0,0 @@ -import { expect } from 'chai'; -import * as cheerio from 'cheerio'; -import { loadFixture } from './test-utils.js'; - -describe('Custom 404 server', () => { - let fixture; - - before(async () => { - fixture = await loadFixture({ - root: './fixtures/custom-404-server/', - site: 'http://example.com', - }); - }); - - describe('dev', () => { - let devServer; - let $; - - before(async () => { - devServer = await fixture.startDevServer(); - }); - - after(async () => { - await devServer.stop(); - }); - - it('renders /', async () => { - const html = await fixture.fetch('/').then((res) => res.text()); - $ = cheerio.load(html); - - expect($('h1').text()).to.equal('Home'); - }); - - it('renders 404 for /a', async () => { - const res = await fixture.fetch('/a'); - expect(res.status).to.equal(404); - - const html = await res.text(); - $ = cheerio.load(html); - - expect($('h1').text()).to.equal('Page not found'); - expect($('p').text()).to.equal('/a'); - }); - }); -}); diff --git a/packages/astro/test/custom-404.test.js b/packages/astro/test/custom-404-static.test.js similarity index 78% rename from packages/astro/test/custom-404.test.js rename to packages/astro/test/custom-404-static.test.js index c744f124c2cc..4400a1d3604c 100644 --- a/packages/astro/test/custom-404.test.js +++ b/packages/astro/test/custom-404-static.test.js @@ -7,7 +7,7 @@ describe('Custom 404', () => { before(async () => { fixture = await loadFixture({ - root: './fixtures/custom-404/', + root: './fixtures/custom-404-static/', site: 'http://example.com', }); }); @@ -42,4 +42,15 @@ describe('Custom 404', () => { expect($('p').text()).to.equal('/a'); }); }); + + describe('build', () => { + before(async () => { + await fixture.build(); + }) + + it('builds to 404.html', async () => { + const html = await fixture.readFile('/404.html'); + expect(html).to.be.ok; + }); + }); }); diff --git a/packages/astro/test/fixtures/custom-404-server/astro.config.mjs b/packages/astro/test/fixtures/custom-404-server/astro.config.mjs deleted file mode 100644 index 980ae408c9ab..000000000000 --- a/packages/astro/test/fixtures/custom-404-server/astro.config.mjs +++ /dev/null @@ -1,8 +0,0 @@ -import { defineConfig } from 'astro/config'; -import testAdapter from '../../test-adapter.js'; - -// https://astro.build/config -export default defineConfig({ - output: 'server', - adapter: testAdapter(), -}); diff --git a/packages/astro/test/fixtures/custom-404-server/package.json b/packages/astro/test/fixtures/custom-404-server/package.json deleted file mode 100644 index 184b79c954b8..000000000000 --- a/packages/astro/test/fixtures/custom-404-server/package.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "name": "@test/custom-404-server", - "version": "0.0.0", - "private": true, - "dependencies": { - "astro": "workspace:*" - } -} diff --git a/packages/astro/test/fixtures/custom-404-server/src/pages/[slug].astro b/packages/astro/test/fixtures/custom-404-server/src/pages/[slug].astro deleted file mode 100644 index 406a10a65b40..000000000000 --- a/packages/astro/test/fixtures/custom-404-server/src/pages/[slug].astro +++ /dev/null @@ -1,6 +0,0 @@ ---- -return new Response(null, { - status: 404, - statusText: 'Not Found' -}) ---- diff --git a/packages/astro/test/fixtures/custom-404/astro.config.mjs b/packages/astro/test/fixtures/custom-404-static/astro.config.mjs similarity index 100% rename from packages/astro/test/fixtures/custom-404/astro.config.mjs rename to packages/astro/test/fixtures/custom-404-static/astro.config.mjs diff --git a/packages/astro/test/fixtures/custom-404/package.json b/packages/astro/test/fixtures/custom-404-static/package.json similarity index 100% rename from packages/astro/test/fixtures/custom-404/package.json rename to packages/astro/test/fixtures/custom-404-static/package.json diff --git a/packages/astro/test/fixtures/custom-404-server/src/pages/404.astro b/packages/astro/test/fixtures/custom-404-static/src/pages/404.astro similarity index 100% rename from packages/astro/test/fixtures/custom-404-server/src/pages/404.astro rename to packages/astro/test/fixtures/custom-404-static/src/pages/404.astro diff --git a/packages/astro/test/fixtures/custom-404-server/src/pages/index.astro b/packages/astro/test/fixtures/custom-404-static/src/pages/index.astro similarity index 100% rename from packages/astro/test/fixtures/custom-404-server/src/pages/index.astro rename to packages/astro/test/fixtures/custom-404-static/src/pages/index.astro diff --git a/packages/astro/test/fixtures/custom-404/src/pages/404.astro b/packages/astro/test/fixtures/custom-404/src/pages/404.astro deleted file mode 100644 index 63d560b0fb91..000000000000 --- a/packages/astro/test/fixtures/custom-404/src/pages/404.astro +++ /dev/null @@ -1,13 +0,0 @@ ---- -const canonicalURL = new URL(Astro.url.pathname, Astro.site); ---- - - - - Not Found - Custom 404 - - -

Page not found

-

{canonicalURL.pathname}

- - diff --git a/packages/astro/test/fixtures/custom-404/src/pages/index.astro b/packages/astro/test/fixtures/custom-404/src/pages/index.astro deleted file mode 100644 index cf5ef9b586c6..000000000000 --- a/packages/astro/test/fixtures/custom-404/src/pages/index.astro +++ /dev/null @@ -1,11 +0,0 @@ ---- ---- - - - - Custom 404 - - -

Home

- - diff --git a/packages/astro/test/fixtures/ssr-api-route-custom-404/package.json b/packages/astro/test/fixtures/ssr-error-pages/package.json similarity index 100% rename from packages/astro/test/fixtures/ssr-api-route-custom-404/package.json rename to packages/astro/test/fixtures/ssr-error-pages/package.json diff --git a/packages/astro/test/fixtures/ssr-api-route-custom-404/src/content/pages/index.md b/packages/astro/test/fixtures/ssr-error-pages/src/content/pages/index.md similarity index 100% rename from packages/astro/test/fixtures/ssr-api-route-custom-404/src/content/pages/index.md rename to packages/astro/test/fixtures/ssr-error-pages/src/content/pages/index.md diff --git a/packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/404.astro b/packages/astro/test/fixtures/ssr-error-pages/src/pages/404.astro similarity index 100% rename from packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/404.astro rename to packages/astro/test/fixtures/ssr-error-pages/src/pages/404.astro diff --git a/packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/500.astro b/packages/astro/test/fixtures/ssr-error-pages/src/pages/500.astro similarity index 100% rename from packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/500.astro rename to packages/astro/test/fixtures/ssr-error-pages/src/pages/500.astro diff --git a/packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/api/route.js b/packages/astro/test/fixtures/ssr-error-pages/src/pages/api/route.js similarity index 100% rename from packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/api/route.js rename to packages/astro/test/fixtures/ssr-error-pages/src/pages/api/route.js diff --git a/packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/blog/[...ssrPath].astro b/packages/astro/test/fixtures/ssr-error-pages/src/pages/blog/[...ssrPath].astro similarity index 100% rename from packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/blog/[...ssrPath].astro rename to packages/astro/test/fixtures/ssr-error-pages/src/pages/blog/[...ssrPath].astro diff --git a/packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/causes-404.astro b/packages/astro/test/fixtures/ssr-error-pages/src/pages/causes-404.astro similarity index 100% rename from packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/causes-404.astro rename to packages/astro/test/fixtures/ssr-error-pages/src/pages/causes-404.astro diff --git a/packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/causes-error.astro b/packages/astro/test/fixtures/ssr-error-pages/src/pages/causes-error.astro similarity index 100% rename from packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/causes-error.astro rename to packages/astro/test/fixtures/ssr-error-pages/src/pages/causes-error.astro diff --git a/packages/astro/test/fixtures/ssr-api-route-custom-404/src/styles/main.css b/packages/astro/test/fixtures/ssr-error-pages/src/styles/main.css similarity index 100% rename from packages/astro/test/fixtures/ssr-api-route-custom-404/src/styles/main.css rename to packages/astro/test/fixtures/ssr-error-pages/src/styles/main.css diff --git a/packages/astro/test/fixtures/status-code/package.json b/packages/astro/test/fixtures/status-code/package.json deleted file mode 100644 index fcb9f1c94382..000000000000 --- a/packages/astro/test/fixtures/status-code/package.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "name": "@test/status-code", - "version": "0.0.0", - "private": true, - "dependencies": { - "astro": "workspace:*" - } -} diff --git a/packages/astro/test/fixtures/status-code/src/pages/404.astro b/packages/astro/test/fixtures/status-code/src/pages/404.astro deleted file mode 100644 index a80af2f506b0..000000000000 --- a/packages/astro/test/fixtures/status-code/src/pages/404.astro +++ /dev/null @@ -1,8 +0,0 @@ - - - Testing - - -

Testing

- - diff --git a/packages/astro/test/fixtures/status-code/src/pages/index.astro b/packages/astro/test/fixtures/status-code/src/pages/index.astro deleted file mode 100644 index a80af2f506b0..000000000000 --- a/packages/astro/test/fixtures/status-code/src/pages/index.astro +++ /dev/null @@ -1,8 +0,0 @@ - - - Testing - - -

Testing

- - diff --git a/packages/astro/test/ssr-404-500-pages.test.js b/packages/astro/test/ssr-error-pages.test.js similarity index 64% rename from packages/astro/test/ssr-404-500-pages.test.js rename to packages/astro/test/ssr-error-pages.test.js index 1c735e88995d..5332180fe81d 100644 --- a/packages/astro/test/ssr-404-500-pages.test.js +++ b/packages/astro/test/ssr-error-pages.test.js @@ -4,12 +4,12 @@ import testAdapter from './test-adapter.js'; import * as cheerio from 'cheerio'; describe('404 and 500 pages', () => { - /** @type {import('./test-utils').Fixture} */ + /** @type {import('./test-utils.js').Fixture} */ let fixture; before(async () => { fixture = await loadFixture({ - root: './fixtures/ssr-api-route-custom-404/', + root: './fixtures/ssr-error-pages/', output: 'server', adapter: testAdapter(), // test suite was authored when inlineStylesheets defaulted to never @@ -18,8 +18,9 @@ describe('404 and 500 pages', () => { }); describe('Development', () => { - /** @type {import('./test-utils').DevServer} */ + /** @type {import('./test-utils.js').DevServer} */ let devServer; + before(async () => { devServer = await fixture.startDevServer(); }); @@ -39,12 +40,15 @@ describe('404 and 500 pages', () => { }); describe('Production', () => { + /** @type {import('./test-utils.js').App} */ + let app; + before(async () => { await fixture.build({}); + app = await fixture.loadTestAdapterApp(); }); it('404 page returned when a route does not match', async () => { - const app = await fixture.loadTestAdapterApp(); const request = new Request('http://example.com/some/fake/route'); const response = await app.render(request); expect(response.status).to.equal(404); @@ -54,10 +58,9 @@ describe('404 and 500 pages', () => { }); it('404 page returned when a route does not match and passing routeData', async () => { - const app = await fixture.loadTestAdapterApp(); const request = new Request('http://example.com/some/fake/route'); const routeData = app.match(request); - const response = await app.render(request, routeData); + const response = await app.render(request, { routeData }); expect(response.status).to.equal(404); const html = await response.text(); const $ = cheerio.load(html); @@ -65,10 +68,9 @@ describe('404 and 500 pages', () => { }); it('404 page returned when a route does not match and imports are included', async () => { - const app = await fixture.loadTestAdapterApp(); const request = new Request('http://example.com/blog/fake/route'); const routeData = app.match(request); - const response = await app.render(request, routeData); + const response = await app.render(request, { routeData }); expect(response.status).to.equal(404); const html = await response.text(); const $ = cheerio.load(html); @@ -76,7 +78,6 @@ describe('404 and 500 pages', () => { }); it('404 page returned when there is an 404 response returned from route', async () => { - const app = await fixture.loadTestAdapterApp(); const request = new Request('http://example.com/causes-404'); const response = await app.render(request); expect(response.status).to.equal(404); @@ -86,7 +87,6 @@ describe('404 and 500 pages', () => { }); it('500 page returned when there is an error', async () => { - const app = await fixture.loadTestAdapterApp(); const request = new Request('http://example.com/causes-error'); const response = await app.render(request); expect(response.status).to.equal(500); @@ -96,7 +96,6 @@ describe('404 and 500 pages', () => { }); it('Returns 404 when hitting an API route with the wrong method', async () => { - const app = await fixture.loadTestAdapterApp(); const request = new Request('http://example.com/api/route', { method: 'PUT', }); @@ -108,3 +107,54 @@ describe('404 and 500 pages', () => { }); }); }); + +describe('trailing slashes for error pages', () => { + /** @type {import('./test-utils.js').Fixture} */ + let fixture; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/ssr-error-pages/', + output: 'server', + adapter: testAdapter(), + trailingSlash: 'always' + }); + }); + + describe('Development', () => { + /** @type {import('./test-utils.js').DevServer} */ + let devServer; + + before(async () => { + devServer = await fixture.startDevServer(); + }); + + it('renders 404 page when a route does not match the request', async () => { + const response = await fixture.fetch('/ashbfjkasn'); + expect(response).to.deep.include({ status: 404 }); + const html = await response.text(); + expect(html).to.not.be.empty; + const $ = cheerio.load(html); + expect($('h1').text()).to.equal(`Something went horribly wrong!`); + }); + }); + + describe('Production', () => { + /** @type {import('./test-utils.js').App} */ + let app; + + before(async () => { + await fixture.build({}); + app = await fixture.loadTestAdapterApp(); + }); + + it('renders 404 page when a route does not match the request', async () => { + const response = await app.render(new Request('http://example.com/ajksalscla')); + expect(response).to.deep.include({ status: 404 }); + const html = await response.text(); + expect(html).to.not.be.empty; + const $ = cheerio.load(html); + expect($('h1').text()).to.equal('Something went horribly wrong!'); + }) + }); +}); diff --git a/packages/astro/test/status-page.test.js b/packages/astro/test/status-page.test.js deleted file mode 100644 index 06b2d0ae8de8..000000000000 --- a/packages/astro/test/status-page.test.js +++ /dev/null @@ -1,19 +0,0 @@ -import { expect } from 'chai'; -import { loadFixture } from './test-utils.js'; - -// Asset bundling -describe('Status Code Pages', () => { - let fixture; - - before(async () => { - fixture = await loadFixture({ - root: './fixtures/status-code/', - }); - await fixture.build(); - }); - - it('builds to 404.html', async () => { - const html = await fixture.readFile('/404.html'); - expect(html).to.be.ok; - }); -}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 796f0da528c2..7be5adf7c679 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2554,12 +2554,6 @@ importers: specifier: workspace:* version: link:../../.. - packages/astro/test/fixtures/custom-404: - dependencies: - astro: - specifier: workspace:* - version: link:../../.. - packages/astro/test/fixtures/custom-404-html: dependencies: astro: @@ -2617,7 +2611,7 @@ importers: specifier: workspace:* version: link:../../.. - packages/astro/test/fixtures/custom-404-server: + packages/astro/test/fixtures/custom-404-static: dependencies: astro: specifier: workspace:* @@ -3353,12 +3347,6 @@ importers: specifier: workspace:* version: link:../../.. - packages/astro/test/fixtures/ssr-api-route-custom-404: - dependencies: - astro: - specifier: workspace:* - version: link:../../.. - packages/astro/test/fixtures/ssr-assets: dependencies: astro: @@ -3383,6 +3371,12 @@ importers: specifier: ^10.19.2 version: 10.19.2 + packages/astro/test/fixtures/ssr-error-pages: + dependencies: + astro: + specifier: workspace:* + version: link:../../.. + packages/astro/test/fixtures/ssr-hoisted-script: dependencies: astro: @@ -3538,12 +3532,6 @@ importers: packages/astro/test/fixtures/static-build/pkg: {} - packages/astro/test/fixtures/status-code: - dependencies: - astro: - specifier: workspace:* - version: link:../../.. - packages/astro/test/fixtures/streaming: dependencies: astro: