From fa19eea87391c3ffbbc013081544096eb53ff1e5 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 9 Aug 2023 09:34:44 +0700 Subject: [PATCH] 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. (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. (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) ``` --- .../web/types/src/util/compiler-interfaces.ts | 13 ++++++- developer/src/kmc-analyze/src/messages.ts | 4 +- .../src/kmc-keyboard-info/src/messages.ts | 3 +- .../src/kmc-kmn/src/compiler/messages.ts | 19 ++++----- .../src/kmc-ldml/src/compiler/messages.ts | 2 +- developer/src/kmc-model-info/src/messages.ts | 3 +- .../kmc-model/src/model-compiler-errors.ts | 12 +++--- .../src/kmc-package/src/compiler/messages.ts | 3 +- developer/src/kmc/src/commands/analyze.ts | 2 +- developer/src/kmc/src/commands/build.ts | 26 +++++++++---- .../buildClasses/BuildKeyboardInfo.ts | 2 +- .../commands/buildClasses/BuildModelInfo.ts | 2 +- .../src/commands/buildClasses/BuildProject.ts | 2 +- developer/src/kmc/src/kmc.ts | 4 ++ ...{messages.ts => infrastructureMessages.ts} | 3 +- developer/src/kmc/src/util/KeymanSentry.ts | 38 ++++++++++++++---- .../src/kmc/src/util/NodeCompilerCallbacks.ts | 11 +++++- developer/src/kmc/src/util/fileLists.ts | 2 +- developer/src/kmc/src/util/projectLoader.ts | 2 +- ...ages.ts => test-infrastructureMessages.ts} | 39 +++++++------------ developer/src/kmc/test/test-project-build.ts | 2 +- package-lock.json | 8 ++-- package.json | 1 + 23 files changed, 123 insertions(+), 80 deletions(-) rename developer/src/kmc/src/messages/{messages.ts => infrastructureMessages.ts} (96%) rename developer/src/kmc/test/{test-messages.ts => test-infrastructureMessages.ts} (71%) diff --git a/common/web/types/src/util/compiler-interfaces.ts b/common/web/types/src/util/compiler-interfaces.ts index 92fc1f65ec..3283fb2598 100644 --- a/common/web/types/src/util/compiler-interfaces.ts +++ b/common/web/types/src/util/compiler-interfaces.ts @@ -6,6 +6,11 @@ export interface CompilerEvent { line?: number; code: number; message: string; + /** + * an internal error occurred that should be captured with a stack trace + * e.g. to the Keyman sentry instance by kmc + */ + exceptionVar?: any; }; export enum CompilerErrorSeverity { @@ -399,7 +404,13 @@ export const defaultCompilerOptions: CompilerOptions = { * @param message * @returns */ -export const CompilerMessageSpec = (code: number, message: string) : CompilerEvent => { return { code, message } }; +export const CompilerMessageSpec = (code: number, message: string, exceptionVar?: any) : CompilerEvent => ({ + code, + message: exceptionVar + ? (message ?? `Unexpected exception`) + `: ${exceptionVar.toString()}\n\nCall stack:\n${(exceptionVar instanceof Error ? exceptionVar.stack : (new Error()).stack)}` : + message, + exceptionVar +}); /** * @deprecated use `CompilerError.exceptionToString` instead diff --git a/developer/src/kmc-analyze/src/messages.ts b/developer/src/kmc-analyze/src/messages.ts index 2512fbdc19..a800fb944a 100644 --- a/developer/src/kmc-analyze/src/messages.ts +++ b/developer/src/kmc-analyze/src/messages.ts @@ -1,4 +1,4 @@ -import { CompilerErrorNamespace, CompilerErrorSeverity, CompilerMessageSpec as m, compilerExceptionToString as exc } from "@keymanapp/common-types"; +import { CompilerErrorNamespace, CompilerErrorSeverity, CompilerMessageSpec as m } from "@keymanapp/common-types"; const Namespace = CompilerErrorNamespace.Analyzer; const SevInfo = CompilerErrorSeverity.Info | Namespace; @@ -8,7 +8,7 @@ const SevInfo = CompilerErrorSeverity.Info | Namespace; const SevFatal = CompilerErrorSeverity.Fatal | Namespace; export class AnalyzerMessages { - static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, `Unexpected exception: ${exc(o.e)}`); + static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, null, o.e ?? 'unknown error'); static FATAL_UnexpectedException = SevFatal | 0x0001; static Info_ScanningFile = (o:{type: string, name: string}) => m(this.INFO_ScanningFile, diff --git a/developer/src/kmc-keyboard-info/src/messages.ts b/developer/src/kmc-keyboard-info/src/messages.ts index 06db26815a..e72fd2f0e9 100644 --- a/developer/src/kmc-keyboard-info/src/messages.ts +++ b/developer/src/kmc-keyboard-info/src/messages.ts @@ -8,8 +8,7 @@ const SevError = CompilerErrorSeverity.Error | Namespace; const SevFatal = CompilerErrorSeverity.Fatal | Namespace; export class KeyboardInfoCompilerMessages { - static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, - `Unexpected exception: ${(o.e ?? 'unknown error').toString()}\n\nCall stack:\n${(o.e instanceof Error ? o.e.stack : (new Error()).stack)}`); + static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, null, o.e ?? 'unknown error'); static FATAL_UnexpectedException = SevFatal | 0x0001; static Error_FileDoesNotExist = (o:{filename: string}) => m(this.ERROR_FileDoesNotExist, `File ${o.filename} does not exist.`); diff --git a/developer/src/kmc-kmn/src/compiler/messages.ts b/developer/src/kmc-kmn/src/compiler/messages.ts index 6b0c3ce879..1f6bf7c327 100644 --- a/developer/src/kmc-kmn/src/compiler/messages.ts +++ b/developer/src/kmc-kmn/src/compiler/messages.ts @@ -1,4 +1,4 @@ -import { CompilerErrorNamespace, CompilerErrorSeverity, CompilerEvent, CompilerMessageSpec as m, compilerExceptionToString as exc } from "@keymanapp/common-types"; +import { CompilerErrorNamespace, CompilerErrorSeverity, CompilerEvent, CompilerMessageSpec as m } from "@keymanapp/common-types"; const Namespace = CompilerErrorNamespace.KmnCompiler; const SevInfo = CompilerErrorSeverity.Info | Namespace; @@ -46,20 +46,21 @@ export const enum KmnCompilerMessageRanges { are reserved for kmcmplib messages. */ export class CompilerMessages { - static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, `Unexpected exception: ${exc(o.e)}`); + static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, null, o.e ?? 'unknown error'); static FATAL_UnexpectedException = SevFatal | 0x900; - static Fatal_MissingWasmModule = (o:{e?: any}) => m(this.FATAL_MissingWasmModule, `Could not instantiate WASM compiler module or initialization failed: ${exc(o.e)}`); + static Fatal_MissingWasmModule = (o:{e?: any}) => m(this.FATAL_MissingWasmModule, + `Could not instantiate WASM compiler module or initialization failed`, o.e ?? 'unknown error'); static FATAL_MissingWasmModule = SevFatal | 0x901; // TODO: Is this now deprecated? - static Fatal_UnableToSetCompilerOptions = () => m(this.FATAL_UnableToSetCompilerOptions, `Unable to set compiler options`); + static Fatal_UnableToSetCompilerOptions = () => m(this.FATAL_UnableToSetCompilerOptions, null, `Unable to set compiler options`); static FATAL_UnableToSetCompilerOptions = SevFatal | 0x902; - static Fatal_CallbacksNotSet = () => m(this.FATAL_CallbacksNotSet, `Callbacks were not set with init`); + static Fatal_CallbacksNotSet = () => m(this.FATAL_CallbacksNotSet, null, `Callbacks were not set with init`); static FATAL_CallbacksNotSet = SevFatal | 0x903; - static Fatal_UnicodeSetOutOfRange = () => m(this.FATAL_UnicodeSetOutOfRange, `UnicodeSet buffer was too small`); + static Fatal_UnicodeSetOutOfRange = () => m(this.FATAL_UnicodeSetOutOfRange, null, `UnicodeSet buffer was too small`); static FATAL_UnicodeSetOutOfRange = SevFatal | 0x904; static Error_UnicodeSetHasStrings = () => m(this.ERROR_UnicodeSetHasStrings, `UnicodeSet contains strings, not allowed`); @@ -72,7 +73,7 @@ export class CompilerMessages { static ERROR_UnicodeSetSyntaxError = SevError | 0x907; static Error_InvalidKvksFile = (o:{filename: string, e: any}) => m(this.ERROR_InvalidKvksFile, - `Error encountered parsing ${o.filename}: ${o.e}`); + `Error encountered parsing ${o.filename}: ${o.e ?? 'unknown error'}`); // Note, not fatal, not reporting to Sentry static ERROR_InvalidKvksFile = SevError | 0x908; static Warn_InvalidVkeyInKvksFile = (o:{filename: string, invalidVkey: string}) => m(this.WARN_InvalidVkeyInKvksFile, @@ -80,11 +81,11 @@ export class CompilerMessages { static WARN_InvalidVkeyInKvksFile = SevWarn | 0x909; static Error_InvalidDisplayMapFile = (o:{filename: string, e: any}) => m(this.ERROR_InvalidDisplayMapFile, - `Error encountered parsing display map ${o.filename}: ${o.e}`); + `Error encountered parsing display map ${o.filename}: ${o.e ?? 'unknown error'}`); // Note, not fatal, not reporting to Sentry static ERROR_InvalidDisplayMapFile = SevError | 0x90A; static Error_InvalidKvkFile = (o:{filename: string, e: any}) => m(this.ERROR_InvalidKvkFile, - `Error encountered loading ${o.filename}: ${o.e}`); + `Error encountered loading ${o.filename}: ${o.e ?? 'unknown error'}`); // Note, not fatal, not reporting to Sentry static ERROR_InvalidKvkFile = SevError | 0x90B; }; diff --git a/developer/src/kmc-ldml/src/compiler/messages.ts b/developer/src/kmc-ldml/src/compiler/messages.ts index e3ec8bcbf2..4a2686c1a7 100644 --- a/developer/src/kmc-ldml/src/compiler/messages.ts +++ b/developer/src/kmc-ldml/src/compiler/messages.ts @@ -60,7 +60,7 @@ export class CompilerMessages { static ERROR_MustBeAtLeastOneLayerElement = SevError | 0x000E; static Fatal_SectionCompilerFailed = (o:{sect: string}) => - m(this.FATAL_SectionCompilerFailed, `The compiler for '${o.sect}' failed unexpectedly.`); + m(this.FATAL_SectionCompilerFailed, null, `The compiler for '${o.sect}' failed unexpectedly.`); static FATAL_SectionCompilerFailed = SevFatal | 0x000F; static Error_DisplayIsRepeated = (o:{to: string}) => diff --git a/developer/src/kmc-model-info/src/messages.ts b/developer/src/kmc-model-info/src/messages.ts index 26ba8f315c..a4d61e8b23 100644 --- a/developer/src/kmc-model-info/src/messages.ts +++ b/developer/src/kmc-model-info/src/messages.ts @@ -8,8 +8,7 @@ const SevError = CompilerErrorSeverity.Error | Namespace; const SevFatal = CompilerErrorSeverity.Fatal | Namespace; export class ModelInfoCompilerMessages { - static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, - `Unexpected exception: ${(o.e ?? 'unknown error').toString()}\n\nCall stack:\n${(o.e instanceof Error ? o.e.stack : (new Error()).stack)}`); + static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, null, o.e ?? 'unknown error'); static FATAL_UnexpectedException = SevFatal | 0x0001; static Error_FileDoesNotExist = (o:{filename: string}) => m(this.ERROR_FileDoesNotExist, `File ${o.filename} does not exist.`); diff --git a/developer/src/kmc-model/src/model-compiler-errors.ts b/developer/src/kmc-model/src/model-compiler-errors.ts index f7062da44e..beb3398caa 100644 --- a/developer/src/kmc-model/src/model-compiler-errors.ts +++ b/developer/src/kmc-model/src/model-compiler-errors.ts @@ -1,4 +1,4 @@ -import { CompilerErrorNamespace, CompilerErrorSeverity, CompilerEvent } from "@keymanapp/common-types"; +import { CompilerErrorNamespace, CompilerErrorSeverity, CompilerEvent, CompilerMessageSpec } from "@keymanapp/common-types"; const Namespace = CompilerErrorNamespace.ModelCompiler; // const SevInfo = CompilerErrorSeverity.Info | Namespace; @@ -7,12 +7,11 @@ const SevWarn = CompilerErrorSeverity.Warn | Namespace; const SevError = CompilerErrorSeverity.Error | Namespace; const SevFatal = CompilerErrorSeverity.Fatal | Namespace; -const m = (code: number, message: string) : CompilerEvent => { return { +const m = (code: number, message: string, exceptionVar?: any) : CompilerEvent => ({ + ...CompilerMessageSpec(code, message, exceptionVar), line: ModelCompilerMessageContext.line, filename: ModelCompilerMessageContext.filename, - code, - message -} }; +}); export class ModelCompilerMessageContext { // Context added to all messages @@ -22,8 +21,7 @@ export class ModelCompilerMessageContext { export class ModelCompilerMessages { - static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, - `Unexpected exception: ${(o.e ?? 'unknown error').toString()}\n\nCall stack:\n${(o.e instanceof Error ? o.e.stack : (new Error()).stack)}`); + static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, null, o.e ?? 'unknown error'); static FATAL_UnexpectedException = SevFatal | 0x0001; static Warn_MixedNormalizationForms = (o:{wordform: string}) => m(this.WARN_MixedNormalizationForms, diff --git a/developer/src/kmc-package/src/compiler/messages.ts b/developer/src/kmc-package/src/compiler/messages.ts index 0843d4f921..c230c69772 100644 --- a/developer/src/kmc-package/src/compiler/messages.ts +++ b/developer/src/kmc-package/src/compiler/messages.ts @@ -8,8 +8,7 @@ const SevError = CompilerErrorSeverity.Error | Namespace; const SevFatal = CompilerErrorSeverity.Fatal | Namespace; export class CompilerMessages { - static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, - `Unexpected exception: ${(o.e ?? 'unknown error').toString()}\n\nCall stack:\n${(o.e instanceof Error ? o.e.stack : (new Error()).stack)}`); + static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, null, o.e ?? 'unknown error'); static FATAL_UnexpectedException = SevFatal | 0x0001; static Warn_AbsolutePath = (o:{filename: string}) => m(this.WARN_AbsolutePath, `File ${o.filename} has an absolute path, which is not portable.`); diff --git a/developer/src/kmc/src/commands/analyze.ts b/developer/src/kmc/src/commands/analyze.ts index 7a2cec2843..eceb29f7bf 100644 --- a/developer/src/kmc/src/commands/analyze.ts +++ b/developer/src/kmc/src/commands/analyze.ts @@ -2,7 +2,7 @@ import * as fs from 'fs'; import * as path from 'path'; import { Command, Option } from 'commander'; import { NodeCompilerCallbacks } from '../util/NodeCompilerCallbacks.js'; -import { InfrastructureMessages } from '../messages/messages.js'; +import { InfrastructureMessages } from '../messages/infrastructureMessages.js'; import { CompilerCallbacks, CompilerLogLevel } from '@keymanapp/common-types'; import { AnalyzeOskCharacterUse, AnalyzeOskRewritePua } from '@keymanapp/kmc-analyze'; import { BaseOptions } from '../util/baseOptions.js'; diff --git a/developer/src/kmc/src/commands/build.ts b/developer/src/kmc/src/commands/build.ts index 04c390e59d..1047dd8208 100644 --- a/developer/src/kmc/src/commands/build.ts +++ b/developer/src/kmc/src/commands/build.ts @@ -4,7 +4,7 @@ import { Command } from 'commander'; import { buildActivities } from './buildClasses/buildActivities.js'; import { BuildProject } from './buildClasses/BuildProject.js'; import { NodeCompilerCallbacks } from '../util/NodeCompilerCallbacks.js'; -import { InfrastructureMessages } from '../messages/messages.js'; +import { InfrastructureMessages } from '../messages/infrastructureMessages.js'; import { CompilerFileCallbacks, CompilerOptions, KeymanFileTypes } from '@keymanapp/common-types'; import { BaseOptions } from '../util/baseOptions.js'; import { expandFileLists } from '../util/fileLists.js'; @@ -80,13 +80,18 @@ If no input file is supplied, kmc will build the current folder.`) } async function build(filename: string, parentCallbacks: NodeCompilerCallbacks, options: CompilerOptions): Promise { - - if(!fs.existsSync(filename)) { - parentCallbacks.reportMessage(InfrastructureMessages.Error_FileDoesNotExist({filename})); - return false; - } - try { + // TEST: allow command-line simulation of infrastructure fatal errors, and + // also for unit tests + if(process.env.SENTRY_CLIENT_TEST_BUILD_EXCEPTION == '1') { + throw new Error('Test exception from SENTRY_CLIENT_TEST_BUILD_EXCEPTION'); + } + + if(!fs.existsSync(filename)) { + parentCallbacks.reportMessage(InfrastructureMessages.Error_FileDoesNotExist({filename})); + return false; + } + let builder = null; // 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 return false; } } + +/** + * these are exported only for unit tests, do not use + */ +export const unitTestEndpoints = { + build +}; diff --git a/developer/src/kmc/src/commands/buildClasses/BuildKeyboardInfo.ts b/developer/src/kmc/src/commands/buildClasses/BuildKeyboardInfo.ts index 14670c99b4..42bc4e4a95 100644 --- a/developer/src/kmc/src/commands/buildClasses/BuildKeyboardInfo.ts +++ b/developer/src/kmc/src/commands/buildClasses/BuildKeyboardInfo.ts @@ -3,7 +3,7 @@ import { BuildActivity } from './BuildActivity.js'; import { CompilerCallbacks, CompilerOptions, KeymanDeveloperProject, KeymanFileTypes } from '@keymanapp/common-types'; import { KeyboardInfoCompiler } from '@keymanapp/kmc-keyboard-info'; import { loadProject } from '../../util/projectLoader.js'; -import { InfrastructureMessages } from '../../messages/messages.js'; +import { InfrastructureMessages } from '../../messages/infrastructureMessages.js'; import { KmpCompiler } from '@keymanapp/kmc-package'; import { calculateSourcePath } from '../../util/calculateSourcePath.js'; diff --git a/developer/src/kmc/src/commands/buildClasses/BuildModelInfo.ts b/developer/src/kmc/src/commands/buildClasses/BuildModelInfo.ts index bfb167bdcc..9dd4d476e7 100644 --- a/developer/src/kmc/src/commands/buildClasses/BuildModelInfo.ts +++ b/developer/src/kmc/src/commands/buildClasses/BuildModelInfo.ts @@ -4,7 +4,7 @@ import { CompilerCallbacks, CompilerOptions, KeymanFileTypes } from '@keymanapp/ import { writeMergedModelMetadataFile } from '@keymanapp/kmc-model-info'; import { KmpCompiler } from '@keymanapp/kmc-package'; import { loadProject } from '../../util/projectLoader.js'; -import { InfrastructureMessages } from '../../messages/messages.js'; +import { InfrastructureMessages } from '../../messages/infrastructureMessages.js'; import { calculateSourcePath } from '../../util/calculateSourcePath.js'; export class BuildModelInfo extends BuildActivity { diff --git a/developer/src/kmc/src/commands/buildClasses/BuildProject.ts b/developer/src/kmc/src/commands/buildClasses/BuildProject.ts index 44cb990704..460a5af88f 100644 --- a/developer/src/kmc/src/commands/buildClasses/BuildProject.ts +++ b/developer/src/kmc/src/commands/buildClasses/BuildProject.ts @@ -3,7 +3,7 @@ import * as fs from 'fs'; import { CompilerCallbacks, CompilerFileCallbacks, CompilerOptions, KeymanDeveloperProject, KeymanDeveloperProjectFile, KeymanFileTypes } from '@keymanapp/common-types'; import { BuildActivity } from './BuildActivity.js'; import { buildActivities } from './buildActivities.js'; -import { InfrastructureMessages } from '../../messages/messages.js'; +import { InfrastructureMessages } from '../../messages/infrastructureMessages.js'; import { loadProject } from '../../util/projectLoader.js'; export class BuildProject extends BuildActivity { diff --git a/developer/src/kmc/src/kmc.ts b/developer/src/kmc/src/kmc.ts index 502c94d3c0..d22fc22348 100644 --- a/developer/src/kmc/src/kmc.ts +++ b/developer/src/kmc/src/kmc.ts @@ -17,6 +17,10 @@ try { KeymanSentry.captureException(e); } +// Ensure any messages reported to Sentry have had time to be uploaded before we +// exit. In most cases, this will be a no-op so should not affect performance. +await KeymanSentry.close(); + async function run() { /* Arguments */ diff --git a/developer/src/kmc/src/messages/messages.ts b/developer/src/kmc/src/messages/infrastructureMessages.ts similarity index 96% rename from developer/src/kmc/src/messages/messages.ts rename to developer/src/kmc/src/messages/infrastructureMessages.ts index 95c2e73741..ee828dae75 100644 --- a/developer/src/kmc/src/messages/messages.ts +++ b/developer/src/kmc/src/messages/infrastructureMessages.ts @@ -8,8 +8,7 @@ const SevError = CompilerErrorSeverity.Error | Namespace; const SevFatal = CompilerErrorSeverity.Fatal | Namespace; export class InfrastructureMessages { - static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, - `Unexpected exception: ${(o.e ?? 'unknown error').toString()}\n\nCall stack:\n${(o.e instanceof Error ? o.e.stack : (new Error()).stack)}`); + static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, null, o.e ?? 'unknown error'); static FATAL_UnexpectedException = SevFatal | 0x0001; // For this message, we override the filename with the passed-in file. A bit of a hack but does the job diff --git a/developer/src/kmc/src/util/KeymanSentry.ts b/developer/src/kmc/src/util/KeymanSentry.ts index a2a466b83d..c7fa7eb7eb 100644 --- a/developer/src/kmc/src/util/KeymanSentry.ts +++ b/developer/src/kmc/src/util/KeymanSentry.ts @@ -89,18 +89,34 @@ export class KeymanSentry { isInit = true; } + private static writeSentryMessage(eventId: string) { + process.stderr.write(` + This error has been automatically reported to the Keyman team. + Identifier: ${eventId} + Application: Keyman Developer + Reported at: https://sentry.io/organizations/keyman/projects/keyman-developer/events/${eventId}/ + `); + } + + static async reportException(e: any, silent: boolean = true) { + if(isInit) { + const eventId = await Sentry.captureException(e); + if(!silent) { + this.writeSentryMessage(eventId); + } + return eventId; + } + return null; + } + static async captureException(e: any) { if(isInit) { const eventId = Sentry.captureException(e); process.stderr.write(` -Fatal error: ${(e??'').toString()} - -This error has been automatically reported to the Keyman team. - Identifier: ${eventId} - Application: Keyman Developer - Reported at: https://sentry.io/organizations/keyman/projects/keyman-developer/events/${eventId}/ -`); - await Sentry.close(2000); + Fatal error: ${(e??'').toString()} + `); + this.writeSentryMessage(eventId); + this.close(); // For local development, we don't want to bury the trace; we need the cast to avoid // TS2367 (comparison appears to be unintentional) @@ -112,4 +128,10 @@ This error has been automatically reported to the Keyman team. throw e; } } + + static async close() { + if(isInit) { + await Sentry.close(2000); + } + } } \ No newline at end of file diff --git a/developer/src/kmc/src/util/NodeCompilerCallbacks.ts b/developer/src/kmc/src/util/NodeCompilerCallbacks.ts index 0b66b301be..58fb355e3d 100644 --- a/developer/src/kmc/src/util/NodeCompilerCallbacks.ts +++ b/developer/src/kmc/src/util/NodeCompilerCallbacks.ts @@ -6,9 +6,10 @@ import { CompilerCallbacks, CompilerEvent, CompilerError, CompilerCallbackOptions, CompilerFileCallbacks} from '@keymanapp/common-types'; -import { InfrastructureMessages } from '../messages/messages.js'; +import { InfrastructureMessages } from '../messages/infrastructureMessages.js'; import chalk from 'chalk'; import supportsColor from 'supports-color'; +import { KeymanSentry } from './KeymanSentry.js'; const color = chalk.default; const severityColors: {[value in CompilerErrorSeverity]: chalk.Chalk} = { @@ -121,6 +122,14 @@ export class NodeCompilerCallbacks implements CompilerCallbacks { this.messages.push({...event}); + // report fatal errors to Sentry, but don't display; note, it won't be + // reported if user has disabled the Sentry setting + if(CompilerError.severity(event.code) == CompilerErrorSeverity.Fatal) { + // this is async so returns a Promise, we'll let it resolve + // in its own time, and it can print its message then + KeymanSentry.reportException(event.exceptionVar, false); + } + if(CompilerError.severity(event.code) < compilerLogLevelToSeverity[this.options.logLevel]) { // collect messages but don't print to console return; diff --git a/developer/src/kmc/src/util/fileLists.ts b/developer/src/kmc/src/util/fileLists.ts index 11ce21905e..ecd786d0ae 100644 --- a/developer/src/kmc/src/util/fileLists.ts +++ b/developer/src/kmc/src/util/fileLists.ts @@ -1,7 +1,7 @@ import * as fs from 'fs'; import * as path from 'path'; import { CompilerCallbacks } from "@keymanapp/common-types"; -import { InfrastructureMessages } from "../messages/messages.js"; +import { InfrastructureMessages } from "../messages/infrastructureMessages.js"; /** * Replaces each entry starting with `@` with the content of the file, with one diff --git a/developer/src/kmc/src/util/projectLoader.ts b/developer/src/kmc/src/util/projectLoader.ts index e92a84c399..c7141522a3 100644 --- a/developer/src/kmc/src/util/projectLoader.ts +++ b/developer/src/kmc/src/util/projectLoader.ts @@ -2,7 +2,7 @@ import * as path from 'path'; import * as fs from 'fs'; import { CompilerCallbacks, KeymanDeveloperProject, KeymanFileTypes, KPJFileReader } from "@keymanapp/common-types"; -import { InfrastructureMessages } from "../messages/messages.js"; +import { InfrastructureMessages } from "../messages/infrastructureMessages.js"; export const isProject = (filename: string): boolean => fs.existsSync(filename) && ( diff --git a/developer/src/kmc/test/test-messages.ts b/developer/src/kmc/test/test-infrastructureMessages.ts similarity index 71% rename from developer/src/kmc/test/test-messages.ts rename to developer/src/kmc/test/test-infrastructureMessages.ts index 9fa2c0efbd..a9b55d075d 100644 --- a/developer/src/kmc/test/test-messages.ts +++ b/developer/src/kmc/test/test-infrastructureMessages.ts @@ -1,10 +1,11 @@ import 'mocha'; import { assert } from 'chai'; -import { InfrastructureMessages } from '../src/messages/messages.js'; +import { InfrastructureMessages } from '../src/messages/infrastructureMessages.js'; import { verifyCompilerMessagesObject } from '@keymanapp/developer-test-helpers'; import { makePathToFixture } from './helpers/index.js'; import { NodeCompilerCallbacks } from '../src/util/NodeCompilerCallbacks.js'; import { CompilerErrorNamespace } from '@keymanapp/common-types'; +import { unitTestEndpoints } from '../src/commands/build.js'; describe('InfrastructureMessages', function () { it('should have a valid InfrastructureMessages object', function() { @@ -15,34 +16,20 @@ describe('InfrastructureMessages', function () { // Message tests // - /* - TODO: + // FATAL_UnexpectedException - let callbacks = new TestCompilerCallbacks(); - - async function testForMessage(context: Mocha.Context, fixture: string[], messageId?: number) { - context.timeout(10000); - - callbacks.clear(); - - const builder = new BuildKmnKeyboard(); - const path = makePathToFixture(...fixture); - let result = await builder.build(path, callbacks, { - compilerVersion: false, - compilerWarningsAsErrors: true, - debug: false, - warnDeprecatedCode: true, + it('should generate FATAL_UnexpectedException if an exception is raised', async function() { + const ncb = new NodeCompilerCallbacks({logLevel: 'silent'}); + process.env.SENTRY_CLIENT_TEST_BUILD_EXCEPTION = '1'; + await unitTestEndpoints.build(null, ncb, {}); + delete process.env.SENTRY_CLIENT_TEST_BUILD_EXCEPTION; + assert.isTrue(ncb.hasMessage(InfrastructureMessages.FATAL_UnexpectedException), + `FATAL_UnexpectedException not generated, instead got: `+JSON.stringify(ncb.messages,null,2)); + assert.lengthOf(ncb.messages, 1); + assert.instanceOf(ncb.messages[0].exceptionVar, Error); }); - if(messageId) { - assert.isTrue(callbacks.hasMessage(messageId), `messageId ${messageId.toString(16)} not generated, instead got: `+JSON.stringify(callbacks.messages,null,2)); - assert.lengthOf(callbacks.messages, 1); - } else { - assert.lengthOf(callbacks.messages, 0, `messages should be empty, but instead got: `+JSON.stringify(callbacks.messages,null,2)); - assert.isTrue(result); - } - } - +/* // ERROR_FileDoesNotExist it('should generate ERROR_FileDoesNotExist if a file does not exist', async function() { diff --git a/developer/src/kmc/test/test-project-build.ts b/developer/src/kmc/test/test-project-build.ts index 85929d7844..088de1e40f 100644 --- a/developer/src/kmc/test/test-project-build.ts +++ b/developer/src/kmc/test/test-project-build.ts @@ -3,7 +3,7 @@ import { assert } from 'chai'; import 'mocha'; import { BuildProject } from '../src/commands/buildClasses/BuildProject.js'; import { makePathToFixture } from './helpers/index.js'; -import { InfrastructureMessages } from '../src/messages/messages.js'; +import { InfrastructureMessages } from '../src/messages/infrastructureMessages.js'; const callbacks = new TestCompilerCallbacks(); diff --git a/package-lock.json b/package-lock.json index 49ef815928..405d2154cf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -35,6 +35,7 @@ "@keymanapp/ldml-keyboard-constants": "file:core/include/ldml" }, "devDependencies": { + "@types/chai": "^4.3.5", "@typescript-eslint/eslint-plugin": "^5.59.1", "chai": "^4.3.4", "esbuild": "^0.15.16", @@ -3649,9 +3650,10 @@ } }, "node_modules/@types/chai": { - "version": "4.3.0", - "dev": true, - "license": "MIT" + "version": "4.3.5", + "resolved": "https://registry.npmjs.org/@types/chai/-/chai-4.3.5.tgz", + "integrity": "sha512-mEo1sAde+UCE6b2hxn332f1g1E8WfYRu6p5SvTKr2ZKC1f7gFJXk4h5PyGP9Dt6gCaG8y8XhwnXWC6Iy2cmBng==", + "dev": true }, "node_modules/@types/component-emitter": { "version": "1.2.11", diff --git a/package.json b/package.json index ebf64981ff..0afd9ade6e 100644 --- a/package.json +++ b/package.json @@ -2,6 +2,7 @@ "name": "root", "private": true, "devDependencies": { + "@types/chai": "^4.3.5", "@typescript-eslint/eslint-plugin": "^5.59.1", "chai": "^4.3.4", "esbuild": "^0.15.16",