Skip to content

Commit fa19eea

Browse files
mcdurdinmcdurdin
committed
chore(developer): improve kmc sentry reporting on fatal build errors
kmc already reported unhandled exceptions, but any handled fatal errors were captured and only reported to the user. It is better to report these to Sentry as these are still unexpected. I have refactored all the fatal exception messages in various kmc modules to use a common mechanism, keeping all the Sentry integration in kmc, now passing exception data up in the `CompilerEvent.exceptionVar` property. * I took the opportunity to rename messages.ts to infrastructureMessages.ts * @types/chai was missing which gave intellisense errors in vscode * normal exit of kmc now provides an opportunity for error reports to Sentry to be finalized * Added a unit test for fatal errors in kmc * Added a manual test pathway with `SENTRY_CLIENT_TEST_BUILD_EXCEPTION` env var to trip the build fatal error mechanism and verify that it looks ok; the following shows test runs demonstrate how fatal build errors are reported: ``` mcdurdin@THARK MINGW64 /c/Projects/keyman/app/developer/src/kmc (chore/developer/report-fatal-compiler-errors-to-sentry) $ SENTRY_CLIENT_TEST_BUILD_EXCEPTION=1 node . --error-reporting build fatal KM05001: Unexpected exception: Error: Test exception from SENTRY_CLIENT_TEST_BUILD_EXCEPTION Call stack: Error: Test exception from SENTRY_CLIENT_TEST_BUILD_EXCEPTION at build (file:///C:/Projects/keyman/app/developer/src/kmc/build/src/commands/build.js:78:19) at Command.<anonymous> (file:///C:/Projects/keyman/app/developer/src/kmc/build/src/commands/build.js:66:24) at Command.listener [as _actionHandler] (C:\Projects\keyman\app\node_modules\commander\lib\command.js:482:17) at C:\Projects\keyman\app\node_modules\commander\lib\command.js:1283:65 at Command._chainOrCall (C:\Projects\keyman\app\node_modules\commander\lib\command.js:1177:12) at Command._parseCommand (C:\Projects\keyman\app\node_modules\commander\lib\command.js:1283:27) at C:\Projects\keyman\app\node_modules\commander\lib\command.js:1081:27 at Command._chainOrCall (C:\Projects\keyman\app\node_modules\commander\lib\command.js:1177:12) at Command._dispatchSubcommand (C:\Projects\keyman\app\node_modules\commander\lib\command.js:1077:23) at Command._parseCommand (C:\Projects\keyman\app\node_modules\commander\lib\command.js:1248:19) This error has been automatically reported to the Keyman team. Identifier: 6f0fca1a26694c22b03f02b2463d39c5 Application: Keyman Developer Reported at: https://sentry.io/organizations/keyman/projects/keyman-developer/events/6f0fca1a26694c22b03f02b2463d39c5/ mcdurdin@THARK MINGW64 /c/Projects/keyman/app/developer/src/kmc (chore/developer/report-fatal-compiler-errors-to-sentry) $ SENTRY_CLIENT_TEST_BUILD_EXCEPTION=1 node . --no-error-reporting build fatal KM05001: Unexpected exception: Error: Test exception from SENTRY_CLIENT_TEST_BUILD_EXCEPTION Call stack: Error: Test exception from SENTRY_CLIENT_TEST_BUILD_EXCEPTION at build (file:///C:/Projects/keyman/app/developer/src/kmc/build/src/commands/build.js:78:19) at Command.<anonymous> (file:///C:/Projects/keyman/app/developer/src/kmc/build/src/commands/build.js:66:24) at Command.listener [as _actionHandler] (C:\Projects\keyman\app\node_modules\commander\lib\command.js:482:17) at C:\Projects\keyman\app\node_modules\commander\lib\command.js:1283:65 at Command._chainOrCall (C:\Projects\keyman\app\node_modules\commander\lib\command.js:1177:12) at Command._parseCommand (C:\Projects\keyman\app\node_modules\commander\lib\command.js:1283:27) at C:\Projects\keyman\app\node_modules\commander\lib\command.js:1081:27 at Command._chainOrCall (C:\Projects\keyman\app\node_modules\commander\lib\command.js:1177:12) at Command._dispatchSubcommand (C:\Projects\keyman\app\node_modules\commander\lib\command.js:1077:23) at Command._parseCommand (C:\Projects\keyman\app\node_modules\commander\lib\command.js:1248:19) ```
1 parent 574e850 commit fa19eea

File tree

23 files changed

+122
-79
lines changed

23 files changed

+122
-79
lines changed

‎common/web/types/src/util/compiler-interfaces.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ export interface CompilerEvent {
66
line?: number;
77
code: number;
88
message: string;
9+
/**
10+
* an internal error occurred that should be captured with a stack trace
11+
* e.g. to the Keyman sentry instance by kmc
12+
*/
13+
exceptionVar?: any;
914
};
1015

