Add a new error overlay (#5495)

* Add new overlay

* Fix CSS errors missing the proper stacktrace

* Fix names not working in some cases

* Add changeset

* Fix Playwright not detecting the overlay

* Update E2E test

* Fix tests

* Small refactor, fix syntax highlight on light mode, fix code element showing even with no code

* Simplier injection

* Add Markdown support to CLI reporting

* Fix not being able to navigate with the keyboard to the open in editor link

* aria-hide some svgs (#5508)

we should also make the "open in editor" button a button, not a link, if we are using JS for interactions

* Implement close method so Vite can close the overlay when needed

* Fix filepaths not being absolute when coming from node_modules for errors

* Fix multi line errors with indentation not showing correctly

* Fix entire page being scrolled to the error line in certain cases

* Update docs links

* Put the new error overlay behind a flag

* add flag for e2e tests

Co-authored-by: Caleb Jasik <calebjasik@jasik.xyz>
Co-authored-by: Matthew Phillips <matthew@skypack.dev>

.changeset/happy-chefs-ring.md

@@ -0,0 +1,5 @@
+---
+'astro': minor
+---
+
+Add a new error overlay designed by @doodlemarks! This new overlay should be much more informative, clearer, astro-y, and prettier than the previous one.

packages/astro/e2e/error-cyclic.test.js

@@ -1,7 +1,10 @@
 import { expect } from '@playwright/test';
-import { testFactory, getErrorOverlayMessage } from './test-utils.js';
+import { testFactory, getErrorOverlayContent } from './test-utils.js';
 
-const test = testFactory({ root: './fixtures/error-cyclic/' });
+const test = testFactory({
+	experimentalErrorOverlay: true,
+	root: './fixtures/error-cyclic/'
+});
 
 let devServer;
 
@@ -18,7 +21,7 @@ test.describe('Error: Cyclic Reference', () => {
 	test('overlay', async ({ page, astro }) => {
 		await page.goto(astro.resolveUrl('/'));
 
-		const message = await getErrorOverlayMessage(page);
+		const message = (await getErrorOverlayContent(page)).message;
 		expect(message).toMatch('Cyclic reference');
 	});
 });

packages/astro/e2e/error-react-spectrum.test.js

@@ -1,7 +1,10 @@
 import { expect } from '@playwright/test';
-import { testFactory, getErrorOverlayMessage } from './test-utils.js';
+import { testFactory, getErrorOverlayContent } from './test-utils.js';
 
-const test = testFactory({ root: './fixtures/error-react-spectrum/' });
+const test = testFactory({
+	experimentalErrorOverlay: true,
+	root: './fixtures/error-react-spectrum/'
+});
 
 let devServer;
 
@@ -17,7 +20,7 @@ test.describe('Error: React Spectrum', () => {
 	test('overlay', async ({ page, astro }) => {
 		await page.goto(astro.resolveUrl('/'));
 
-		const message = await getErrorOverlayMessage(page);
+		const message = (await getErrorOverlayContent(page)).hint;
 		expect(message).toMatch('@adobe/react-spectrum is not compatible');
 	});
 });

packages/astro/e2e/error-sass.test.js

@@ -1,7 +1,10 @@
 import { expect } from '@playwright/test';
-import { testFactory, getErrorOverlayMessage } from './test-utils.js';
+import { testFactory, getErrorOverlayContent } from './test-utils.js';
 
-const test = testFactory({ root: './fixtures/error-sass/' });
+const test = testFactory({
+	experimentalErrorOverlay: true,
+	root: './fixtures/error-sass/'
+});
 
 let devServer;
 
@@ -18,7 +21,7 @@ test.describe('Error: Sass', () => {
 	test('overlay', async ({ page, astro }) => {
 		await page.goto(astro.resolveUrl('/'));
 
-		const message = await getErrorOverlayMessage(page);
+		const message = (await getErrorOverlayContent(page)).message;
 		expect(message).toMatch('Undefined variable');
 	});
 });

packages/astro/e2e/errors.test.js

@@ -1,7 +1,10 @@
 import { expect } from '@playwright/test';
-import { getErrorOverlayMessage, testFactory } from './test-utils.js';
+import { getErrorOverlayContent, testFactory } from './test-utils.js';
 
-const test = testFactory({ root: './fixtures/errors/' });
+const test = testFactory({
+	experimentalErrorOverlay: true,
+	root: './fixtures/errors/'
+});
 
 let devServer;
 
@@ -18,7 +21,7 @@ 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);
+		const message = (await getErrorOverlayContent(page)).message;
 		expect(message).toMatch('Unexpected "}"');
 
 		await Promise.all([
@@ -37,10 +40,8 @@ test.describe('Error display', () => {
 	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(
-			'Could not import `../abc.astro`.\n\nThis is often caused by a typo in the import path. Please make sure the file exists.'
-		);
+		const message = (await getErrorOverlayContent(page)).message;
+		expect(message).toMatch('Could not import ../abc.astro');
 
 		await Promise.all([
 			// Wait for page reload
@@ -55,7 +56,7 @@ test.describe('Error display', () => {
 	test('framework errors recover when fixed', async ({ page, astro }) => {
 		await page.goto(astro.resolveUrl('/svelte-syntax-error'));
 
-		const message = await getErrorOverlayMessage(page);
+		const message = (await getErrorOverlayContent(page)).message;
 		expect(message).toMatch('</div> attempted to close an element that was not open');
 
 		await Promise.all([

packages/astro/e2e/shared-component-tests.js

@@ -1,5 +1,5 @@
 import { expect } from '@playwright/test';
-import { testFactory, getErrorOverlayMessage } from './test-utils.js';
+import { testFactory } from './test-utils.js';
 
 export function prepareTestFactory(opts) {
 	const test = testFactory(opts);

packages/astro/e2e/test-utils.js

@@ -32,15 +32,23 @@ export function testFactory(inlineConfig) {
 	return test;
 }
 
-export async function getErrorOverlayMessage(page) {
+/**
+ *
+ * @param {string} page
+ * @returns {Promise<{message: string, hint: string}>}
+ */
+export async function getErrorOverlayContent(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);
+	const message = await overlay.$$eval('#message-content', (m) => m[0].textContent);
+	const hint = await overlay.$$eval('#hint-content', (m) => m[0].textContent);
+
+	return { message, hint };
 }
 
 /**

packages/astro/src/@types/astro.ts

@@ -82,6 +82,7 @@ export interface CLIFlags {
 	port?: number;
 	config?: string;
 	drafts?: boolean;
+	experimentalErrorOverlay?: boolean;
 }
 
 export interface BuildConfig {
@@ -894,6 +895,12 @@ export interface AstroUserConfig {
 		astroFlavoredMarkdown?: boolean;
 	};
 
+	/**
+	 * @hidden
+	 * Turn on experimental support for the new error overlay component.
+	 */
+	experimentalErrorOverlay?: boolean;
+
 	// Legacy options to be removed
 
 	/** @deprecated - Use "integrations" instead. Run Astro to learn more about migrating. */

packages/astro/src/core/compile/style.ts

@@ -62,6 +62,7 @@ function enhanceCSSError(err: any, filename: string) {
 				line: errorLine,
 				column: err.column,
 			},
+			stack: err.stack,
 		});
 	}
 
@@ -78,6 +79,7 @@ function enhanceCSSError(err: any, filename: string) {
 				column: err.column,
 			},
 			frame: err.frame,
+			stack: err.stack,
 		});
 	}
 
@@ -94,5 +96,6 @@ function enhanceCSSError(err: any, filename: string) {
 			column: 0,
 		},
 		frame: err.frame,
+		stack: err.stack,
 	});
 }

packages/astro/src/core/config/config.ts

@@ -100,6 +100,7 @@ export function resolveFlags(flags: Partial<Flags>): CLIFlags {
 		host:
 			typeof flags.host === 'string' || typeof flags.host === 'boolean' ? flags.host : undefined,
 		drafts: typeof flags.drafts === 'boolean' ? flags.drafts : undefined,
+		experimentalErrorOverlay: typeof flags.experimentalErrorOverlay === 'boolean' ? flags.experimentalErrorOverlay : undefined,
 	};
 }
 
@@ -127,6 +128,7 @@ function mergeCLIFlags(astroConfig: AstroUserConfig, flags: CLIFlags, cmd: strin
 		// TODO: Come back here and refactor to remove this expected error.
 		astroConfig.server.host = flags.host;
 	}
+	astroConfig.experimentalErrorOverlay = flags.experimentalErrorOverlay ?? false;
 	return astroConfig;
 }
 

packages/astro/src/core/config/schema.ts

@@ -47,6 +47,7 @@ const ASTRO_CONFIG_DEFAULTS: AstroUserConfig & any = {
 	legacy: {
 		astroFlavoredMarkdown: false,
 	},
+	experimentalErrorOverlay: false,
 };
 
 export const AstroConfigSchema = z.object({
@@ -196,6 +197,7 @@ export const AstroConfigSchema = z.object({
 		})
 		.optional()
 		.default({}),
+	experimentalErrorOverlay: z.boolean().optional().default(false),
 });
 
 interface PostCSSConfigResult {

packages/astro/src/core/errors/dev/utils.ts

@@ -1,12 +1,15 @@
 import * as fs from 'node:fs';
-import { join } from 'node:path';
+import { isAbsolute, join } from 'node:path';
 import { fileURLToPath } from 'node:url';
 import stripAnsi from 'strip-ansi';
+import { escape } from 'html-escaper';
+import type { BuildResult } from 'esbuild';
 import type { ESBuildTransformResult } from 'vite';
 import type { SSRError } from '../../../@types/astro.js';
 import { AggregateError, ErrorWithMetadata } from '../errors.js';
 import { codeFrame } from '../printer.js';
 import { normalizeLF } from '../utils.js';
+import { bold, underline } from 'kleur/colors';
 
 type EsbuildMessage = ESBuildTransformResult['warnings'][number];
 
@@ -30,30 +33,49 @@ export function collectErrorMetadata(e: any, rootFolder?: URL | undefined): Erro
 			error = collectInfoFromStacktrace(e);
 		}
 
-		if (error.loc?.file && rootFolder && !error.loc.file.startsWith('/')) {
+		// Make sure the file location is absolute, otherwise:
+		// - It won't be clickable in the terminal
+		// - We'll fail to show the file's content in the browser
+		// - We'll fail to show the code frame in the terminal
+		// - The "Open in Editor" button won't work
+		if (
+			error.loc?.file &&
+			rootFolder &&
+			(!error.loc.file.startsWith(rootFolder.pathname) || !isAbsolute(error.loc.file))
+		) {
 			error.loc.file = join(fileURLToPath(rootFolder), error.loc.file);
 		}
 
 		// If we don't have a frame, but we have a location let's try making up a frame for it
-		if (!error.frame && error.loc) {
+		if (error.loc && (!error.frame || !error.fullCode)) {
 			try {
 				const fileContents = fs.readFileSync(error.loc.file!, 'utf8');
-				const frame = codeFrame(fileContents, error.loc);
-				error.frame = frame;
+
+				if (!error.frame) {
+					const frame = codeFrame(fileContents, error.loc);
+					error.frame = frame;
+				}
+
+				if (!error.fullCode) {
+					error.fullCode = fileContents;
+				}
 			} catch {}
 		}
 
 		// Generic error (probably from Vite, and already formatted)
 		error.hint = generateHint(e);
 	});
 
-	// If we received an array of errors and it's not from us, it should be from ESBuild, try to extract info for Vite to display
+	// If we received an array of errors and it's not from us, it's most likely from ESBuild, try to extract info for Vite to display
+	// NOTE: We still need to be defensive here, because it might not necessarily be from ESBuild, it's just fairly likely.
 	if (!AggregateError.is(e) && Array.isArray((e as any).errors)) {
 		(e.errors as EsbuildMessage[]).forEach((buildError, i) => {
 			const { location, pluginName, text } = buildError;
 
 			// ESBuild can give us a slightly better error message than the one in the error, so let's use it
-			err[i].message = text;
+			if (text) {
+				err[i].message = text;
+			}
 
 			if (location) {
 				err[i].loc = { file: location.file, line: location.line, column: location.column };
@@ -71,13 +93,17 @@ export function collectErrorMetadata(e: any, rootFolder?: URL | undefined): Erro
 				}
 			}
 
-			const possibleFilePath = err[i].pluginCode || err[i].id || location?.file;
-			if (possibleFilePath && !err[i].frame) {
+			const possibleFilePath = location?.file ?? err[i].id;
+			if (possibleFilePath && err[i].loc && (!err[i].frame || !err[i].fullCode)) {
 				try {
 					const fileContents = fs.readFileSync(possibleFilePath, 'utf8');
-					err[i].frame = codeFrame(fileContents, { ...err[i].loc, file: possibleFilePath });
+					if (!err[i].frame) {
+						err[i].frame = codeFrame(fileContents, { ...err[i].loc, file: possibleFilePath });
+					}
+
+					err[i].fullCode = fileContents;
 				} catch {
-					// do nothing, code frame isn't that big a deal
+					err[i].fullCode = err[i].pluginCode;
 				}
 			}
 
@@ -94,15 +120,17 @@ export function collectErrorMetadata(e: any, rootFolder?: URL | undefined): Erro
 }
 
 function generateHint(err: ErrorWithMetadata): string | undefined {
+	const commonBrowserAPIs = ['document', 'window'];
+
 	if (/Unknown file extension \"\.(jsx|vue|svelte|astro|css)\" for /.test(err.message)) {
 		return 'You likely need to add this package to `vite.ssr.noExternal` in your astro config file.';
-	} else if (err.toString().includes('document')) {
+	} else if (commonBrowserAPIs.some((api) => err.toString().includes(api))) {
 		const hint = `Browser APIs are not available on the server.
 
 ${
 	err.loc?.file?.endsWith('.astro')
-		? 'Move your code to a <script> tag outside of the frontmatter, so the code runs on the client'
-		: 'If the code is in a framework component, try to access these objects after rendering using lifecycle methods or use a `client:only` directive to make the component exclusively run on the client'
+		? 'Move your code to a <script> tag outside of the frontmatter, so the code runs on the client.'
+		: 'If the code is in a framework component, try to access these objects after rendering using lifecycle methods or use a `client:only` directive to make the component exclusively run on the client.'
 }
 
 See https://docs.astro.build/en/guides/troubleshooting/#document-or-window-is-not-defined for more information.
@@ -173,3 +201,26 @@ function cleanErrorStack(stack: string) {
 		.map((l) => l.replace(/\/@fs\//g, '/'))
 		.join('\n');
 }
+
+/**
+ * Render a subset of Markdown to HTML or a CLI output
+ */
+export function renderErrorMarkdown(markdown: string, target: 'html' | 'cli') {
+	const linkRegex = /\[(.+)\]\((.+)\)/gm;
+	const boldRegex = /\*\*(.+)\*\*/gm;
+	const urlRegex = / (\b(https?|ftp):\/\/[-A-Z0-9+&@#\\/%?=~_|!:,.;]*[-A-Z0-9+&@#\\/%=~_|]) /gim;
+	const codeRegex = /`([^`]+)`/gim;
+
+	if (target === 'html') {
+		return escape(markdown)
+			.replace(linkRegex, `<a href="$2" target="_blank">$1</a>`)
+			.replace(boldRegex, '<b>$1</b>')
+			.replace(urlRegex, ' <a href="$1" target="_blank">$1</a> ')
+			.replace(codeRegex, '<code>$1</code>');
+	} else {
+		return markdown
+			.replace(linkRegex, (fullMatch, m1, m2) => `${bold(m1)} ${underline(m2)}`)
+			.replace(urlRegex, (fullMatch, m1) => ` ${underline(fullMatch.trim())} `)
+			.replace(boldRegex, (fullMatch, m1) => `${bold(m1)}`);
+	}
+}