Skip to content

Commit

Permalink
Refactor error handling (#5206)
Browse files Browse the repository at this point in the history
* Refactor error handling

* Fix import path in test

* Revert "Fix import path in test"

This reverts commit 5ca34f3.

* Fix import path in test

* Fix AggregateErrors actually not being.. an aggregration of errors

* Fix missing info in Vite enhanced error

* Conserve original error name if we have one

* Workaround compiler issue

* GitHub action please

* Update E2E test

* Wrap ssrFixStacktrace in try/catch

* Refactor Vite/Node methods out of the general index.ts

* Fix missing import

* Add changeset
  • Loading branch information
Princesseuh committed Oct 27, 2022
1 parent 3c1af36 commit d64d5b9
Show file tree
Hide file tree
Showing 26 changed files with 744 additions and 307 deletions.
5 changes: 5 additions & 0 deletions .changeset/ten-cheetahs-perform.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Improve error messages related to CSS and compiler errors
2 changes: 1 addition & 1 deletion packages/astro/e2e/errors.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ test.describe('Error display', () => {
await page.goto(astro.resolveUrl('/import-not-found'));

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

await Promise.all([
// Wait for page reload
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
} from '../core/config/index.js';
import { ASTRO_VERSION } from '../core/constants.js';
import devServer from '../core/dev/index.js';
import { collectErrorMetadata } from '../core/errors.js';
import { collectErrorMetadata } from '../core/errors/dev/index.js';
import { debug, error, info, LogOptions } from '../core/logger/core.js';
import { enableVerboseLogging, nodeLogDestination } from '../core/logger/node.js';
import { formatConfigErrorMessage, formatErrorMessage, printHelp } from '../core/messages.js';
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/src/core/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
runHookConfigSetup,
} from '../../integrations/index.js';
import { createVite } from '../create-vite.js';
import { fixViteErrorMessage } from '../errors.js';
import { enhanceViteSSRError } from '../errors/dev/index.js';
import { debug, info, levels, timerMessage } from '../logger/core.js';
import { apply as applyPolyfill } from '../polyfill.js';
import { RouteCache } from '../render/route-cache.js';
Expand Down Expand Up @@ -169,7 +169,7 @@ class AstroBuilder {
try {
await this.build(setupData);
} catch (_err) {
throw fixViteErrorMessage(_err);
throw enhanceViteSSRError(_err as Error);
}
}

Expand Down
50 changes: 38 additions & 12 deletions packages/astro/src/core/compile/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ import type { AstroConfig } from '../../@types/astro';
import type { TransformStyle } from './types';

import { transform } from '@astrojs/compiler';
import { AstroErrorCodes } from '../errors.js';
import { AstroErrorCodes } from '../errors/codes.js';
import { AggregateError, AstroError, CompilerError } from '../errors/errors.js';
import { prependForwardSlash } from '../path.js';
import { AggregateError, resolvePath, viteID } from '../util.js';
import { resolvePath, viteID } from '../util.js';
import { createStylePreprocessor } from './style.js';

type CompilationCache = Map<string, CompileResult>;
Expand All @@ -30,7 +31,7 @@ async function compile({
transformStyle,
}: CompileProps): Promise<CompileResult> {
let cssDeps = new Set<string>();
let cssTransformErrors: Error[] = [];
let cssTransformErrors: AstroError[] = [];

// Transform from `.astro` to valid `.ts`
// use `sourcemap: "both"` so that sourcemap is included in the code
Expand All @@ -51,26 +52,51 @@ async function compile({
return resolvePath(specifier, filename);
},
})
.catch((err) => {
// throw compiler errors here if encountered
err.code = err.code || AstroErrorCodes.UnknownCompilerError;
throw err;
.catch((err: Error) => {
// The compiler should be able to handle errors by itself, however
// for the rare cases where it can't let's directly throw here with as much info as possible
throw new CompilerError({
errorCode: AstroErrorCodes.UnknownCompilerError,
message: err.message ?? 'Unknown compiler error',
stack: err.stack,
location: {
file: filename,
},
});
})
.then((result) => {
const compilerError = result.diagnostics.find(
// HACK: The compiler currently mistakenly returns the wrong severity for warnings, so we'll also filter by code
// https://github.com/withastro/compiler/issues/595
(diag) => diag.severity === 1 && diag.code < 2000
);

if (compilerError) {
throw new CompilerError({
errorCode: compilerError.code,
message: compilerError.text,
location: {
line: compilerError.location.line,
column: compilerError.location.column,
file: compilerError.location.file,
},
hint: compilerError.hint ? compilerError.hint : undefined,
});
}

switch (cssTransformErrors.length) {
case 0:
return result;
case 1: {
let error = cssTransformErrors[0];
if (!(error as any).code) {
(error as any).code = AstroErrorCodes.UnknownCompilerCSSError;
if (!error.errorCode) {
error.errorCode = AstroErrorCodes.UnknownCompilerCSSError;
}

throw cssTransformErrors[0];
}
default: {
const aggregateError = new AggregateError(cssTransformErrors);
(aggregateError as any).code = AstroErrorCodes.UnknownCompilerCSSError;
throw aggregateError;
throw new AggregateError({ ...cssTransformErrors[0], errors: cssTransformErrors });
}
}
});
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/create-vite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import legacyMarkdownVitePlugin from '../vite-plugin-markdown-legacy/index.js';
import markdownVitePlugin from '../vite-plugin-markdown/index.js';
import astroScriptsPlugin from '../vite-plugin-scripts/index.js';
import astroScriptsPageSSRPlugin from '../vite-plugin-scripts/page-ssr.js';
import { createCustomViteLogger } from './errors.js';
import { createCustomViteLogger } from './errors/dev/index.js';
import { resolveDependency } from './util.js';

interface CreateViteOptions {
Expand Down
209 changes: 0 additions & 209 deletions packages/astro/src/core/errors.ts

This file was deleted.

24 changes: 24 additions & 0 deletions packages/astro/src/core/errors/codes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
export enum AstroErrorCodes {
// 1xxx are reserved for compiler errors
StaticRedirectNotAllowed = 2005,
UnavailableInSSR = 2006,
// Runtime errors
GenericRuntimeError = 3000,
// PostCSS errors
CssSyntaxError = 4000,
CssUnknownError = 4001,
// Vite SSR errors
FailedToLoadModuleSSR = 5000,
// Config Errors
ConfigError = 6000,

// Markdown Errors
GenericMarkdownError = 7000,
MarkdownFrontmatterParseError = 7001,

// General catch-alls for cases where we have zero information
UnknownCompilerError = 9000,
UnknownCompilerCSSError = 9001,
UnknownViteSSRError = 9002,
UnknownError = 9999,
}
2 changes: 2 additions & 0 deletions packages/astro/src/core/errors/dev/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export { collectErrorMetadata } from './utils.js';
export { createCustomViteLogger, enhanceViteSSRError, getViteErrorPayload } from './vite.js';
Loading

0 comments on commit d64d5b9

Please sign in to comment.