From cdc6687e50aba0bd11d67bec2e5cae791a1621fe Mon Sep 17 00:00:00 2001 From: Steven Hugg Date: Tue, 29 Sep 2026 12:02:15 +0200 Subject: [PATCH] added telemetry and BuildError/InternalError --- extension/CHANGELOG.md | 3 ++ extension/README.md | 11 ++++ extension/src/buildcore.ts | 11 +++- extension/src/emuworker.ts | 4 ++ extension/src/engine.ts | 4 +- extension/src/extension.ts | 16 ++++-- extension/src/telemetry.ts | 48 +++++++++++++++++ src/common/telemetry.ts | 81 +++++++++++++++++++++++++++++ src/common/workertypes.ts | 3 ++ src/ide/errorreport.ts | 102 +++++++++++++------------------------ src/ide/ui.ts | 9 ++-- src/tools/8bws.ts | 8 ++- src/tools/buildpresets.ts | 10 +++- src/tools/cliformat.ts | 5 ++ src/tools/testlib.ts | 5 +- src/worker/builder.ts | 26 ++++++++-- src/worker/workermain.ts | 13 +++-- test/unit/testtelemetry.ts | 96 ++++++++++++++++++++++++++++++++++ 18 files changed, 367 insertions(+), 88 deletions(-) create mode 100644 extension/src/telemetry.ts create mode 100644 src/common/telemetry.ts create mode 100644 test/unit/testtelemetry.ts diff --git a/extension/CHANGELOG.md b/extension/CHANGELOG.md index 7e1584f0..ca4c4a42 100644 --- a/extension/CHANGELOG.md +++ b/extension/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Compiler and emulator crashes send an error report to 8bitworkshop.com, + following VS Code's `telemetry.telemetryLevel` setting. See the README. + - The Atari 2600 now stops at breakpoints and steps by instruction, not by frame, and can step back. - `.s`, `.asm`, `.inc` and `.a` files in a project get the highlighting diff --git a/extension/README.md b/extension/README.md index 0187d2b1..2c99da63 100644 --- a/extension/README.md +++ b/extension/README.md @@ -118,6 +118,17 @@ and Open Example lists show which platforms have one. - [Report a problem](https://github.com/sehugg/8bitworkshop/issues) - [Source code](https://github.com/sehugg/8bitworkshop) +## Error reports + +When a compiler or the emulator crashes, the extension sends an error report +to 8bitworkshop.com: the error message, stack trace, platform, tool, and +VS Code and extension versions. It doesn't send your source files. Errors in your +program, like a syntax error or a crash in the emulated machine, aren't +reported. + +The reports follow VS Code's `telemetry.telemetryLevel` setting: set it to +`off` to stop them. + ## License GPL-3.0. The included components (toolchains, emulators, libraries, firmware, diff --git a/extension/src/buildcore.ts b/extension/src/buildcore.ts index 501334ef..ebbbdf6e 100644 --- a/extension/src/buildcore.ts +++ b/extension/src/buildcore.ts @@ -4,7 +4,8 @@ import * as fs from 'fs'; import * as path from 'path'; -import type { CodeListingMap, FileData, Segment, WorkerError, WorkerResult } from "../../src/common/workertypes"; +import type { CodeListingMap, FileData, Segment, WorkerError, WorkerErrorResult, WorkerResult } from "../../src/common/workertypes"; +import { toInternalError } from "../../src/common/telemetry"; import { getBasePlatform, isProbablyBinary } from "../../src/common/util"; import { getToolForPlatform } from "../../src/common/toolselect"; import { FileProvider, buildWorkerMessage, resolveDependencies } from "../../src/common/projectcore"; @@ -50,6 +51,8 @@ export interface BuildOutcome { files?: { [path: string]: FileData }; /** true when no inputs changed since the previous build */ unchanged?: boolean; + /** set when the tool crashed, for error reports */ + internal?: WorkerErrorResult['internal']; } /** @@ -103,7 +106,10 @@ export class Builder { } result = await handleMessage(msg); } catch (e) { - return { success: false, tool, paths, diagnostics: [{ path: req.mainPath, line: 0, msg: String(e && e.message || e) }] }; + return { + success: false, tool, paths, diagnostics: [{ path: req.mainPath, line: 0, msg: String(e && e.message || e) }], + internal: { tool, platform: req.platform, ...toInternalError(e) }, + }; } if (!result || ('unchanged' in result && result.unchanged)) { // the worker skips unchanged builds, but Run still needs the output. @@ -117,6 +123,7 @@ export class Builder { return { success: false, tool, paths, diagnostics: result.errors.map(err => ({ path: toPath(err), line: err.line || 0, msg: err.msg })), + internal: result.internal, }; } if ('output' in result) { diff --git a/extension/src/emuworker.ts b/extension/src/emuworker.ts index 5904895b..7dbea2c4 100644 --- a/extension/src/emuworker.ts +++ b/extension/src/emuworker.ts @@ -11,6 +11,7 @@ import { clearLastKeycodeMap, describeControls, getLastKeycodeMap, setHaltHandle import { ControlHint, PLATFORM_CONTROLS } from '../../src/common/controls'; import { AudioStream, setAudioStreamFactory } from '../../src/common/audio'; import { getRootBasePlatform } from '../../src/common/util'; +import { toInternalError } from '../../src/common/telemetry'; import type { FileData } from '../../src/common/workertypes'; import type { StopEvent } from '../../src/common/debugcontroller'; import { BuildInfo, DebugService, TimelineInfo } from '../../src/tools/debugservice'; @@ -279,6 +280,9 @@ function tick() { frames++; } } catch (e) { + // EmuHalt (the program's fault) is a stop, caught in DebugController; + // anything reaching here is an emulator bug + rpc.emit('internalError', { platform: target.id, ...toInternalError(e) }); halt(String(e && e.message || e), 'exception'); return; } diff --git a/extension/src/engine.ts b/extension/src/engine.ts index 44f9f962..c80cc6d6 100644 --- a/extension/src/engine.ts +++ b/extension/src/engine.ts @@ -14,7 +14,8 @@ export class WorkerHandle { readonly script: string, readonly rootDir: string, readonly handlers: { [method: string]: (...args: any[]) => any } = {}, - readonly log: (msg: string) => void = () => { }) { } + readonly log: (msg: string) => void = () => { }, + readonly onCrash: (err: Error) => void = () => { }) { } get(): Rpc { if (!this.rpc) { @@ -27,6 +28,7 @@ export class WorkerHandle { }; worker.on('error', err => { this.log(`${this.script}: ${err && err.stack || err}`); + this.onCrash(err); fail(err); }); worker.on('exit', code => fail(new Error(`${this.script} exited (${code})`))); diff --git a/extension/src/extension.ts b/extension/src/extension.ts index 93b53b21..c993690b 100644 --- a/extension/src/extension.ts +++ b/extension/src/extension.ts @@ -22,9 +22,11 @@ import { Detection, classifyFinding, isBuildableSource } from '../../src/common/ import { TOOL_META } from '../../src/common/toolmeta'; import { AssetStore } from './assets'; import { AssetManifest, packsForPlatform } from './assetpacks'; +import { ErrorTelemetry } from './telemetry'; let context: vscode.ExtensionContext; let output: vscode.OutputChannel; +let telemetry: ErrorTelemetry; let diagnostics: vscode.DiagnosticCollection; let status: vscode.StatusBarItem; let scope: ProjectScope; @@ -62,13 +64,14 @@ let debugging: { session: vscode.DebugSession, backend: WorkerDebugBackend } | u export function activate(ctx: vscode.ExtensionContext) { context = ctx; output = vscode.window.createOutputChannel('8bitworkshop'); + telemetry = new ErrorTelemetry(msg => output.appendLine(msg)); diagnostics = vscode.languages.createDiagnosticCollection('8bitworkshop'); status = vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Left); status.command = '8bitworkshop.projectMenu'; scope = new ProjectScope(ctx.workspaceState); templates = new Templates(ctx.extensionPath, () => toolchainRoot()); scheduler = new BuildScheduler(reason => autoBuild(reason)); - ctx.subscriptions.push(output, diagnostics, status, scope); + ctx.subscriptions.push(output, telemetry, diagnostics, status, scope); ctx.subscriptions.push(vscode.workspace.registerFileSystemProvider(PRESET_SCHEME, new PresetFileSystem(templates), { isCaseSensitive: true, isReadonly: PresetFileSystem.readonlyMessage() })); @@ -246,7 +249,7 @@ async function getBuilds(platform: string): Promise { output.appendLine(`Toolchain root: ${root}`); builds = new WorkerHandle('buildworker.js', root, { readFile: (buildId: number, rel: string) => readers.get(buildId)?.(rel) ?? null, - }, msg => output.appendLine(msg)); + }, msg => output.appendLine(msg), err => telemetry.reportError('worker', err, { platform })); } return builds; } @@ -254,7 +257,9 @@ async function getBuilds(platform: string): Promise { async function getEmu(platform: string): Promise { var root = await toolchainRoot(platform); if (!emu) { - emu = new WorkerHandle('emuworker.js', root, {}, msg => output.appendLine(msg)); + emu = new WorkerHandle('emuworker.js', root, {}, msg => output.appendLine(msg), + err => telemetry.reportError('emu', err, { platform })); + emu.on('internalError', e => telemetry.reportInternal('emu', e, { platform: e.platform })); emu.on('frame', frame => panel?.showFrame(frame)); emu.on('audio', (chunk: AudioChunk) => panel?.showAudio(chunk)); emu.on('audioReset', () => panel?.resetAudio()); @@ -529,6 +534,7 @@ async function startEmulator(target: Target, build: BuildOutcome, opts: { paused output.appendLine(`Running ${title} on ${target.platform}`); return true; } catch (e) { + telemetry.reportError('emu', e, { platform: target.platform }); output.appendLine(`Emulator failed to start: ${e && e.stack || e}`); output.show(true); return false; @@ -646,6 +652,7 @@ async function runBuild(target: Target, reason: BuildReason): Promise('build', args); } catch (e) { + telemetry.reportError('worker', e, { platform: target.platform, tool: target.tool }); output.appendLine(`Build crashed: ${e && e.stack || e}`); output.show(true); return; @@ -653,6 +660,9 @@ async function runBuild(target: Target, reason: BuildReason): Promise void, private url = ERROR_REPORT_URL) { + this.logger = vscode.env.createTelemetryLogger({ + sendEventData: (_name, data) => this.post(data || {}), + // exceptions VS Code caught from this extension + sendErrorData: (err, data) => { + if (this.unhandled++ >= MAX_REPORTS_PER_SESSION) return; + this.post({ ...data, source: 'vscode', msg: err.message, stack: err.stack }); + }, + }); + this.reporter = new ErrorReporter('vscode', payload => this.logger.logError('error', payload)); + } + + /** A tool or emulator failure, as a worker returned it. */ + reportInternal(source: ErrorSource, ie: InternalError, fields?: ErrorReportFields) { + this.reporter.report(ie.msg, ie.stack, fields, source); + } + + reportError(source: ErrorSource, err: any, fields?: ErrorReportFields) { + this.reporter.reportError(err, fields, source); + } + + private post(data: Record) { + var payload: ErrorPayload = {}; + for (var k in data) payload[k] = clampField(data[k], 2000); + fetch(this.url, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(payload), + }).catch(e => this.log(`error report failed: ${e}`)); + } + + dispose() { + this.logger.dispose(); + } +} diff --git a/src/common/telemetry.ts b/src/common/telemetry.ts new file mode 100644 index 00000000..d96e61e7 --- /dev/null +++ b/src/common/telemetry.ts @@ -0,0 +1,81 @@ +// Error reports sent to our own server (web/error.php), shared by the web IDE +// and the VS Code extension. Each front end supplies a sender; this module +// limits sizes and volume so an error in a loop can't flood the server. + +/** Where the report came from: the IDE page, a build tool, the emulator, or VS Code. */ +export type ErrorSource = 'ide' | 'worker' | 'emu' | 'vscode'; + +export const ERROR_REPORT_URL = 'https://8bitworkshop.com/error.php'; + +export const MAX_REPORTS_PER_SESSION = 10; +const MAX_FIELD = 2000; + +/** Fields beyond msg/stack; error.php keeps any short alphanumeric key. */ +export interface ErrorReportFields { + platform?: string; + tool?: string; + window?: string; + [key: string]: any; +} + +/** A tool or emulator failure that isn't the user's fault, carried as plain data. */ +export interface InternalError { + msg: string; + stack?: string; +} + +export type ErrorPayload = { [k: string]: string }; + +export function clampField(val: any, max: number): string { + if (val == null) return ''; + try { + var s = typeof val === 'string' ? val : (typeof val === 'object' ? JSON.stringify(val) : val + ''); + return s.substring(0, max); + } catch (e) { + return ''; + } +} + +export function toInternalError(e: any): InternalError { + return { msg: String(e && e.message || e), stack: e && e.stack }; +} + +export class ErrorReporter { + private sent = 0; + private seen = new Set(); + + constructor( + readonly source: ErrorSource, + private send: (payload: ErrorPayload) => void, + readonly common: ErrorReportFields = {}) { } + + /** + * Report an error once per session: the same message from the same source + * and tool (a build that crashes on every keystroke, say) goes out once. + * Returns true if it was sent. + */ + report(msg: string, stack?: string, fields?: ErrorReportFields, source?: ErrorSource): boolean { + source = source || this.source; + var key = [source, fields && fields.tool, msg].join('|'); + if (this.sent >= MAX_REPORTS_PER_SESSION || this.seen.has(key)) return false; + this.seen.add(key); + this.sent++; + var payload: ErrorPayload = { + source, + msg: clampField(msg, 500), + stack: clampField(stack, MAX_FIELD), + clientTime: new Date().toISOString(), + }; + var extra = { ...this.common, ...fields }; + for (var k in extra) { + if (!(k in payload) && extra[k] != null) payload[k] = clampField(extra[k], MAX_FIELD); + } + this.send(payload); + return true; + } + + reportError(err: any, fields?: ErrorReportFields, source?: ErrorSource): boolean { + var ie = toInternalError(err); + return this.report(ie.msg, ie.stack, fields, source); + } +} diff --git a/src/common/workertypes.ts b/src/common/workertypes.ts index 7316bb12..cbbccf7d 100644 --- a/src/common/workertypes.ts +++ b/src/common/workertypes.ts @@ -1,3 +1,4 @@ +import type { InternalError } from "./telemetry"; export type FileData = string | Uint8Array; @@ -186,6 +187,8 @@ export interface WorkerErrorResult { errors: WorkerError[] listings?: CodeListingMap uppercaseOnly?: boolean + /** set when a tool crashed rather than rejected the program, for error reports */ + internal?: InternalError & { tool: string, platform: string } } export interface WorkerOutputResult { diff --git a/src/ide/errorreport.ts b/src/ide/errorreport.ts index 51fea59a..ceaf400d 100644 --- a/src/ide/errorreport.ts +++ b/src/ide/errorreport.ts @@ -1,79 +1,45 @@ -// Lightweight error reporting to our own server (replaces sentry). -// POSTs a small JSON payload to /error.php, which logs it server-side. -// See web/error.php. No dependencies and fire-and-forget: safe to call -// from error handlers, never throws. Client-side limits below prevent -// flooding. +// Error reporting from the web IDE to our own server (replaces sentry). +// POSTs a small JSON payload to /error.php (see web/error.php), which logs +// it. Fire-and-forget: safe to call from error handlers, never throws. +// ErrorReporter (common/telemetry) limits volume. -import { current_project } from "./ui"; +import { ErrorReporter, ErrorPayload, ErrorReportFields, ErrorSource, InternalError } from "../common/telemetry"; -const MAX_ERRORS_PER_SESSION = 10; -const MIN_MS_BETWEEN_IDENTICAL = 10000; const SEND_URL = '/error.php'; -const MAX_FIELD = 2000; -// Extra context passed by the caller (active IDE window, platform, etc.) -export interface ErrorReportContext { - window?: string; - platform?: string; - [key: string]: any; -} - -let errorsSent = 0; -let lastMsgSent = ''; -let lastMsgTime = 0; - -function clamp(val: any, max: number): string { - try { - if (val == null) return ''; - var s = typeof val === 'string' ? val : (typeof val === 'object' ? JSON.stringify(val) : val + ''); - return s.substring(0, max); - } catch (e) { - return ''; +function send(payload: ErrorPayload) { + payload.url = window.location.href.substring(0, 500); + payload.userAgent = navigator.userAgent.substring(0, 500); + payload.language = navigator.language; + var body = JSON.stringify(payload); + // sendBeacon survives page unload; fall back to fetch with keepalive + if (navigator.sendBeacon) { + navigator.sendBeacon(SEND_URL, new Blob([body], { type: 'application/json' })); + } else { + fetch(SEND_URL, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: body, + keepalive: true, + }).catch(e => console.log('error report failed', e)); } } -export function reportErrorToServer(msg: string, err?: any, context?: ErrorReportContext) { +const reporter = new ErrorReporter('ide', send); + +export function reportErrorToServer(msg: string, err?: any, fields?: ErrorReportFields, source?: ErrorSource) { try { - if (errorsSent >= MAX_ERRORS_PER_SESSION) return; - // don't repeat the same error more than once every 10 seconds - var now = Date.now(); - if (msg === lastMsgSent && now - lastMsgTime < MIN_MS_BETWEEN_IDENTICAL) return; - errorsSent++; - lastMsgSent = msg; - lastMsgTime = now; - - var payload: { [k: string]: string } = { - msg: clamp(msg, 500), - stack: clamp(err && err.stack, 2000), - window: clamp(context && context.window, 200), - platform: clamp(context && context.platform, 200), - url: clamp(window.location.href, 500), - userAgent: clamp(navigator.userAgent, 500), - language: clamp(navigator.language, 50), - clientTime: new Date().toISOString(), - }; - - // copy any extra caller-supplied context fields - if (context) { - for (var k in context) { - if (!(k in payload)) payload[k] = clamp(context[k], MAX_FIELD); - } - } - - var body = JSON.stringify(payload); - - // sendBeacon survives page unload; fall back to fetch with keepalive - if (typeof navigator !== 'undefined' && navigator.sendBeacon) { - navigator.sendBeacon(SEND_URL, new Blob([body], { type: 'application/json' })); - } else if (typeof fetch !== 'undefined') { - fetch(SEND_URL, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: body, - keepalive: true, - }).catch(function () { }); - } + reporter.report(msg, err && err.stack, fields, source); } catch (e) { - // never let error reporting throw + console.log('error report failed', e); + } +} + +/** A build tool crash that the worker returned as `internal` on its result. */ +export function reportInternalError(source: ErrorSource, ie: InternalError, fields?: ErrorReportFields) { + try { + reporter.report(ie.msg, ie.stack, fields, source); + } catch (e) { + console.log('error report failed', e); } } diff --git a/src/ide/ui.ts b/src/ide/ui.ts index 4dc72c80..c5503910 100644 --- a/src/ide/ui.ts +++ b/src/ide/ui.ts @@ -14,8 +14,8 @@ import { } from "../common/util"; import { getSkeletonName, getPlatformToolHelpURL, getToolMeta, TOOL_META } from "../common/toolmeta"; import { PLATFORM_PARAMS } from "../worker/platforms"; -import { CodeListingMap, FileData, WorkerError, WorkerResult } from "../common/workertypes"; -import { reportErrorToServer } from "./errorreport"; +import { CodeListingMap, FileData, WorkerError, WorkerResult, isErrorResult } from "../common/workertypes"; +import { reportErrorToServer, reportInternalError } from "./errorreport"; import { importPlatform } from "../platform/_index"; import { alertError, alertInfo, fatalError, setWaitDialog } from "./dialogs"; import { openSettings, loadSettings } from "./settings"; @@ -253,6 +253,9 @@ async function initProject() { current_project.remoteTool = qs.tool || null; projectWindows = new ProjectWindows($("#workspace")[0] as HTMLElement, current_project); current_project.callbackBuildResult = (result: WorkerResult) => { + if (isErrorResult(result) && result.internal && isProductionHost()) { + reportInternalError('worker', result.internal, { tool: result.internal.tool, platform: result.internal.platform }); + } setCompileOutput(result); }; current_project.callbackBuildStatus = (busy: boolean) => { @@ -2120,7 +2123,7 @@ async function showWelcomeMessage() { element: "#dropdownMenuButton", placement: 'right', title: "Developer Analytics", - content: 'BTW, we send stack traces to sentry.io when exceptions are thrown. Hope that\'s ok.' + content: 'BTW, we send stack traces to 8bitworkshop.com when exceptions are thrown. Hope that\'s ok.' }); steps.unshift({ element: "#dropdownMenuButton", diff --git a/src/tools/8bws.ts b/src/tools/8bws.ts index f5d0aa8b..b3bb6f7f 100644 --- a/src/tools/8bws.ts +++ b/src/tools/8bws.ts @@ -14,7 +14,7 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import { isProbablyBinary } from '../common/util'; -import { fail, hasOutput, note, output, setJsonMode, setServerMode } from './cliformat'; +import { fail, hasOutput, note, output, setJsonMode, setServerMode, warn } from './cliformat'; import { EmuTarget, loadPlatform } from './emutarget'; import { RUN_SCRIPT_HELP, RunScript, parseNum } from './runscript'; import { verifyReplay } from './verifyreplay'; @@ -133,8 +133,12 @@ async function compileSource(args: Args, source: string, platform: string): Prom } await preload(tool, platform); const result = await compileSourceFile(tool, platform, source, undefined, buildOverrides(args)); + if (result.internal) { + warn(`${result.internal.tool} crashed; the IDE would send an error report: ${result.internal.msg}`); + if (process.env.DEBUG) console.error(result.internal.stack); + } if (!result.success) { - fail('build', `${tool} failed on ${source}`, { errors: result.errors }); + fail('build', `${tool} failed on ${source}`, { errors: result.errors, internal: result.internal }); } return { rom: romBytes(result), symbolmap: result.symbolmap || {}, tool, platform, source, result }; } diff --git a/src/tools/buildpresets.ts b/src/tools/buildpresets.ts index e72a849d..64ae33c8 100644 --- a/src/tools/buildpresets.ts +++ b/src/tools/buildpresets.ts @@ -69,6 +69,7 @@ export interface PresetResult { size?: number; // bytes of output, when it built ms: number; errors?: string[]; + internal?: string; // the tool crashed: the IDE would send an error report log?: string; // what the tool printed, kept only when it failed run?: RunResult; // present when built with --run } @@ -570,7 +571,8 @@ export async function buildPreset( } const ms = Date.now() - started; - if (errors.length) return { preset, platform, tool, ok: false, ms, errors, log: log || undefined }; + const internal = result?.internal ? `${result.internal.tool}: ${result.internal.msg}` : undefined; + if (errors.length) return { preset, platform, tool, ok: false, ms, errors, internal, log: log || undefined }; const built: PresetResult = { preset, platform, tool, ok: true, size: outputSize(result), ms }; if (opts.run) built.run = await runPreset(result, preset, platform, opts); return built; @@ -694,6 +696,7 @@ async function main() { const runText = r.run ? ' ' + runStatus(r.run) : ''; console.log(`${status} ${r.preset} ${cyan(`[${r.tool}/${r.platform}]`)} ${dim(size)} ${dim(r.ms + 'ms')}${runText}`); for (const e of r.errors || []) console.log(` ${yellow(e)}`); + if (r.internal) console.log(` ${red(bold('CRASH'))} ${red(r.internal)} ${dim('(the IDE would report this)')}`); if (r.run?.png) console.log(` ${dim(r.run.png)}`); // only a failure gets the tool's / platform's own output if (r.log) console.log(dim(r.log.split('\n').map((l) => ' | ' + l).join('\n'))); @@ -717,6 +720,11 @@ async function main() { console.log('\nby tool: ' + Object.keys(byTool).sort() .map((t) => `${cyan(t)}=${byTool[t]}`).join(' ')); } + const crashed = results.filter((r) => r.internal); + if (crashed.length) { + console.log('\n' + bold(red(`${crashed.length} tool crash(es)`)) + dim(' -- would be sent as error reports:')); + for (const r of crashed) console.log(` ${red(r.preset)} ${cyan(`[${r.tool}/${r.platform}]`)} ${r.internal}`); + } // what the builds did when loaded, separately from whether they built const ran = results.filter((r) => r.run); if (ran.length) { diff --git a/src/tools/cliformat.ts b/src/tools/cliformat.ts index 5df73a62..a1925726 100644 --- a/src/tools/cliformat.ts +++ b/src/tools/cliformat.ts @@ -66,6 +66,11 @@ export function note(msg: string): void { if (!jsonMode || serving) process.stderr.write(`${c.dim}${msg}${c.reset}\n`); } +/** A problem worth fixing that doesn't stop the command; stderr in any mode. */ +export function warn(msg: string): void { + process.stderr.write(`${c.yellow}Warning: ${msg}${c.reset}\n`); +} + let emitted = false; /** True once a CLIResult has been printed -- lets main() detect a silent exit. */ diff --git a/src/tools/testlib.ts b/src/tools/testlib.ts index eacbc110..13fb7f0a 100644 --- a/src/tools/testlib.ts +++ b/src/tools/testlib.ts @@ -5,7 +5,7 @@ import * as fs from 'fs'; import * as path from 'path'; -import type { WorkerResult, WorkerMessage, BuildArgLists, BuildSymbolLists, FileData } from "../common/workertypes"; +import type { WorkerResult, WorkerErrorResult, WorkerMessage, BuildArgLists, BuildSymbolLists, FileData } from "../common/workertypes"; import { isProbablyBinary, getBasePlatform } from "../common/util"; import { getToolForPlatform } from "../common/toolselect"; import { FileProvider, buildWorkerMessage, resolveDependencies } from "../common/projectcore"; @@ -40,6 +40,8 @@ export interface CompileResult { unchanged?: boolean; /** every file the build read, by worker filename (verilog's $readmem reads them at load time) */ files?: { [path: string]: FileData }; + /** the tool crashed: the IDE and extension report this to us (common/telemetry) */ + internal?: WorkerErrorResult['internal']; } let initialized = false; @@ -213,6 +215,7 @@ function workerResultToCompileResult(result: WorkerResult): CompileResult { return { success: false, errors: result.errors, + internal: result.internal, }; } if ('output' in result) { diff --git a/src/worker/builder.ts b/src/worker/builder.ts index 143555b8..0ae3674c 100644 --- a/src/worker/builder.ts +++ b/src/worker/builder.ts @@ -1,6 +1,7 @@ import { getPlatformToolConfig, getToolMeta } from "../common/toolmeta"; import { convertDataToUint8Array, getBasePlatform } from "../common/util"; import { WorkerBuildStep, WorkerError, WorkerErrorResult, WorkerMessage, WorkerResult, WorkingStore } from "../common/workertypes"; +import { toInternalError } from "../common/telemetry"; import { PLATFORM_PARAMS } from "./platforms"; import { TOOLS } from "./workertools"; @@ -118,6 +119,22 @@ export function errorResult(msg: string): WorkerErrorResult { return { errors: [{ line: 0, msg: msg }] }; } +/** A tool crash: shown like any build error, and flagged for error reports. */ +export function internalErrorResult(e: any, tool: string, platform: string): WorkerErrorResult { + var result = errorResult(e + ""); + result.internal = { tool, platform, ...toInternalError(e) }; + return result; +} + +/** A build failure caused by the user's project (a bad directive, a missing + * file), as opposed to a tool crash, which gets reported to us. */ +export class BuildError extends Error { + constructor(msg: string) { + super(msg); + Object.setPrototypeOf(this, BuildError.prototype); + } +} + export class Builder { steps: BuildStep[] = []; startseq: number = 0; @@ -167,8 +184,9 @@ export class Builder { applyPlatformAndStepParams(step); step.result = await toolfn(step); } catch (e) { + if (e instanceof BuildError) return errorResult(e.message); console.log("EXCEPTION", e, e.stack); - return errorResult(e + ""); // TODO: catch errors already generated? + return internalErrorResult(e, tool, platform); } if (step.result) { (step.result as any).params = step.params; // TODO: type check @@ -566,7 +584,7 @@ function applyPlatformAndStepParams(step: BuildStep) { mergeDirectives(dir, directivesFromOverrides(symbols, buildArgs)); applyBuildDirectives(dir, params); if (dir.errors.length) - throw new Error('build config error: ' + dir.errors.join('; ')); + throw new BuildError('build config error: ' + dir.errors.join('; ')); } /** Split a directive body into argv, honoring single/double quotes. */ @@ -775,7 +793,7 @@ export function fixParamsWithDefines(path: string, params) { applyBuildDirectives(dir, params); // malformed directives are a build-config error, not a silent no-op if (dir.errors.length) - throw new Error('build directive error: ' + dir.errors.join('; ')); + throw new BuildError('build directive error: ' + dir.errors.join('; ')); } export function processEmbedDirective(code: string) { @@ -791,7 +809,7 @@ export function processEmbedDirective(code: string) { let filename = m[1]; let filedata = store.getFileData(filename); let bytes = convertDataToUint8Array(filedata); - if (!bytes) throw new Error('#embed: file not found: "' + filename + '"'); + if (!bytes) throw new BuildError('#embed: file not found: "' + filename + '"'); let out = ''; for (let i = 0; i < bytes.length; i++) { out += bytes[i].toString() + ','; diff --git a/src/worker/workermain.ts b/src/worker/workermain.ts index 78a05d48..bedfd768 100644 --- a/src/worker/workermain.ts +++ b/src/worker/workermain.ts @@ -1,7 +1,7 @@ import type { WorkerResult, WorkerMessage, WorkerError, SourceLine } from "../common/workertypes"; import { getSharedFileSystemName } from "../common/toolmeta"; -import { store, builder, errorResult, getWorkFileAsString, readLibraryHeader } from "./builder"; +import { store, builder, internalErrorResult, getWorkFileAsString, readLibraryHeader } from "./builder"; import { emglobal, fsMeta, loadFilesystem, listSharedFiles, readSharedFile, ensureFilesystem, readWasiSharedFile, listWasiSharedFiles, ensureWasiFilesystem } from "./wasmutils"; // shared FS names starting with 'wasi:' refer to a WASI filesystem zip @@ -79,7 +79,14 @@ if (ENVIRONMENT_IS_WORKER) { var lastpromise = null; onmessage = async function (e) { await lastpromise; // wait for previous message to complete - lastpromise = handleMessage(e.data); + var data = e.data as WorkerMessage; + lastpromise = handleMessage(data).catch(err => { + // otherwise the IDE waits forever for a result + console.log(err); + var r: any = internalErrorResult(err, 'worker', data.platform || ''); + if (data.qid != null) r.qid = data.qid; + return r as WorkerResult; + }); var result = await lastpromise; lastpromise = null; if (result) { @@ -87,7 +94,7 @@ if (ENVIRONMENT_IS_WORKER) { postMessage(result); } catch (e) { console.log(e); - postMessage(errorResult(`${e}`)); + postMessage(internalErrorResult(e, 'worker', '')); } } } diff --git a/test/unit/testtelemetry.ts b/test/unit/testtelemetry.ts new file mode 100644 index 00000000..218cf046 --- /dev/null +++ b/test/unit/testtelemetry.ts @@ -0,0 +1,96 @@ +import assert from "assert"; +import fs from "fs"; +import os from "os"; +import path from "path"; +import { after, before, describe, it } from "mocha"; +import { ErrorPayload, ErrorReporter, MAX_REPORTS_PER_SESSION } from "../../src/common/telemetry"; +import { fixParamsWithDefines, gatherFiles } from "../../src/worker/builder"; +import { TOOLS } from "../../src/worker/workertools"; +import { compileSourceFile } from "../../src/tools/testlib"; + +describe("ErrorReporter", function () { + function reporter() { + const sent: ErrorPayload[] = []; + return { sent, r: new ErrorReporter('ide', p => sent.push(p), { version: '1.2' }) }; + } + + it("sends source, message, and fields", function () { + const { sent, r } = reporter(); + assert.ok(r.report("boom", "at foo", { tool: "cc65", platform: "nes" }, 'worker')); + assert.strictEqual(sent.length, 1); + assert.strictEqual(sent[0].source, 'worker'); + assert.strictEqual(sent[0].msg, 'boom'); + assert.strictEqual(sent[0].stack, 'at foo'); + assert.strictEqual(sent[0].tool, 'cc65'); + assert.strictEqual(sent[0].version, '1.2'); + }); + + it("sends the same error once per session", function () { + const { sent, r } = reporter(); + r.report("boom", "", { tool: "cc65" }); + assert.ok(!r.report("boom", "", { tool: "cc65" })); + // a different tool is a different error + assert.ok(r.report("boom", "", { tool: "sdcc" })); + assert.strictEqual(sent.length, 2); + }); + + it("stops after the session limit", function () { + const { sent, r } = reporter(); + for (let i = 0; i < MAX_REPORTS_PER_SESSION + 5; i++) r.report("error " + i); + assert.strictEqual(sent.length, MAX_REPORTS_PER_SESSION); + }); + + it("clamps long fields", function () { + const { sent, r } = reporter(); + r.report("x".repeat(10000), "y".repeat(10000)); + assert.strictEqual(sent[0].msg.length, 500); + assert.strictEqual(sent[0].stack.length, 2000); + }); +}); + +describe("Build tool crashes", function () { + let dir: string; + const tools = TOOLS as any; + + before(function () { + dir = fs.mkdtempSync(path.join(os.tmpdir(), "8bws-telemetry-")); + tools.test_crash = () => { throw new TypeError("cannot read properties of undefined"); }; + // a malformed directive: the user's mistake, which a real tool reports the same way + tools.test_usererror = (step: any) => { + gatherFiles(step); + fixParamsWithDefines(step.path, step.params); + return { output: new Uint8Array(1) }; + }; + }); + + after(function () { + delete tools.test_crash; + delete tools.test_usererror; + fs.rmSync(dir, { recursive: true, force: true }); + }); + + function source(name: string, code: string) { + const fn = path.join(dir, name); + fs.writeFileSync(fn, code); + return fn; + } + + it("flags an exception in a tool as internal", async function () { + const result = await compileSourceFile("test_crash", "nes", source("crash.c", "int main;\n")); + assert.ok(!result.success); + assert.ok(result.internal, "expected internal"); + assert.strictEqual(result.internal.tool, "test_crash"); + assert.strictEqual(result.internal.platform, "nes"); + assert.match(result.internal.msg, /cannot read properties/); + assert.match(result.internal.stack, /TypeError/); + // the user still sees it as a build error + assert.match(result.errors[0].msg, /cannot read properties/); + }); + + it("doesn't flag a bad build directive", async function () { + const result = await compileSourceFile("test_usererror", "nes", source("bad.c", '//#symbol ld BAD="str"\n')); + assert.ok(!result.success); + assert.match(result.errors[0].msg, /build directive error/); + assert.strictEqual(result.internal, undefined); + }); +});