0
Fork 0
mirror of https://github.com/withastro/astro.git synced 2024-12-16 21:46:22 -05:00

feat: 500.astro improvements (#11134)

* feat: better error handling

* feat: allow passing props to render context render

* feat: work on tests

* Update 500.astro

* feat: test preview custom 500

* feat: test for custom 500 failing

* feat: add changeset

* Update rich-dolls-compete.md

* Delete packages/astro/e2e/custom-500.test.js

* Apply suggestions from code review

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>

* fix: merge

* Update packages/astro/test/custom-500.test.js

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>

* Apply suggestions from code review

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>

* Update packages/astro/src/core/app/index.ts

* feat: update

---------

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Matthew Phillips <matthew@skypack.dev>
This commit is contained in:
Florian Lefebvre 2024-06-19 16:00:16 +02:00 committed by GitHub
parent c44f7f4bab
commit 9042be0491
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 270 additions and 10 deletions

View file

@ -0,0 +1,22 @@
---
"astro": minor
---
Improves the developer experience of the `500.astro` file by passing it a new `error` prop.
When an error is thrown, the special `src/pages/500.astro` page now automatically receives the error as a prop. This allows you to display more specific information about the error on a custom 500 page.
```astro
---
// src/pages/500.astro
interface Props {
error: unknown
}
const { error } = Astro.props
---
<div>{error instanceof Error ? error.message : 'Unknown error'}</div>
```
If an error occurs rendering this page, your host's default 500 error page will be shown to your visitor in production, and Astro's default error overlay will be shown in development.

View file

@ -1,9 +1,10 @@
import { defineConfig } from '@playwright/test';
// NOTE: Sometimes, tests fail with `TypeError: process.stdout.clearLine is not a function` // NOTE: Sometimes, tests fail with `TypeError: process.stdout.clearLine is not a function`
// for some reason. This comes from Vite, and is conditionally called based on `isTTY`. // for some reason. This comes from Vite, and is conditionally called based on `isTTY`.
// We set it to false here to skip this odd behavior. // We set it to false here to skip this odd behavior.
process.stdout.isTTY = false; process.stdout.isTTY = false;
const config = { export default defineConfig({
testMatch: 'e2e/*.test.js', testMatch: 'e2e/*.test.js',
/* Maximum time one test can run for. */ /* Maximum time one test can run for. */
timeout: 40 * 1000, timeout: 40 * 1000,
@ -37,6 +38,4 @@ const config = {
}, },
}, },
], ],
}; });
export default config;

View file

@ -67,6 +67,10 @@ export interface RenderErrorOptions {
* Whether to skip middleware while rendering the error page. Defaults to false. * Whether to skip middleware while rendering the error page. Defaults to false.
*/ */
skipMiddleware?: boolean; skipMiddleware?: boolean;
/**
* Allows passing an error to 500.astro. It will be available through `Astro.props.error`.
*/
error?: unknown;
} }
export class App { export class App {
@ -289,8 +293,9 @@ export class App {
} }
if (locals) { if (locals) {
if (typeof locals !== 'object') { if (typeof locals !== 'object') {
this.#logger.error(null, new AstroError(AstroErrorData.LocalsNotAnObject).stack!); const error = new AstroError(AstroErrorData.LocalsNotAnObject);
return this.#renderError(request, { status: 500 }); this.#logger.error(null, error.stack!);
return this.#renderError(request, { status: 500, error });
} }
Reflect.set(request, clientLocalsSymbol, locals); Reflect.set(request, clientLocalsSymbol, locals);
} }
@ -324,7 +329,7 @@ export class App {
response = await renderContext.render(await mod.page()); response = await renderContext.render(await mod.page());
} catch (err: any) { } catch (err: any) {
this.#logger.error(null, err.stack || err.message || String(err)); this.#logger.error(null, err.stack || err.message || String(err));
return this.#renderError(request, { locals, status: 500 }); return this.#renderError(request, { locals, status: 500, error: err });
} }
if ( if (
@ -335,6 +340,9 @@ export class App {
locals, locals,
response, response,
status: response.status as 404 | 500, status: response.status as 404 | 500,
// We don't have an error to report here. Passing null means we pass nothing intentionally
// while undefined means there's no error
error: response.status === 500 ? null : undefined,
}); });
} }
@ -385,7 +393,13 @@ export class App {
*/ */
async #renderError( async #renderError(
request: Request, request: Request,
{ locals, status, response: originalResponse, skipMiddleware = false }: RenderErrorOptions {
locals,
status,
response: originalResponse,
skipMiddleware = false,
error,
}: RenderErrorOptions
): Promise<Response> { ): Promise<Response> {
const errorRoutePath = `/${status}${this.#manifest.trailingSlash === 'always' ? '/' : ''}`; const errorRoutePath = `/${status}${this.#manifest.trailingSlash === 'always' ? '/' : ''}`;
const errorRouteData = matchRoute(errorRoutePath, this.#manifestData); const errorRouteData = matchRoute(errorRoutePath, this.#manifestData);
@ -415,6 +429,7 @@ export class App {
request, request,
routeData: errorRouteData, routeData: errorRouteData,
status, status,
props: { error }
}); });
const response = await renderContext.render(await mod.page()); const response = await renderContext.render(await mod.page());
return this.#mergeResponses(response, originalResponse); return this.#mergeResponses(response, originalResponse);