1116
export enum CompilerErrorSeverity {
@@ -399,7 +404,13 @@ export const defaultCompilerOptions: CompilerOptions = {
399404
* @param message
400405
* @returns
401406
*/
402-
export const CompilerMessageSpec = (code: number, message: string) : CompilerEvent => { return { code, message } };
407+
export const CompilerMessageSpec = (code: number, message: string, exceptionVar?: any) : CompilerEvent => ({
408+
code,
409+
message: exceptionVar
410+
? (message ?? `Unexpected exception`) + `: ${exceptionVar.toString()}\n\nCall stack:\n${(exceptionVar instanceof Error ? exceptionVar.stack : (new Error()).stack)}` :
411+
message,
412+
exceptionVar
413+
});
403414

404415
/**
405416
* @deprecated use `CompilerError.exceptionToString` instead

‎developer/src/kmc-analyze/src/messages.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { CompilerErrorNamespace, CompilerErrorSeverity, CompilerMessageSpec as m, compilerExceptionToString as exc } from "@keymanapp/common-types";
1+
import { CompilerErrorNamespace, CompilerErrorSeverity, CompilerMessageSpec as m } from "@keymanapp/common-types";
22

33
const Namespace = CompilerErrorNamespace.Analyzer;
44
const SevInfo = CompilerErrorSeverity.Info | Namespace;
@@ -8,7 +8,7 @@ const SevInfo = CompilerErrorSeverity.Info | Namespace;
88
const SevFatal = CompilerErrorSeverity.Fatal | Namespace;
99

1010
export class AnalyzerMessages {
11-
static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, `Unexpected exception: ${exc(o.e)}`);
11+
static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, null, o.e ?? 'unknown error');
1212
static FATAL_UnexpectedException = SevFatal | 0x0001;
1313

1414
static Info_ScanningFile = (o:{type: string, name: string}) => m(this.INFO_ScanningFile,

‎developer/src/kmc-keyboard-info/src/messages.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ const SevError = CompilerErrorSeverity.Error | Namespace;
88
const SevFatal = CompilerErrorSeverity.Fatal | Namespace;
99

1010
export class KeyboardInfoCompilerMessages {
11-
static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException,
12-
`Unexpected exception: ${(o.e ?? 'unknown error').toString()}\n\nCall stack:\n${(o.e instanceof Error ? o.e.stack : (new Error()).stack)}`);
11+
static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, null, o.e ?? 'unknown error');
1312
static FATAL_UnexpectedException = SevFatal | 0x0001;
1413

1514
static Error_FileDoesNotExist = (o:{filename: string}) => m(this.ERROR_FileDoesNotExist, `File ${o.filename} does not exist.`);

‎developer/src/kmc-kmn/src/compiler/messages.ts‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { CompilerErrorNamespace, CompilerErrorSeverity, CompilerEvent, CompilerMessageSpec as m, compilerExceptionToString as exc } from "@keymanapp/common-types";
1+
import { CompilerErrorNamespace, CompilerErrorSeverity, CompilerEvent, CompilerMessageSpec as m } from "@keymanapp/common-types";
22

33
const Namespace = CompilerErrorNamespace.KmnCompiler;
44
const SevInfo = CompilerErrorSeverity.Info | Namespace;
@@ -46,20 +46,21 @@ export const enum KmnCompilerMessageRanges {
4646
are reserved for kmcmplib messages.
4747
*/
4848
export class CompilerMessages {
49-
static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, `Unexpected exception: ${exc(o.e)}`);
49+
static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, null, o.e ?? 'unknown error');
5050
static FATAL_UnexpectedException = SevFatal | 0x900;
5151

52-
static Fatal_MissingWasmModule = (o:{e?: any}) => m(this.FATAL_MissingWasmModule, `Could not instantiate WASM compiler module or initialization failed: ${exc(o.e)}`);
52+
static Fatal_MissingWasmModule = (o:{e?: any}) => m(this.FATAL_MissingWasmModule,
53+
`Could not instantiate WASM compiler module or initialization failed`, o.e ?? 'unknown error');
5354
static FATAL_MissingWasmModule = SevFatal | 0x901;
5455

5556
// TODO: Is this now deprecated?
56-
static Fatal_UnableToSetCompilerOptions = () => m(this.FATAL_UnableToSetCompilerOptions, `Unable to set compiler options`);
57+
static Fatal_UnableToSetCompilerOptions = () => m(this.FATAL_UnableToSetCompilerOptions, null, `Unable to set compiler options`);
5758
static FATAL_UnableToSetCompilerOptions = SevFatal | 0x902;
5859

59-
static Fatal_CallbacksNotSet = () => m(this.FATAL_CallbacksNotSet, `Callbacks were not set with init`);
60+
static Fatal_CallbacksNotSet = () => m(this.FATAL_CallbacksNotSet, null, `Callbacks were not set with init`);
6061
static FATAL_CallbacksNotSet = SevFatal | 0x903;
6162

62-
static Fatal_UnicodeSetOutOfRange = () => m(this.FATAL_UnicodeSetOutOfRange, `UnicodeSet buffer was too small`);
63+
static Fatal_UnicodeSetOutOfRange = () => m(this.FATAL_UnicodeSetOutOfRange, null, `UnicodeSet buffer was too small`);
6364
static FATAL_UnicodeSetOutOfRange = SevFatal | 0x904;
6465

6566
static Error_UnicodeSetHasStrings = () => m(this.ERROR_UnicodeSetHasStrings, `UnicodeSet contains strings, not allowed`);
@@ -72,19 +73,19 @@ export class CompilerMessages {
7273
static ERROR_UnicodeSetSyntaxError = SevError | 0x907;
7374

7475
static Error_InvalidKvksFile = (o:{filename: string, e: any}) => m(this.ERROR_InvalidKvksFile,
75-
`Error encountered parsing ${o.filename}: ${o.e}`);
76+
`Error encountered parsing ${o.filename}: ${o.e ?? 'unknown error'}`); // Note, not fatal, not reporting to Sentry
7677
static ERROR_InvalidKvksFile = SevError | 0x908;
7778

7879
static Warn_InvalidVkeyInKvksFile = (o:{filename: string, invalidVkey: string}) => m(this.WARN_InvalidVkeyInKvksFile,
7980
`Invalid virtual key ${o.invalidVkey} found in ${o.filename}`);
8081
static WARN_InvalidVkeyInKvksFile = SevWarn | 0x909;
8182

8283
static Error_InvalidDisplayMapFile = (o:{filename: string, e: any}) => m(this.ERROR_InvalidDisplayMapFile,
83-
`Error encountered parsing display map ${o.filename}: ${o.e}`);
84+
`Error encountered parsing display map ${o.filename}: ${o.e ?? 'unknown error'}`); // Note, not fatal, not reporting to Sentry
8485
static ERROR_InvalidDisplayMapFile = SevError | 0x90A;
8586

