From 7fc5d559521b4b347d8225ec49d63f5419d3a980 Mon Sep 17 00:00:00 2001 From: Erika <3019731+Princesseuh@users.noreply.github.com> Date: Thu, 27 Oct 2022 15:44:48 -0300 Subject: [PATCH] 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 | 2 +- packages/astro/src/core/errors/index.ts | 2 +- packages/astro/src/core/errors/utils.ts | 7 ++++ packages/astro/src/core/util.ts | 7 ---- .../src/vite-plugin-astro-server/index.ts | 4 ++- .../vite-style-transform/style-transform.ts | 2 +- packages/astro/test/error-non-error.test.js | 33 +++++++++++++++++++ .../fixtures/error-non-error/astro.config.mjs | 3 ++ .../fixtures/error-non-error/package.json | 15 +++++++++ .../fixtures/error-non-error/src/env.d.ts | 1 + .../error-non-error/src/pages/index.astro | 3 ++ pnpm-lock.yaml | 6 ++++ 12 files changed, 74 insertions(+), 11 deletions(-) create mode 100644 packages/astro/test/error-non-error.test.js create mode 100644 packages/astro/test/fixtures/error-non-error/astro.config.mjs create mode 100644 packages/astro/test/fixtures/error-non-error/package.json create mode 100644 packages/astro/test/fixtures/error-non-error/src/env.d.ts create mode 100644 packages/astro/test/fixtures/error-non-error/src/pages/index.astro diff --git a/packages/astro/src/cli/index.ts b/packages/astro/src/cli/index.ts index a8ead6707..ceeb1a915 100644 --- a/packages/astro/src/cli/index.ts +++ b/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'; diff --git a/packages/astro/src/core/errors/index.ts b/packages/astro/src/core/errors/index.ts index 7eefbc824..9ebf0e8a6 100644 --- a/packages/astro/src/core/errors/index.ts +++ b/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'; diff --git a/packages/astro/src/core/errors/utils.ts b/packages/astro/src/core/errors/utils.ts index 9999b81b4..531e2941f 100644 --- a/packages/astro/src/core/errors/utils.ts +++ b/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)); +} diff --git a/packages/astro/src/core/util.ts b/packages/astro/src/core/util.ts index da51d848c..dbfe1ad35 100644 --- a/packages/astro/src/core/util.ts +++ b/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), diff --git a/packages/astro/src/vite-plugin-astro-server/index.ts b/packages/astro/src/vite-plugin-astro-server/index.ts index 023c9dd77..1666c453f 100644 --- a/packages/astro/src/vite-plugin-astro-server/index.ts +++ b/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); diff --git a/packages/astro/src/vite-style-transform/style-transform.ts b/packages/astro/src/vite-style-transform/style-transform.ts index ffa3ea57d..c8dfbd3f0 100644 --- a/packages/astro/src/vite-style-transform/style-transform.ts +++ b/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; diff --git a/packages/astro/test/error-non-error.test.js b/packages/astro/test/error-non-error.test.js new file mode 100644 index 000000000..facf99633 --- /dev/null +++ b/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'); + }); +}); diff --git a/packages/astro/test/fixtures/error-non-error/astro.config.mjs b/packages/astro/test/fixtures/error-non-error/astro.config.mjs new file mode 100644 index 000000000..86dbfb924 --- /dev/null +++ b/packages/astro/test/fixtures/error-non-error/astro.config.mjs @@ -0,0 +1,3 @@ +import { defineConfig } from 'astro/config'; + +export default defineConfig({}); diff --git a/packages/astro/test/fixtures/error-non-error/package.json b/packages/astro/test/fixtures/error-non-error/package.json new file mode 100644 index 000000000..2366e345e --- /dev/null +++ b/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:*" + } +} diff --git a/packages/astro/test/fixtures/error-non-error/src/env.d.ts b/packages/astro/test/fixtures/error-non-error/src/env.d.ts new file mode 100644 index 000000000..f964fe0cf --- /dev/null +++ b/packages/astro/test/fixtures/error-non-error/src/env.d.ts @@ -0,0 +1 @@ +/// diff --git a/packages/astro/test/fixtures/error-non-error/src/pages/index.astro b/packages/astro/test/fixtures/error-non-error/src/pages/index.astro new file mode 100644 index 000000000..5697b7424 --- /dev/null +++ b/packages/astro/test/fixtures/error-non-error/src/pages/index.astro @@ -0,0 +1,3 @@ +--- +throw {} +--- diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1535ff301..9a8638cae 100644 --- a/pnpm-lock.yaml +++ b/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:*