View file

@ -74,8 +74,9 @@ export class RenderContext {
request, request,
routeData, routeData,
status = 200, status = 200,
props,
}: Pick<RenderContext, 'pathname' | 'pipeline' | 'request' | 'routeData'> & }: Pick<RenderContext, 'pathname' | 'pipeline' | 'request' | 'routeData'> &
Partial<Pick<RenderContext, 'locals' | 'middleware' | 'status'>>): RenderContext { Partial<Pick<RenderContext, 'locals' | 'middleware' | 'status' | 'props'>>): RenderContext {
return new RenderContext( return new RenderContext(
pipeline, pipeline,
locals, locals,
@ -83,7 +84,11 @@ export class RenderContext {
pathname, pathname,
request, request,
routeData, routeData,
status status,
undefined,
undefined,
undefined,
props
); );
} }

View file

@ -230,6 +230,7 @@ export async function handleRoute({
logger.error('router', err.stack || err.message); logger.error('router', err.stack || err.message);
const filePath500 = new URL(`./${custom500.component}`, config.root); const filePath500 = new URL(`./${custom500.component}`, config.root);
const preloaded500Component = await pipeline.preload(custom500, filePath500); const preloaded500Component = await pipeline.preload(custom500, filePath500);
renderContext.props.error = err;
response = await renderContext.render(preloaded500Component); response = await renderContext.render(preloaded500Component);
status = 500; status = 500;
} }

View file