8687
static Error_InvalidKvkFile = (o:{filename: string, e: any}) => m(this.ERROR_InvalidKvkFile,
87-
`Error encountered loading ${o.filename}: ${o.e}`);
88+
`Error encountered loading ${o.filename}: ${o.e ?? 'unknown error'}`); // Note, not fatal, not reporting to Sentry
8889
static ERROR_InvalidKvkFile = SevError | 0x90B;
8990
};
9091

‎developer/src/kmc-ldml/src/compiler/messages.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export class CompilerMessages {
6060
static ERROR_MustBeAtLeastOneLayerElement = SevError | 0x000E;
6161

6262
static Fatal_SectionCompilerFailed = (o:{sect: string}) =>
63-
m(this.FATAL_SectionCompilerFailed, `The compiler for '${o.sect}' failed unexpectedly.`);
63+
m(this.FATAL_SectionCompilerFailed, null, `The compiler for '${o.sect}' failed unexpectedly.`);
6464
static FATAL_SectionCompilerFailed = SevFatal | 0x000F;
6565

6666
static Error_DisplayIsRepeated = (o:{to: string}) =>

‎developer/src/kmc-model-info/src/messages.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ const SevError = CompilerErrorSeverity.Error | Namespace;
88
const SevFatal = CompilerErrorSeverity.Fatal | Namespace;
99

1010
export class ModelInfoCompilerMessages {
11-
static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException,
12-
`Unexpected exception: ${(o.e ?? 'unknown error').toString()}\n\nCall stack:\n${(o.e instanceof Error ? o.e.stack : (new Error()).stack)}`);
11+
static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, null, o.e ?? 'unknown error');
1312
static FATAL_UnexpectedException = SevFatal | 0x0001;
1413

1514
static Error_FileDoesNotExist = (o:{filename: string}) => m(this.ERROR_FileDoesNotExist, `File ${o.filename} does not exist.`);

‎developer/src/kmc-model/src/model-compiler-errors.ts‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { CompilerErrorNamespace, CompilerErrorSeverity, CompilerEvent } from "@keymanapp/common-types";
1+
import { CompilerErrorNamespace, CompilerErrorSeverity, CompilerEvent, CompilerMessageSpec } from "@keymanapp/common-types";
22

33
const Namespace = CompilerErrorNamespace.ModelCompiler;
44
// const SevInfo = CompilerErrorSeverity.Info | Namespace;
@@ -7,12 +7,11 @@ const SevWarn = CompilerErrorSeverity.Warn | Namespace;
77
const SevError = CompilerErrorSeverity.Error | Namespace;
88
const SevFatal = CompilerErrorSeverity.Fatal | Namespace;
99

