Skip to content

Commit

Permalink
Make error handling more resilient in the dev server (#4723)
Browse files Browse the repository at this point in the history
* Make error handling more resilient in the dev server

* Better approach

* Add a changeset
  • Loading branch information
matthewp committed Sep 12, 2022
1 parent 562147a commit 0dba3b6
Show file tree
Hide file tree
Showing 10 changed files with 96 additions and 10 deletions.
5 changes: 5 additions & 0 deletions .changeset/orange-cycles-serve.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Makes the dev server more resilient to crashes
23 changes: 14 additions & 9 deletions packages/astro/src/core/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ export function cleanErrorStack(stack: string) {
export function fixViteErrorMessage(_err: unknown, server?: ViteDevServer, filePath?: URL) {
const err = createSafeError(_err);
// Vite will give you better stacktraces, using sourcemaps.
server?.ssrFixStacktrace(err);
try {
server?.ssrFixStacktrace(err);
} catch {}

// Fix: Astro.glob() compiles to import.meta.glob() by the time Vite sees it,
// so we need to update this error message in case it originally came from Astro.glob().
if (err.message === 'import.meta.glob() can only accept string literals.') {
Expand All @@ -57,14 +60,16 @@ export function fixViteErrorMessage(_err: unknown, server?: ViteDevServer, fileP
if (filePath && /failed to load module for ssr:/.test(err.message)) {
const importName = err.message.split('for ssr:').at(1)?.trim();
if (importName) {
const content = fs.readFileSync(fileURLToPath(filePath)).toString();
const lns = content.split('\n');
const line = lns.findIndex((ln) => ln.includes(importName));
if (line == -1) return err;
const column = lns[line]?.indexOf(importName);
if (!(err as any).id) {
(err as any).id = `${fileURLToPath(filePath)}:${line + 1}:${column + 1}`;
}
try {
const content = fs.readFileSync(fileURLToPath(filePath)).toString();
const lns = content.split('\n');
const line = lns.findIndex((ln) => ln.includes(importName));
if (line == -1) return err;
const column = lns[line]?.indexOf(importName);
if (!(err as any).id) {
(err as any).id = `${fileURLToPath(filePath)}:${line + 1}:${column + 1}`;
}
} catch {}
}
}
return err;
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/vite-plugin-astro-server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ async function handleRequest(
return await writeSSRResult(result, res);
}
} catch (_err) {
const err = fixViteErrorMessage(createSafeError(_err), viteServer, filePath);
const err = fixViteErrorMessage(_err, viteServer, filePath);
const errorWithMetadata = collectErrorMetadata(err);
error(logging, null, msg.formatErrorMessage(errorWithMetadata));
handle500Response(viteServer, origin, req, res, errorWithMetadata);
Expand Down
34 changes: 34 additions & 0 deletions packages/astro/test/error-bad-js.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { expect } from 'chai';
import * as cheerio from 'cheerio';
import { loadFixture } from './test-utils.js';

describe('Errors in JavaScript', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;

/** @type {import('./test-utils').DevServer} */
let devServer;

before(async () => {
fixture = await loadFixture({
root: './fixtures/error-bad-js',
});
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('ReferenceError');

res = await fixture.fetch('/');
await res.text();

expect(html).to.include('ReferenceError');
});
});
3 changes: 3 additions & 0 deletions packages/astro/test/fixtures/error-bad-js/astro.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { defineConfig } from 'astro/config';

export default defineConfig({});
15 changes: 15 additions & 0 deletions packages/astro/test/fixtures/error-bad-js/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"name": "@test/error-bad-js",
"version": "0.0.1",
"private": true,
"scripts": {
"dev": "astro dev",
"start": "astro dev",
"build": "astro build",
"preview": "astro preview",
"astro": "astro"
},
"dependencies": {
"astro": "workspace:*"
}
}
1 change: 1 addition & 0 deletions packages/astro/test/fixtures/error-bad-js/src/env.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/// <reference types="astro/client" />
16 changes: 16 additions & 0 deletions packages/astro/test/fixtures/error-bad-js/src/pages/index.astro
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
import something from '../something.js';
---

<html lang="en">
<head>
<meta charset="utf-8" />
<link rel="icon" type="image/svg+xml" href="/favicon.svg" />
<meta name="viewport" content="width=device-width" />
<meta name="generator" content={Astro.generator} />
<title>Astro</title>
</head>
<body>
<h1>Astro</h1>
</body>
</html>
1 change: 1 addition & 0 deletions packages/astro/test/fixtures/error-bad-js/src/something.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export var foo = bar;
6 changes: 6 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 0dba3b6

Please sign in to comment.