@ -0,0 +1,122 @@
import assert from 'node:assert/strict';
import { afterEach, describe, it } from 'node:test';
import * as cheerio from 'cheerio';
import { loadFixture } from './test-utils.js';
import testAdapter from './test-adapter.js';
import { renameSync } from 'node:fs';
describe('Custom 500', () => {
/** @type {Awaited<ReturnType<typeof loadFixture>>} */
let fixture;
describe('dev', () => {
/** @type {Awaited<ReturnType<(typeof fixture)["startDevServer"]>>} */
let devServer;
afterEach(async () => {
await devServer?.stop();
try {
renameSync(
new URL('./fixtures/custom-500/src/pages/_500.astro', import.meta.url),
new URL('./fixtures/custom-500/src/pages/500.astro', import.meta.url)
);
} catch (_) {}
});
it('renders default error overlay', async () => {
renameSync(
new URL('./fixtures/custom-500/src/pages/500.astro', import.meta.url),
new URL('./fixtures/custom-500/src/pages/_500.astro', import.meta.url)
);
fixture = await loadFixture({
root: './fixtures/custom-500/',
output: 'server',
adapter: testAdapter(),
});
devServer = await fixture.startDevServer();
const response = await fixture.fetch('/');
assert.equal(response.status, 500);
const html = await response.text();
assert.equal(html, '<title>Error</title><script type="module" src="/@vite/client"></script>');
});
it('renders custom 500', async () => {
fixture = await loadFixture({
root: './fixtures/custom-500/',
output: 'server',
adapter: testAdapter(),
});
devServer = await fixture.startDevServer();
const response = await fixture.fetch('/');
assert.equal(response.status, 500);
const html = await response.text();
const $ = cheerio.load(html);
assert.equal($('h1').text(), 'Server error');
assert.equal($('p').text(), 'some error');
});
it('renders default error overlay if custom 500 throws', async () => {
fixture = await loadFixture({
root: './fixtures/custom-500-failing/',
output: 'server',
adapter: testAdapter(),
});
devServer = await fixture.startDevServer();
const response = await fixture.fetch('/');
assert.equal(response.status, 500);
const html = await response.text();
assert.equal(html, '<title>Error</title><script type="module" src="/@vite/client"></script>');
});
});
describe('SSR', () => {
/** @type {Awaited<ReturnType<(typeof fixture)["loadTestAdapterApp"]>>} */
let app;
it('renders custom 500', async () => {
fixture = await loadFixture({
root: './fixtures/custom-500/',
output: 'server',
adapter: testAdapter(),
});
await fixture.build();
app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/');
const response = await app.render(request);
assert.equal(response.status, 500);
const html = await response.text();
const $ = cheerio.load(html);
assert.equal($('h1').text(), 'Server error');
assert.equal($('p').text(), 'some error');
});
it('renders nothing if custom 500 throws', async () => {
fixture = await loadFixture({
root: './fixtures/custom-500-failing/',
output: 'server',
adapter: testAdapter(),
});
await fixture.build();
app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/');
const response = await app.render(request);
assert.equal(response.status, 500);
const html = await response.text();
assert.equal(html, '');
});
});
});

View file

@ -0,0 +1,4 @@
import { defineConfig } from 'astro/config';
// https://astro.build/config
export default defineConfig({});

View file

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

View file

@ -0,0 +1,19 @@
---
interface Props {
error: unknown
}
const { error } = Astro.props
throw "custom 500 fail"
---
<html lang="en">
<head>
<title>Server error - Custom 500</title>
</head>
<body>
<h1>Server error</h1>
<p>{error}</p>
</body>
</html>

View file

@ -0,0 +1,12 @@
---
throw "some error"
---
<html lang="en">
<head>
<title>Custom 500</title>
</head>
<body>
<h1>Home</h1>
</body>
</html>

View file

@ -0,0 +1,4 @@
import { defineConfig } from 'astro/config';
// https://astro.build/config
export default defineConfig({});

View file

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

View file

@ -0,0 +1,17 @@
---
interface Props {
error: unknown
}
const { error } = Astro.props
---
<html lang="en">
<head>
<title>Server error - Custom 500</title>
</head>
<body>
<h1>Server error</h1>
<p>{error}</p>
</body>
</html>

View file

@ -0,0 +1,12 @@
---
throw "some error"
---
<html lang="en">
<head>
<title>Custom 500</title>
</head>
<body>
<h1>Home</h1>
</body>
</html>

View file

@ -2895,6 +2895,18 @@ importers:
specifier: workspace:* specifier: workspace:*
version: link:../../.. version: link:../../..
packages/astro/test/fixtures/custom-500:
dependencies:
astro:
specifier: workspace:*
version: link:../../..
packages/astro/test/fixtures/custom-500-failing:
dependencies:
astro:
specifier: workspace:*
version: link:../../..
packages/astro/test/fixtures/custom-assets-name: packages/astro/test/fixtures/custom-assets-name:
dependencies: dependencies:
'@astrojs/node': '@astrojs/node':