Skip to content

Commit

Permalink
Improve error handling (#3859)
Browse files Browse the repository at this point in the history
* feat: tag JSX exports with correct renderer

* feat(error): enhance generic errors with frame

* feat(error): surface errors from streaming response

* feat(error): use vite overlay to display errors

* chore: fix build issues

* feat: use custom logger to hide vite errors for known packages

* chore: move error-react-spectrum to e2e test

* chore: add todo comment

* test: fix error overlay handling

* refactor: extract overlay message to util

* test(e2e): update shared component tests

* fix: give error overlay more time

* refactor: move errors tests to e2e

* fix: appease ts

* test: move sass error to e2e tests

* fix: scope optimizeDeps to `src/pages/**/*`

* chore: update lockfile

* chore: update test script

* chore: log error overlay

* chore: log error tests

* chore: update playwright config

* test(e2e): update errors tests

* test(e2e): fix overlay util

* test(e2e): fix test utils

* test(e2e): try timeout

* test(e2e): give up on overlay tests

* fix: typo

* fix: typo

* refactor: collapse definition

* fix: let errors throw

* chore: revert scanner change

* chore: refactor err.plugin handling

* chore: add clarifying comments

* fix: make astro:renderer non enumerable

* chore: update comments

* refactor: replace astro:renderer string with Symbol

* chore: add comment about tagged components

* feat: improve error overlay when hint exists

Co-authored-by: Nate Moore <nate@astro.build>
  • Loading branch information
natemoo-re and natemoo-re authored Jul 19, 2022
1 parent 4ee997d commit 4412fe6
Show file tree
Hide file tree
Showing 68 changed files with 452 additions and 374 deletions.
24 changes: 24 additions & 0 deletions packages/astro/e2e/error-react-spectrum.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { expect } from '@playwright/test';
import { testFactory, getErrorOverlayMessage } from './test-utils.js';

const test = testFactory({ root: './fixtures/error-react-spectrum/' });

let devServer;

test.beforeEach(async ({ astro }) => {
devServer = await astro.startDevServer();
});

test.afterEach(async ({ astro }) => {
await devServer.stop();
astro.resetAllFiles();
});

test.describe('Error: React Spectrum', () => {
test('overlay', async ({ page, astro }) => {
await page.goto(astro.resolveUrl('/'));

const message = await getErrorOverlayMessage(page);
expect(message).toMatch('@adobe/react-spectrum is not compatible')
});
});
24 changes: 24 additions & 0 deletions packages/astro/e2e/error-sass.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { expect } from '@playwright/test';
import { testFactory, getErrorOverlayMessage } from './test-utils.js';

const test = testFactory({ root: './fixtures/error-sass/' });

let devServer;

test.beforeEach(async ({ astro }) => {
devServer = await astro.startDevServer();
});

test.afterEach(async ({ astro }) => {
await devServer.stop();
astro.resetAllFiles();
});

test.describe('Error: Sass', () => {
test('overlay', async ({ page, astro }) => {
await page.goto(astro.resolveUrl('/'));

const message = await getErrorOverlayMessage(page);
expect(message).toMatch('Undefined variable')
});
});
67 changes: 67 additions & 0 deletions packages/astro/e2e/errors.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { expect } from '@playwright/test';
import { testFactory, getErrorOverlayMessage } from './test-utils.js';

const test = testFactory({ root: './fixtures/errors/' });

let devServer;

test.beforeEach(async ({ astro }) => {
devServer = await astro.startDevServer();
});

test.afterEach(async ({ astro }) => {
await devServer.stop();
astro.resetAllFiles();
});

test.describe('Error display', () => {
test('detect syntax errors in template', async ({ page, astro }) => {
await page.goto(astro.resolveUrl('/astro-syntax-error'));

const message = await getErrorOverlayMessage(page);
expect(message).toMatch('Unexpected "}"')

await Promise.all([
// Wait for page reload
page.waitForNavigation(),
// Edit the component file
await astro.editFile('./src/pages/astro-syntax-error.astro', () => `<h1>No syntax error</h1>`)
]);

expect(await page.locator('vite-error-overlay').count()).toEqual(0);
});

test('shows useful error when frontmatter import is not found', async ({ page, astro }) => {
await page.goto(astro.resolveUrl('/import-not-found'));

const message = await getErrorOverlayMessage(page);
expect(message).toMatch('failed to load module for ssr: ../abc.astro')

await Promise.all([
// Wait for page reload
page.waitForNavigation(),
// Edit the component file
astro.editFile('./src/pages/import-not-found.astro', () => `<h1>No import error</h1>`)
]);

expect(await page.locator('vite-error-overlay').count()).toEqual(0);
});

test('framework errors recover when fixed', async ({ page, astro }) => {
await page.goto(astro.resolveUrl('/svelte-syntax-error'));

const message = await getErrorOverlayMessage(page);
expect(message).toMatch('</div> attempted to close an element that was not open')

await Promise.all([
// Wait for page reload
page.waitForNavigation(),
// Edit the component file
astro.editFile('./src/components/SvelteSyntaxError.svelte', () => `<h1>No mismatch</h1>`)
]);

expect(await page.locator('vite-error-overlay').count()).toEqual(0);
});


});
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "@test/error-react-spectrum",
"name": "@e2e/error-react-spectrum",
"version": "0.0.0",
"private": true,
"dependencies": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "@test/sass",
"name": "@e2e/error-sass",
"version": "0.0.0",
"private": true,
"dependencies": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "@test/errors",
"name": "@e2e/errors",
"version": "0.0.0",
"private": true,
"dependencies": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@

export { default } from '../components/Layout.astro';
import Counter from '../components/Counter.jsx';
import PreactComponent from '../components/JSXComponent.jsx';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export { default } from '../components/Layout.astro';
import Counter from '../components/Counter.jsx';
import ReactComponent from '../components/JSXComponent.jsx';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export { default } from '../components/Layout.astro';
import Counter from '../components/Counter.jsx';
import SolidComponent from '../components/SolidComponent.jsx';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export { default } from '../components/Layout.astro';
import Counter from '../components/Counter.svelte';
import SvelteComponent from '../components/SvelteComponent.svelte';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export { default } from '../components/Layout.astro';
import Counter from '../components/Counter.vue';
import VueComponent from '../components/VueComponent.vue';

Expand Down
9 changes: 7 additions & 2 deletions packages/astro/e2e/preact-compat-component.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,23 @@ import { prepareTestFactory } from './shared-component-tests.js';

const { test, createTests } = prepareTestFactory({ root: './fixtures/preact-compat-component/' });

const config = {
counterComponentFilePath: './src/components/Counter.jsx',
componentFilePath: './src/components/JSXComponent.jsx',
}

test.describe('preact/compat components in Astro files', () => {
createTests({
...config,
pageUrl: '/',
pageSourceFilePath: './src/pages/index.astro',
componentFilePath: './src/components/JSXComponent.jsx',
});
});

test.describe('preact/compat components in Markdown files', () => {
createTests({
...config,
pageUrl: '/markdown/',
pageSourceFilePath: './src/pages/markdown.md',
componentFilePath: './src/components/JSXComponent.jsx',
});
});
11 changes: 8 additions & 3 deletions packages/astro/e2e/preact-component.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,31 @@ import { prepareTestFactory } from './shared-component-tests.js';

const { test, createTests } = prepareTestFactory({ root: './fixtures/preact-component/' });

const config = {
counterComponentFilePath: './src/components/Counter.jsx',
componentFilePath: './src/components/JSXComponent.jsx',
}

test.describe('Preact components in Astro files', () => {
createTests({
...config,
pageUrl: '/',
pageSourceFilePath: './src/pages/index.astro',
componentFilePath: './src/components/JSXComponent.jsx',
});
});

test.describe('Preact components in Markdown files', () => {
createTests({
...config,
pageUrl: '/markdown/',
pageSourceFilePath: './src/pages/markdown.md',
componentFilePath: './src/components/JSXComponent.jsx',
});
});

test.describe('Preact components in MDX files', () => {
createTests({
...config,
pageUrl: '/mdx/',
pageSourceFilePath: './src/pages/mdx.mdx',
componentFilePath: './src/components/JSXComponent.jsx',
});
});
11 changes: 8 additions & 3 deletions packages/astro/e2e/react-component.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,31 @@ import { prepareTestFactory } from './shared-component-tests.js';

const { test, createTests } = prepareTestFactory({ root: './fixtures/react-component/' });

const config = {
counterComponentFilePath: './src/components/Counter.jsx',
componentFilePath: './src/components/JSXComponent.jsx',
}

test.describe('React components in Astro files', () => {
createTests({
...config,
pageUrl: '/',
pageSourceFilePath: './src/pages/index.astro',
componentFilePath: './src/components/JSXComponent.jsx',
});
});

test.describe('React components in Markdown files', () => {
createTests({
...config,
pageUrl: '/markdown/',
pageSourceFilePath: './src/pages/markdown.md',
componentFilePath: './src/components/JSXComponent.jsx',
});
});

test.describe('React components in MDX files', () => {
createTests({
...config,
pageUrl: '/mdx/',
pageSourceFilePath: './src/pages/mdx.mdx',
componentFilePath: './src/components/JSXComponent.jsx',
});
});
2 changes: 1 addition & 1 deletion packages/astro/e2e/shared-component-tests.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect } from '@playwright/test';
import { testFactory } from './test-utils.js';
import { testFactory, getErrorOverlayMessage } from './test-utils.js';