10-
const m = (code: number, message: string) : CompilerEvent => { return {
10+
const m = (code: number, message: string, exceptionVar?: any) : CompilerEvent => ({
11+
...CompilerMessageSpec(code, message, exceptionVar),
1112
line: ModelCompilerMessageContext.line,
1213
filename: ModelCompilerMessageContext.filename,
13-
code,
14-
message
15-
} };
14+
});
1615

1716
export class ModelCompilerMessageContext {
1817
// Context added to all messages
@@ -22,8 +21,7 @@ export class ModelCompilerMessageContext {
2221

2322
export class ModelCompilerMessages {
2423

25-
static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException,
26-
`Unexpected exception: ${(o.e ?? 'unknown error').toString()}\n\nCall stack:\n${(o.e instanceof Error ? o.e.stack : (new Error()).stack)}`);
24+
static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, null, o.e ?? 'unknown error');
2725
static FATAL_UnexpectedException = SevFatal | 0x0001;
2826

2927
static Warn_MixedNormalizationForms = (o:{wordform: string}) => m(this.WARN_MixedNormalizationForms,

‎developer/src/kmc-package/src/compiler/messages.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ const SevError = CompilerErrorSeverity.Error | Namespace;
88
const SevFatal = CompilerErrorSeverity.Fatal | Namespace;
99

1010
export class CompilerMessages {
11-
static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException,
12-
`Unexpected exception: ${(o.e ?? 'unknown error').toString()}\n\nCall stack:\n${(o.e instanceof Error ? o.e.stack : (new Error()).stack)}`);
11+
static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, null, o.e ?? 'unknown error');
1312
static FATAL_UnexpectedException = SevFatal | 0x0001;
1413

1514
static Warn_AbsolutePath = (o:{filename: string}) => m(this.WARN_AbsolutePath, `File ${o.filename} has an absolute path, which is not portable.`);

‎developer/src/kmc/src/commands/analyze.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import * as fs from 'fs';
22
import * as path from 'path';
33
import { Command, Option } from 'commander';
44
import { NodeCompilerCallbacks } from '../util/NodeCompilerCallbacks.js';
5-
import { InfrastructureMessages } from '../messages/messages.js';
5+
import { InfrastructureMessages } from '../messages/infrastructureMessages.js';
66
import { CompilerCallbacks, CompilerLogLevel } from '@keymanapp/common-types';
77
import { AnalyzeOskCharacterUse, AnalyzeOskRewritePua } from '@keymanapp/kmc-analyze';
88
import { BaseOptions } from '../util/baseOptions.js';

‎developer/src/kmc/src/commands/build.ts‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { Command } from 'commander';
44
import { buildActivities } from './buildClasses/buildActivities.js';
55
import { BuildProject } from './buildClasses/BuildProject.js';
66
import { NodeCompilerCallbacks } from '../util/NodeCompilerCallbacks.js';
7-
import { InfrastructureMessages } from '../messages/messages.js';
7+
import { InfrastructureMessages } from '../messages/infrastructureMessages.js';
88
import { CompilerFileCallbacks, CompilerOptions, KeymanFileTypes } from '@keymanapp/common-types';
99
import { BaseOptions } from '../util/baseOptions.js';
1010
import { expandFileLists } from '../util/fileLists.js';
@@ -80,13 +80,18 @@ If no input file is supplied, kmc will build the current folder.`)
8080
}
8181

8282
async function build(filename: string, parentCallbacks: NodeCompilerCallbacks, options: CompilerOptions): Promise<boolean> {
83+
try {
84+
// TEST: allow command-line simulation of infrastructure fatal errors, and
85+
// also for unit tests
86+
if(process.env.SENTRY_CLIENT_TEST_BUILD_EXCEPTION == '1') {
87+
throw new Error('Test exception from SENTRY_CLIENT_TEST_BUILD_EXCEPTION');
88+
}
8389

84-
if(!fs.existsSync(filename)) {
85-
parentCallbacks.reportMessage(InfrastructureMessages.Error_FileDoesNotExist({filename}));
86-
return false;
87-
}
90+
if(!fs.existsSync(filename)) {
91+
parentCallbacks.reportMessage(InfrastructureMessages.Error_FileDoesNotExist({filename}));
92+
return false;
93+
}
8894

89-
try {
9095
let builder = null;
9196

9297
// If infile is a directory, then we treat that as a project and build it
@@ -136,3 +141,10 @@ async function build(filename: string, parentCallbacks: NodeCompilerCallbacks, o
136141
return false;
137142
}
138143
}
144+
145+
/**
146+
* these are exported only for unit tests, do not use
147+
*/
148+
export const unitTestEndpoints = {
149+
build
150+
};

0 commit comments

Comments
 (0)