Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[next] Update error logging #9129

Merged
merged 2 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/three-chairs-sip.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': minor
---

Update error log formatting
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We like to make these changesets a paragraph or two, like what would go in a blog post.

2 changes: 1 addition & 1 deletion packages/astro/src/cli/throw-and-exit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export async function throwAndExit(cmd: string, err: unknown) {

const errorWithMetadata = collectErrorMetadata(createSafeError(err));
telemetryPromise = telemetry.record(eventError({ cmd, err: errorWithMetadata, isFatal: true }));
errorMessage = formatErrorMessage(errorWithMetadata);
errorMessage = formatErrorMessage(errorWithMetadata, true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking, but it'd be nice to move to an options object like { verbose: true } or { showFullStacktrace: true } to avoid an unlabelled boolean argument.


// Timeout the error reporter (very short) because the user is waiting.
// NOTE(fks): It is better that we miss some events vs. holding too long.
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/dev/restart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export async function restartContainer(container: Container): Promise<Container
const error = createSafeError(_err);
// Print all error messages except ZodErrors from AstroConfig as the pre-logged error is sufficient
if (!isAstroConfigZodError(_err)) {
logger.error('config', formatErrorMessage(collectErrorMetadata(error)) + '\n');
logger.error('config', formatErrorMessage(collectErrorMetadata(error), logger.level() === 'debug') + '\n');
}
// Inform connected clients of the config error
container.viteServer.ws.send({
Expand Down
7 changes: 4 additions & 3 deletions packages/astro/src/core/errors/errors-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -490,9 +490,10 @@ export const PageNumberParamNotFound = {
*/
export const ImageMissingAlt = {
name: 'ImageMissingAlt',
title: 'Missing alt property.',
message: 'The alt property is required.',
hint: "The `alt` property is important for the purpose of accessibility, without it users using screen readers or other assistive technologies won't be able to understand what your image is supposed to represent. See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img#attr-alt for more information.",
title: 'Image missing required "alt" property.',
message:
'Image missing "alt" property. "alt" text is required to describe important images on the page.',
hint: 'Use an empty string ("") for decorative images.',
} satisfies ErrorData;
/**
* @docs
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/src/core/logger/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ function debug(type: string, ...messages: Array<any>) {
(globalThis as any)._astroGlobalDebug = debug;

export function enableVerboseLogging() {
debugPackage.enable('*,-babel');
debug('cli', '--verbose flag enabled! Enabling: DEBUG="*,-babel"');
debugPackage.enable('astro:*,vite:*');
debug('cli', '--verbose flag enabled! Enabling: DEBUG="astro:*,vite:*"');
debug(
'cli',
'Tip: Set the DEBUG env variable directly for more control. Example: "DEBUG=astro:*,vite:* astro build".'
Expand Down
90 changes: 53 additions & 37 deletions packages/astro/src/core/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,58 +187,74 @@ export function formatConfigErrorMessage(err: ZodError) {
)}`;
}

export function formatErrorMessage(err: ErrorWithMetadata, args: string[] = []): string {
// a regex to match the first line of a stack trace
const STACK_LINE_REGEXP = /^\s+at /g;
const IRRELEVANT_STACK_REGEXP = /(node_modules|astro[\/\\]dist)/g;
function formatErrorStackTrace(err: Error | ErrorWithMetadata, showFullStacktrace: boolean): string {
const stackLines = (err.stack || '').split('\n').filter((line) => STACK_LINE_REGEXP.test(line));
// If full details are required, just return the entire stack trace.
if (showFullStacktrace) {
return stackLines.join('\n');
}
// Grab every string from the user's codebase, exit when you hit node_modules or astro/dist
const irrelevantStackIndex = stackLines.findIndex((line) => IRRELEVANT_STACK_REGEXP.test(line));
if (irrelevantStackIndex <= 0) {
const errorId = (err as ErrorWithMetadata).id;
const errorLoc = (err as ErrorWithMetadata).loc;
if (errorId|| errorLoc?.file) {
const prettyLocation = ` at ${errorId?? errorLoc?.file}${
errorLoc?.line && errorLoc.column ? `:${errorLoc.line}:${errorLoc.column}` : ''
}`;
return prettyLocation + '\n [...] See full stack trace in the browser, or rerun with --verbose.';
} else {
return stackLines.join('\n');
}
}
// If the error occurred inside of a dependency, grab the entire stack.
// Otherwise, only grab the part of the stack that is relevant to the user's codebase.
return stackLines.splice(0, irrelevantStackIndex).join('\n') + '\n [...] See full stack trace in the browser, or rerun with --verbose.';
}

export function formatErrorMessage(err: ErrorWithMetadata, showFullStacktrace: boolean): string {
const isOurError = AstroError.is(err) || CompilerError.is(err) || AstroUserError.is(err);
let message = '';
if (isOurError) {
message += red(`[${err.name}]`) + ' ' + renderErrorMarkdown(err.message, 'cli');
} else {
message += err.message;
}
const output = [message];

args.push(
`${bgRed(black(` error `))}${red(
padMultilineString(isOurError ? renderErrorMarkdown(err.message, 'cli') : err.message)
)}`
);
if (err.hint) {
args.push(` ${bold('Hint:')}`);
args.push(
yellow(padMultilineString(isOurError ? renderErrorMarkdown(err.hint, 'cli') : err.hint, 4))
output.push(` ${bold('Hint:')}`);
output.push(
yellow(padMultilineString(renderErrorMarkdown(err.hint, 'cli'), 4))
);
}

const docsLink = getDocsForError(err);
if (docsLink) {
args.push(` ${bold('Error reference:')}`);
args.push(` ${underline(docsLink)}`);
}
if (err.id || err.loc?.file) {
args.push(` ${bold('File:')}`);
args.push(
red(
` ${err.id ?? err.loc?.file}${
err.loc?.line && err.loc.column ? `:${err.loc.line}:${err.loc.column}` : ''
}`
)
);
output.push(` ${bold('Error reference:')}`);
output.push(` ${cyan(underline(docsLink))}`);
}
if (err.frame) {
args.push(` ${bold('Code:')}`);
args.push(red(padMultilineString(err.frame.trim(), 4)));
}
if (args.length === 1 && err.stack) {
args.push(dim(err.stack));
} else if (err.stack) {
args.push(` ${bold('Stacktrace:')}`);
args.push(dim(err.stack));
args.push(``);

if (err.stack) {
output.push(` ${bold('Stack trace:')}`);
output.push(dim(formatErrorStackTrace(err, showFullStacktrace)));
}

if (err.cause) {
args.push(` ${bold('Cause:')}`);
output.push(` ${bold('Caused by:')}`);
let causeMessage = ' ';
if (err.cause instanceof Error) {
args.push(dim(err.cause.stack ?? err.cause.toString()));
causeMessage += err.cause.message + '\n' + formatErrorStackTrace(err.cause, showFullStacktrace);
} else {
args.push(JSON.stringify(err.cause));
causeMessage += (JSON.stringify(err.cause));
}

args.push(``);
output.push(dim(causeMessage));
}
return args.join('\n');

return output.join('\n');
}

export function printHelp({
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/src/vite-plugin-astro-server/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type http from 'node:http';
import type { ManifestData, SSRManifest } from '../@types/astro.js';
import { collectErrorMetadata } from '../core/errors/dev/index.js';
import { createSafeError } from '../core/errors/index.js';
import * as msg from '../core/messages.js';
import {formatErrorMessage} from '../core/messages.js';
import { collapseDuplicateSlashes, removeTrailingForwardSlash } from '../core/path.js';
import { eventError, telemetry } from '../events/index.js';
import { isServerLikeOutput } from '../prerender/utils.js';
Expand Down Expand Up @@ -102,7 +102,7 @@ export async function handleRequest({

telemetry.record(eventError({ cmd: 'dev', err: errorWithMetadata, isFatal: false }));

pipeline.logger.error(null, msg.formatErrorMessage(errorWithMetadata));
pipeline.logger.error(null, formatErrorMessage(errorWithMetadata, pipeline.logger.level() === 'debug'));
handle500Response(moduleLoader, incomingResponse, errorWithMetadata);

return err;
Expand Down