michael/astro
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Update telemetry to use new errors information (#5547)

Browse source 
* Properly get information from AstroError in telemetry

* Record telemetry for dev errors

* Fix tests

* ci(telemetry): Make sure we don't hit telemetry in tests

* fix(build): Fix types

* test: Remove unnecessary flag

* test(errors): Add test to make sure we don't have stacktraces inside error messages

* refactor(types): Type cast `getErrorDataByCode`'s return type so we don't have to cast to any later
This commit is contained in:
Erika 2023-01-04 17:34:11 +01:00 committed by GitHub
parent 2a57864195
commit 63745f8d5a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 130 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
packages/astro/src/@types/astro.ts
View file

@ -990,6 +990,7 @@ export interface AstroSettings {
tsConfig: TsConfigJson | undefined;
tsConfigPath: string | undefined;
watchFiles: string[];
forceDisableTelemetry: boolean;
}
export type AsyncRendererComponentFn<U> = (

1
packages/astro/src/core/config/settings.ts
View file

@ -18,6 +18,7 @@ export function createBaseSettings(config: AstroConfig): AstroSettings {
renderers: [jsxRenderer],
scripts: [],
watchFiles: [],
forceDisableTelemetry: false,
};
}

11
packages/astro/src/core/dev/container.ts
View file

@ -47,6 +47,7 @@ export interface CreateContainerParams {
// The string passed to --config and the resolved path
configFlag?: string;
configFlagPath?: string;
disableTelemetry?: boolean;
}
export async function createContainer(params: CreateContainerParams = {}): Promise<Container> {
@ -55,8 +56,13 @@ export async function createContainer(params: CreateContainerParams = {}): Promi
logging = defaultLogging,
settings = await createDefaultDevSettings(params.userConfig, params.root),
fs = nodeFs,
disableTelemetry,
} = params;
if (disableTelemetry) {
settings.forceDisableTelemetry = true;
}
// Initialize
applyPolyfill();
settings = await runHookConfigSetup({
@ -143,11 +149,14 @@ export function isStarted(container: Container): boolean {
return !!container.viteServer.httpServer?.listening;
}
/**
* Only used in tests
*/
export async function runInContainer(
params: CreateContainerParams,
callback: (container: Container) => Promise<void> | void
) {
const container = await createContainer(params);
const container = await createContainer({ ...params, disableTelemetry: true });
try {
await callback(container);
} finally {

2
packages/astro/src/core/errors/errors-data.ts
View file

@ -4,7 +4,7 @@
import type { ZodError } from 'zod';
interface ErrorData {
export interface ErrorData {
code: number;
title: string;
message?: string | ((...params: any) => string);

4
packages/astro/src/core/errors/utils.ts
View file

@ -1,6 +1,6 @@
import { DiagnosticCode } from '@astrojs/compiler/shared/diagnostics.js';
import type { SSRError } from '../../@types/astro.js';
import { AstroErrorCodes, AstroErrorData } from './errors-data.js';
import { AstroErrorCodes, AstroErrorData, ErrorData } from './errors-data.js';
/**
* Get the line and character based on the offset
@ -96,7 +96,7 @@ export function getErrorDataByCode(code: AstroErrorCodes | DiagnosticCode) {
if (entry) {
return {
name: entry[0],
data: entry[1],
data: entry[1] as ErrorData,
};
}
}

1
packages/astro/src/core/render/dev/environment.ts
View file

@ -36,6 +36,7 @@ export function createDevelopmentEnvironment(
site: settings.config.site,
ssr: settings.config.output === 'server',
streaming: true,
telemetry: Boolean(settings.forceDisableTelemetry),
});
return {

5
packages/astro/src/core/render/environment.ts
View file

@ -21,6 +21,7 @@ export interface Environment {
site?: string;
ssr: boolean;
streaming: boolean;
telemetry?: boolean;
}
export type CreateEnvironmentArgs = Environment;
@ -33,6 +34,9 @@ export type CreateBasicEnvironmentArgs = Partial<Environment> & {
logging: CreateEnvironmentArgs['logging'];
};
/**
* Only used in tests
*/
export function createBasicEnvironment(options: CreateBasicEnvironmentArgs): Environment {
const mode = options.mode ?? 'development';
return createEnvironment({
@ -48,5 +52,6 @@ export function createBasicEnvironment(options: CreateBasicEnvironmentArgs): Env
routeCache: new RouteCache(options.logging, mode),
ssr: options.ssr ?? true,
streaming: options.streaming ?? true,
telemetry: false,
});
}

54
packages/astro/src/events/error.ts
View file

@ -1,10 +1,12 @@
import { ZodError } from 'zod';
import { AstroErrorData, ErrorWithMetadata } from '../core/errors/index.js';
import { AstroError, AstroErrorData, ErrorWithMetadata } from '../core/errors/index.js';
import { getErrorDataByCode } from '../core/errors/utils.js';
const EVENT_ERROR = 'ASTRO_CLI_ERROR';
interface ErrorEventPayload {
code: number | undefined;
name: string;
isFatal: boolean;
plugin?: string | undefined;
cliCommand: string;
@ -22,10 +24,8 @@ interface ConfigErrorEventPayload extends ErrorEventPayload {
* content like a filename, filepath, or any other code-specific value.
* We also trim this value even further to just a few words.
*
* Our goal is to remove this entirely before v1.0.0 is released, as we work
* to add a proper error code system (see AstroErrorCodes for examples).
*
* TODO(fks): Remove around v1.0.0 release.
* This is only used for errors that do not come from us so we can get a basic
* and anonymous idea of what the error is about.
*/
const ANONYMIZE_MESSAGE_REGEX = /^(\w| )+/;
function anonymizeErrorMessage(msg: string): string | undefined {
@ -47,6 +47,7 @@ export function eventConfigError({
}): { eventName: string; payload: ConfigErrorEventPayload }[] {
const payload: ConfigErrorEventPayload = {
code: AstroErrorData.UnknownConfigError.code,
name: 'ZodError',
isFatal,
isConfig: true,
cliCommand: cmd,
@ -64,12 +65,51 @@ export function eventError({
cmd: string;
isFatal: boolean;
}): { eventName: string; payload: ErrorEventPayload }[] {
const errorData =
AstroError.is(err) && err.errorCode ? getErrorDataByCode(err.errorCode)?.data : undefined;
const payload: ErrorEventPayload = {
code: err.code || AstroErrorData.UnknownError.code,
code: err.code || err.errorCode || AstroErrorData.UnknownError.code,
name: err.name,
plugin: err.plugin,
cliCommand: cmd,
isFatal: isFatal,
anonymousMessageHint: anonymizeErrorMessage(err.message),
anonymousMessageHint:
errorData && errorData.message
? getSafeErrorMessage(errorData.message)
: anonymizeErrorMessage(err.message),
};
return [{ eventName: EVENT_ERROR, payload }];
}
/**
* Safely get the error message from an error, even if it's a function.
*/
// eslint-disable-next-line @typescript-eslint/ban-types
function getSafeErrorMessage(message: string | Function): string {
if (typeof message === 'string') {
return message;
} else {
return String.raw({
raw: extractStringFromFunction(message.toString()),
});
}
function extractStringFromFunction(func: string) {
const arrowIndex = func.indexOf('=>') + '=>'.length;
return func
.slice(arrowIndex)
.trim()
.slice(1, -1)
.replace(
/\${([^}]+)}/gm,
(str, match1) =>
`${match1
.split(/\.?(?=[A-Z])/)
.join('_')
.toUpperCase()}`
)
.replace(/\\`/g, '`');
}
}

5
packages/astro/src/vite-plugin-astro-server/request.ts
View file

@ -8,6 +8,7 @@ import { createSafeError } from '../core/errors/index.js';
import { error } from '../core/logger/core.js';
import * as msg from '../core/messages.js';
import { removeTrailingForwardSlash } from '../core/path.js';
import { eventError, telemetry } from '../events/index.js';
import { runWithErrorHandling } from './controller.js';
import { handle500Response } from './response.js';
import { handleRoute, matchRoute } from './route.js';
@ -83,6 +84,10 @@ export async function handleRequest(
// Our error should already be complete, but let's try to add a bit more through some guesswork
const errorWithMetadata = collectErrorMetadata(err, config.root);
if (env.telemetry !== false) {
telemetry.record(eventError({ cmd: 'dev', err: errorWithMetadata, isFatal: false }));
}
error(env.logging, null, msg.formatErrorMessage(errorWithMetadata));
handle500Response(moduleLoader, res, errorWithMetadata);

45
packages/astro/test/events.test.js
View file

@ -1,5 +1,6 @@
import { expect } from 'chai';
import { AstroErrorData } from '../dist/core/errors/errors-data.js';
import { AstroError } from '../dist/core/errors/errors.js';
import * as events from '../dist/events/index.js';
describe('Events', () => {
@ -427,6 +428,7 @@ describe('Events', () => {
eventName: 'ASTRO_CLI_ERROR',
payload: {
code: AstroErrorData.UnknownConfigError.code,
name: 'ZodError',
isFatal: true,
isConfig: true,
cliCommand: 'COMMAND_NAME',
@ -451,6 +453,7 @@ describe('Events', () => {
payload: {
code: 1234,
plugin: 'TEST PLUGIN',
name: 'Error',
isFatal: true,
cliCommand: 'COMMAND_NAME',
anonymousMessageHint: 'TEST ERROR MESSAGE',
@ -458,6 +461,30 @@ describe('Events', () => {
});
});
it('returns the expected event payload for AstroError', () => {
const [event] = events.eventError({
err: new AstroError({
...AstroErrorData.ClientAddressNotAvailable,
message: AstroErrorData.ClientAddressNotAvailable.message('mysuperadapter'),
}),
cmd: 'COMMAND_NAME',
isFatal: false,
});
expect(event).to.deep.equal({
eventName: 'ASTRO_CLI_ERROR',
payload: {
anonymousMessageHint:
'`Astro.clientAddress` is not available in the `ADAPTER_NAME` adapter. File an issue with the adapter to add support.',
cliCommand: 'COMMAND_NAME',
code: 3002,
isFatal: false,
name: 'ClientAddressNotAvailable',
plugin: undefined,
},
});
});
it('returns the expected event payload with a generic error', () => {
const [event] = events.eventError({
err: new Error('TEST ERROR MESSAGE'),
@ -468,6 +495,7 @@ describe('Events', () => {
eventName: 'ASTRO_CLI_ERROR',
payload: {
code: AstroErrorData.UnknownError.code,
name: 'Error',
plugin: undefined,
isFatal: false,
cliCommand: 'COMMAND_NAME',
@ -480,9 +508,26 @@ describe('Events', () => {
const [event] = events.eventError({
err: new Error('TEST ERROR MESSAGE: Sensitive data is "/Users/MYNAME/foo.astro"'),
cmd: 'COMMAND_NAME',
name: 'Error',
isFatal: true,
});
expect(event.payload.anonymousMessageHint).to.equal('TEST ERROR MESSAGE');
});
it('properly exclude stack traces from anonymousMessageHint', () => {
// Some libraries/frameworks returns stack traces in the error message, make sure we don't include that in the anonymousMessageHint
const [event] = events.eventError({
err: new Error(`[postcss] /home/projects/github-ssfd5p/src/components/Counter.css:3:15: Missed semicolon
at Input.error (file:///home/projects/github-ssfd5p/node_modules/postcss/lib/input.js:148:16)
at Parser.checkMissedSemicolon (file:///home/projects/github-ssfd5p/node_modules/postcss/lib/parser.js:596:22)
at Parser.decl (file:///home/projects/github-ssfd5p/node_modules/postcss/lib/parser.js:279:12)
at Parser.other (file:///home/projects/github-ssfd5p/node_modules/postcss/lib/parser.js:128:18)
at Parser.parse (file:///home/projects/github-ssfd5p/node_modules/postcss/lib/parser.js:72:16)`),
cmd: 'COMMAND_NAME',
name: 'Error',
isFatal: true,
});
expect(event.payload.anonymousMessageHint).to.be.undefined;
});
});
});

16
packages/astro/test/test-utils.js
View file

@ -1,17 +1,17 @@
import { execa } from 'execa';
import { polyfill } from '@astrojs/webapi';
import { execa } from 'execa';
import fastGlob from 'fast-glob';
import fs from 'fs';
import os from 'os';
import stripAnsi from 'strip-ansi';
import { fileURLToPath } from 'url';
import { sync } from '../dist/cli/sync/index.js';
import build from '../dist/core/build/index.js';
import { loadConfig } from '../dist/core/config/config.js';
import { createSettings } from '../dist/core/config/index.js';
import dev from '../dist/core/dev/index.js';
import build from '../dist/core/build/index.js';
import preview from '../dist/core/preview/index.js';
import { sync } from '../dist/cli/sync/index.js';
import { nodeLogDestination } from '../dist/core/logger/node.js';
import os from 'os';
import stripAnsi from 'strip-ansi';
import fastGlob from 'fast-glob';
import preview from '../dist/core/preview/index.js';
// polyfill WebAPIs to globalThis for Node v12, Node v14, and Node v16
polyfill(globalThis, {
@ -243,7 +243,7 @@ const cliPath = fileURLToPath(new URL('../astro.js', import.meta.url));
/** Returns a process running the Astro CLI. */
export function cli(/** @type {string[]} */ ...args) {
const spawned = execa('node', [cliPath, ...args]);
const spawned = execa('node', [cliPath, ...args], { env: {'ASTRO_TELEMETRY_DISABLED': true}});
spawned.stdout.setEncoding('utf8');

8
packages/astro/test/units/vite-plugin-astro-server/request.test.js
View file

@ -1,13 +1,13 @@
import { expect } from 'chai';
import { createLoader } from '../../../dist/core/module-loader/index.js';
import { createController, handleRequest } from '../../../dist/vite-plugin-astro-server/index.js';
import { createDefaultDevSettings } from '../../../dist/core/config/index.js';
import { createLoader } from '../../../dist/core/module-loader/index.js';
import { createBasicEnvironment } from '../../../dist/core/render/index.js';
import { createRouteManifest } from '../../../dist/core/routing/index.js';
import { defaultLogging as logging } from '../../test-utils.js';
import { createComponent, render } from '../../../dist/runtime/server/index.js';
import { createRequestAndResponse, createFs, createAstroModule } from '../test-utils.js';
import { createController, handleRequest } from '../../../dist/vite-plugin-astro-server/index.js';
import { defaultLogging as logging } from '../../test-utils.js';
import { createAstroModule, createFs, createRequestAndResponse } from '../test-utils.js';
async function createDevEnvironment(overrides = {}) {
const env = createBasicEnvironment({

1
packages/telemetry/src/index.ts
View file

@ -145,7 +145,6 @@ export class AstroTelemetry {
// to preview what data would be sent.
return Promise.resolve();
}
return post({
context,
meta,