export function prepareTestFactory(opts) {
const test = testFactory(opts);
Expand Down
11 changes: 8 additions & 3 deletions packages/astro/e2e/solid-component.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,31 @@ import { prepareTestFactory } from './shared-component-tests.js';

const { test, createTests } = prepareTestFactory({ root: './fixtures/solid-component/' });

const config = {
componentFilePath: './src/components/SolidComponent.jsx',
counterComponentFilePath: './src/components/Counter.jsx',
}

test.describe('Solid components in Astro files', () => {
createTests({
...config,
pageUrl: '/',
pageSourceFilePath: './src/pages/index.astro',
componentFilePath: './src/components/SolidComponent.jsx',
});
});

test.describe('Solid components in Markdown files', () => {
createTests({
...config,
pageUrl: '/markdown/',
pageSourceFilePath: './src/pages/markdown.md',
componentFilePath: './src/components/SolidComponent.jsx',
});
});

test.describe('Solid components in MDX files', () => {
createTests({
...config,
pageUrl: '/mdx/',
pageSourceFilePath: './src/pages/mdx.mdx',
componentFilePath: './src/components/SolidComponent.jsx',
});
});
15 changes: 9 additions & 6 deletions packages/astro/e2e/svelte-component.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,32 @@ import { prepareTestFactory } from './shared-component-tests.js';

const { test, createTests } = prepareTestFactory({ root: './fixtures/svelte-component/' });

const config = {
componentFilePath: './src/components/SvelteComponent.svelte',
counterComponentFilePath: './src/components/Counter.svelte',
counterCssFilePath: './src/components/Counter.svelte',
}

test.describe('Svelte components in Astro files', () => {
createTests({
...config,
pageUrl: '/',
pageSourceFilePath: './src/pages/index.astro',
componentFilePath: './src/components/SvelteComponent.svelte',
counterCssFilePath: './src/components/Counter.svelte',
});
});

test.describe('Svelte components in Markdown files', () => {
createTests({
...config,
pageUrl: '/markdown/',
pageSourceFilePath: './src/pages/markdown.md',
componentFilePath: './src/components/SvelteComponent.svelte',
counterCssFilePath: './src/components/Counter.svelte',
});
});

test.describe('Svelte components in MDX files', () => {
createTests({
...config,
pageUrl: '/mdx/',
pageSourceFilePath: './src/pages/mdx.mdx',
componentFilePath: './src/components/SvelteComponent.svelte',
counterCssFilePath: './src/components/Counter.svelte',
});
});
10 changes: 9 additions & 1 deletion packages/astro/e2e/test-utils.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { test as testBase } from '@playwright/test';
import { test as testBase, expect } from '@playwright/test';
import { loadFixture as baseLoadFixture } from '../test/test-utils.js';

export function loadFixture(inlineConfig) {
Expand Down Expand Up @@ -29,3 +29,11 @@ export function testFactory(inlineConfig) {

return test;
}

export async function getErrorOverlayMessage(page) {
const overlay = await page.waitForSelector('vite-error-overlay', { strict: true, timeout: 10 * 1000 })

expect(overlay).toBeTruthy()

return await overlay.$$eval('.message-body', (m) => m[0].textContent)
}
15 changes: 9 additions & 6 deletions packages/astro/e2e/vue-component.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,32 @@ import { prepareTestFactory } from './shared-component-tests.js';

const { test, createTests } = prepareTestFactory({ root: './fixtures/vue-component/' });

const config = {
componentFilePath: './src/components/VueComponent.vue',
counterCssFilePath: './src/components/Counter.vue',
counterComponentFilePath: './src/components/Counter.vue',
}

test.describe('Vue components in Astro files', () => {
createTests({
...config,
pageUrl: '/',
pageSourceFilePath: './src/pages/index.astro',
componentFilePath: './src/components/VueComponent.vue',
counterCssFilePath: './src/components/Counter.vue',
});
});

test.describe('Vue components in Markdown files', () => {
createTests({
...config,
pageUrl: '/markdown/',
pageSourceFilePath: './src/pages/markdown.md',
componentFilePath: './src/components/VueComponent.vue',
counterCssFilePath: './src/components/Counter.vue',
});
});

test.describe('Vue components in MDX files', () => {
createTests({
...config,
pageUrl: '/mdx/',
pageSourceFilePath: './src/pages/mdx.mdx',
componentFilePath: './src/components/VueComponent.vue',
counterCssFilePath: './src/components/Counter.vue',
});
});
Loading

0 comments on commit 4412fe6

Please sign in to comment.