Infrastructure/astro
0
Fork 0
mirror of https://github.com/withastro/astro.git synced 2025-02-03 22:29:08 -05:00
Code Issues Activity

fix(error pages): account for trailingSlash (#9126)

Browse source 
* account for trailingSlash

* add changeset

* add tests

* update lock file
This commit is contained in:
Arsh 2023-12-15 17:15:04 +00:00 • committed by GitHub
parent c01580a2cd
commit 6d2d0e279d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
28 changed files with 87 additions and 166 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
.changeset/old-pandas-travel.md Normal file
View file

@ -0,0 +1,5 @@
---
'astro': patch
---
Fixes an issue where error pages were not shown when trailingSlash was set to "always".

3
packages/astro/src/core/app/index.ts
View file

@ -333,7 +333,8 @@ export class App {
request: Request, request: Request,
{ status, response: originalResponse, skipMiddleware = false }: RenderErrorOptions { status, response: originalResponse, skipMiddleware = false }: RenderErrorOptions
): Promise<Response> { ): Promise<Response> {
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); const url = new URL(request.url);
if (errorRouteData) { if (errorRouteData) {
if (errorRouteData.prerender) { if (errorRouteData.prerender) {

45
packages/astro/test/custom-404-server.test.js
View file

@ -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');
});
});
});

13
packages/astro/test/custom-404.test.js → packages/astro/test/custom-404-static.test.js
View file

@ -7,7 +7,7 @@ describe('Custom 404', () => {
before(async () => { before(async () => {
fixture = await loadFixture({ fixture = await loadFixture({
root: './fixtures/custom-404/', root: './fixtures/custom-404-static/',
site: 'http://example.com', site: 'http://example.com',
}); });
}); });
@ -42,4 +42,15 @@ describe('Custom 404', () => {
expect($('p').text()).to.equal('/a'); 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;
});
});
}); });

8
packages/astro/test/fixtures/custom-404-server/astro.config.mjs vendored
View file

@ -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(),
});

8
packages/astro/test/fixtures/custom-404-server/package.json vendored
View file

@ -1,8 +0,0 @@
{
"name": "@test/custom-404-server",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
}
}

6
packages/astro/test/fixtures/custom-404-server/src/pages/[slug].astro vendored
View file

@ -1,6 +0,0 @@
---
return new Response(null, {
status: 404,
statusText: 'Not Found'
})
---

0
packages/astro/test/fixtures/custom-404/astro.config.mjs → packages/astro/test/fixtures/custom-404-static/astro.config.mjs vendored
View file

0
packages/astro/test/fixtures/custom-404/package.json → packages/astro/test/fixtures/custom-404-static/package.json vendored
View file

0
packages/astro/test/fixtures/custom-404-server/src/pages/404.astro → packages/astro/test/fixtures/custom-404-static/src/pages/404.astro vendored
View file

0
packages/astro/test/fixtures/custom-404-server/src/pages/index.astro → packages/astro/test/fixtures/custom-404-static/src/pages/index.astro vendored
View file

13
packages/astro/test/fixtures/custom-404/src/pages/404.astro vendored
View file

@ -1,13 +0,0 @@
---
const canonicalURL = new URL(Astro.url.pathname, Astro.site);
---
<html lang="en">
<head>
<title>Not Found - Custom 404</title>
</head>
<body>
<h1>Page not found</h1>
<p>{canonicalURL.pathname}</p>
</body>
</html>

11
packages/astro/test/fixtures/custom-404/src/pages/index.astro vendored
View file

@ -1,11 +0,0 @@
---
---
<html lang="en">
<head>
<title>Custom 404</title>
</head>
<body>
<h1>Home</h1>
</body>
</html>

0
packages/astro/test/fixtures/ssr-api-route-custom-404/package.json → packages/astro/test/fixtures/ssr-error-pages/package.json vendored
View file

0
packages/astro/test/fixtures/ssr-api-route-custom-404/src/content/pages/index.md → packages/astro/test/fixtures/ssr-error-pages/src/content/pages/index.md vendored
View file

0
packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/404.astro → packages/astro/test/fixtures/ssr-error-pages/src/pages/404.astro vendored
View file

0
packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/500.astro → packages/astro/test/fixtures/ssr-error-pages/src/pages/500.astro vendored
View file

0
packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/api/route.js → packages/astro/test/fixtures/ssr-error-pages/src/pages/api/route.js vendored
View file

0
packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/blog/[...ssrPath].astro → packages/astro/test/fixtures/ssr-error-pages/src/pages/blog/[...ssrPath].astro vendored
View file

0
packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/causes-404.astro → packages/astro/test/fixtures/ssr-error-pages/src/pages/causes-404.astro vendored
View file

0
packages/astro/test/fixtures/ssr-api-route-custom-404/src/pages/causes-error.astro → packages/astro/test/fixtures/ssr-error-pages/src/pages/causes-error.astro vendored
View file

0
packages/astro/test/fixtures/ssr-api-route-custom-404/src/styles/main.css → packages/astro/test/fixtures/ssr-error-pages/src/styles/main.css vendored
View file

8
packages/astro/test/fixtures/status-code/package.json vendored
View file

