Fix errors that are not instanceof Error crashing the dev server (#5216)

* Fix errors that are not instanceof Error crashing the dev server

* Update lockfile

* Fix import paths

packages/astro/src/cli/index.ts

@@ -22,7 +22,7 @@ import { enableVerboseLogging, nodeLogDestination } from '../core/logger/node.js
 import { formatConfigErrorMessage, formatErrorMessage, printHelp } from '../core/messages.js';
 import { appendForwardSlash } from '../core/path.js';
 import preview from '../core/preview/index.js';
-import { createSafeError } from '../core/util.js';
+import { createSafeError } from '../core/errors/index.js';
 import * as event from '../events/index.js';
 import { eventConfigError, eventError, telemetry } from '../events/index.js';
 import { check } from './check/index.js';

packages/astro/src/core/errors/index.ts

@@ -9,4 +9,4 @@ export {
 	RuntimeError,
 } from './errors.js';
 export { codeFrame } from './printer.js';
-export { collectInfoFromStacktrace, positionAt } from './utils.js';
+export { collectInfoFromStacktrace, positionAt, createSafeError } from './utils.js';

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

@@ -118,3 +118,10 @@ function getLineOffsets(text: string) {
 
 	return lineOffsets;
 }
+
+/** Coalesce any throw variable to an Error instance. */
+export function createSafeError(err: any): Error {
+	return err instanceof Error || (err && err.name && err.message)
+		? err
+		: new Error(JSON.stringify(err));
+}

packages/astro/src/core/util.ts

@@ -91,13 +91,6 @@ export function parseNpmName(
 	};
 }
 
-/** Coalesce any throw variable to an Error instance. */
-export function createSafeError(err: any): Error {
-	return err instanceof Error || (err && err.name && err.message)
-		? err
-		: new Error(JSON.stringify(err));
-}
-
 export function resolveDependency(dep: string, projectRoot: URL) {
 	const resolved = resolve.sync(dep, {
 		basedir: fileURLToPath(projectRoot),

packages/astro/src/vite-plugin-astro-server/index.ts

@@ -18,6 +18,7 @@ import { createRequest } from '../core/request.js';
 import { createRouteManifest, matchAllRoutes } from '../core/routing/index.js';
 import { resolvePages } from '../core/util.js';
 import notFoundTemplate, { subpathNotUsedTemplate } from '../template/4xx.js';
+import { createSafeError } from '../core/errors/index.js';
 
 interface AstroPluginOptions {
 	settings: AstroSettings;
@@ -284,7 +285,8 @@ async function handleRequest(
 	} catch (_err) {
 		// This is our last line of defense regarding errors where we still might have some information about the request
 		// Our error should already be complete, but let's try to add a bit more through some guesswork
-		const errorWithMetadata = collectErrorMetadata(_err);
+		const err = createSafeError(_err);
+		const errorWithMetadata = collectErrorMetadata(err);
 
 		error(env.logging, null, msg.formatErrorMessage(errorWithMetadata));
 		handle500Response(viteServer, origin, req, res, errorWithMetadata);

packages/astro/src/vite-style-transform/style-transform.ts

@@ -7,7 +7,7 @@ import { readFileSync } from 'fs';
 import type * as vite from 'vite';
 import { AstroErrorCodes } from '../core/errors/codes.js';
 import { CSSError } from '../core/errors/errors.js';
-import { positionAt } from '../core/errors/utils.js';
+import { positionAt } from '../core/errors/index.js';
 
 export type ViteStyleTransformer = {
 	viteDevServer?: vite.ViteDevServer;

packages/astro/test/error-non-error.test.js

@@ -0,0 +1,33 @@
+import { expect } from 'chai';
+import { loadFixture } from './test-utils.js';
+
+describe('Can handle errors that are not instanceof Error', () => {
+	/** @type {import('./test-utils').Fixture} */
+	let fixture;
+
+	/** @type {import('./test-utils').DevServer} */
+	let devServer;
+
+	before(async () => {
+		fixture = await loadFixture({
+			root: './fixtures/error-non-error',
+		});
+		devServer = await fixture.startDevServer();
+	});
+
+	after(async () => {
+		await devServer.stop();
+	});
+
+	it('Does not crash the dev server', async () => {
+		let res = await fixture.fetch('/');
+		let html = await res.text();
+
+		expect(html).to.include('Error');
+
+		res = await fixture.fetch('/');
+		await res.text();
+
+		expect(html).to.include('Error');
+	});
+});

packages/astro/test/fixtures/error-non-error/astro.config.mjs

@@ -0,0 +1,3 @@
+import { defineConfig } from 'astro/config';
+
+export default defineConfig({});

packages/astro/test/fixtures/error-non-error/package.json

@@ -0,0 +1,15 @@
+{
+  "name": "@test/error-non-error",
+  "version": "0.0.1",
+  "private": true,
+  "scripts": {
+    "dev": "astro dev",
+    "start": "astro dev",
+    "build": "astro build",
+    "preview": "astro preview",
+    "astro": "astro"
+  },
+  "dependencies": {
+    "astro": "workspace:*"
+  }
+}

packages/astro/test/fixtures/error-non-error/src/env.d.ts

@@ -0,0 +1 @@
+/// <reference types="astro/client" />

packages/astro/test/fixtures/error-non-error/src/pages/index.astro

@@ -0,0 +1,3 @@
+---
+throw {}
+---

pnpm-lock.yaml

@@ -1700,6 +1700,12 @@ importers:
     dependencies:
       astro: link:../../..
 
+  packages/astro/test/fixtures/error-non-error:
+    specifiers:
+      astro: workspace:*
+    dependencies:
+      astro: link:../../..
+
   packages/astro/test/fixtures/fetch:
     specifiers:
       '@astrojs/preact': workspace:*