@ -1,8 +0,0 @@
{
"name": "@test/status-code",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
}
}

8
packages/astro/test/fixtures/status-code/src/pages/404.astro vendored
View file

@ -1,8 +0,0 @@
<html>
<head>
<title>Testing</title>
</head>
<body>
<h1>Testing</h1>
</body>
<html>

8
packages/astro/test/fixtures/status-code/src/pages/index.astro vendored
View file

@ -1,8 +0,0 @@
<html>
<head>
<title>Testing</title>
</head>
<body>
<h1>Testing</h1>
</body>
<html>

72
packages/astro/test/ssr-404-500-pages.test.js → packages/astro/test/ssr-error-pages.test.js
View file

@ -4,12 +4,12 @@ import testAdapter from './test-adapter.js';
import * as cheerio from 'cheerio'; import * as cheerio from 'cheerio';
describe('404 and 500 pages', () => { describe('404 and 500 pages', () => {
/** @type {import('./test-utils').Fixture} */ /** @type {import('./test-utils.js').Fixture} */
let fixture; let fixture;
before(async () => { before(async () => {
fixture = await loadFixture({ fixture = await loadFixture({
root: './fixtures/ssr-api-route-custom-404/', root: './fixtures/ssr-error-pages/',
output: 'server', output: 'server',
adapter: testAdapter(), adapter: testAdapter(),
// test suite was authored when inlineStylesheets defaulted to never // test suite was authored when inlineStylesheets defaulted to never
@ -18,8 +18,9 @@ describe('404 and 500 pages', () => {
}); });
describe('Development', () => { describe('Development', () => {
/** @type {import('./test-utils').DevServer} */ /** @type {import('./test-utils.js').DevServer} */
let devServer; let devServer;
before(async () => { before(async () => {
devServer = await fixture.startDevServer(); devServer = await fixture.startDevServer();
}); });
@ -39,12 +40,15 @@ describe('404 and 500 pages', () => {
}); });
describe('Production', () => { describe('Production', () => {
/** @type {import('./test-utils.js').App} */
let app;
before(async () => { before(async () => {
await fixture.build({}); await fixture.build({});
app = await fixture.loadTestAdapterApp();
}); });
it('404 page returned when a route does not match', async () => { 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 request = new Request('http://example.com/some/fake/route');
const response = await app.render(request); const response = await app.render(request);
expect(response.status).to.equal(404); 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 () => { 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 request = new Request('http://example.com/some/fake/route');
const routeData = app.match(request); 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); expect(response.status).to.equal(404);
const html = await response.text(); const html = await response.text();
const $ = cheerio.load(html); 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 () => { 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 request = new Request('http://example.com/blog/fake/route');
const routeData = app.match(request); 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); expect(response.status).to.equal(404);
const html = await response.text(); const html = await response.text();
const $ = cheerio.load(html); 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 () => { 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 request = new Request('http://example.com/causes-404');
const response = await app.render(request); const response = await app.render(request);
expect(response.status).to.equal(404); 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 () => { 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 request = new Request('http://example.com/causes-error');
const response = await app.render(request); const response = await app.render(request);
expect(response.status).to.equal(500); 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 () => { 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', { const request = new Request('http://example.com/api/route', {
method: 'PUT', 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!');
})
});
});

19
packages/astro/test/status-page.test.js
View file

@ -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;
});
});

26
pnpm-lock.yaml generated
View file

@ -2554,12 +2554,6 @@ importers:
specifier: workspace:* specifier: workspace:*
version: link:../../.. version: link:../../..
packages/astro/test/fixtures/custom-404:
dependencies:
astro:
specifier: workspace:*
version: link:../../..
packages/astro/test/fixtures/custom-404-html: packages/astro/test/fixtures/custom-404-html:
dependencies: dependencies:
astro: astro:
@ -2617,7 +2611,7 @@ importers:
specifier: workspace:* specifier: workspace:*
version: link:../../.. version: link:../../..
packages/astro/test/fixtures/custom-404-server: packages/astro/test/fixtures/custom-404-static:
dependencies: dependencies:
astro: astro:
specifier: workspace:* specifier: workspace:*
@ -3353,12 +3347,6 @@ importers:
specifier: workspace:* specifier: workspace:*
version: link:../../.. version: link:../../..
packages/astro/test/fixtures/ssr-api-route-custom-404:
dependencies:
astro:
specifier: workspace:*
version: link:../../..
packages/astro/test/fixtures/ssr-assets: packages/astro/test/fixtures/ssr-assets:
dependencies: dependencies:
astro: astro:
@ -3383,6 +3371,12 @@ importers:
specifier: ^10.19.2 specifier: ^10.19.2
version: 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: packages/astro/test/fixtures/ssr-hoisted-script:
dependencies: dependencies:
astro: astro:
@ -3538,12 +3532,6 @@ importers:
packages/astro/test/fixtures/static-build/pkg: {} packages/astro/test/fixtures/static-build/pkg: {}
packages/astro/test/fixtures/status-code:
dependencies:
astro:
specifier: workspace:*
version: link:../../..
packages/astro/test/fixtures/streaming: packages/astro/test/fixtures/streaming:
dependencies: dependencies:
astro: astro: