From 08427931b676e7caab40b20217526f97cec17d0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Mon, 20 Apr 2026 12:14:55 -0400 Subject: [PATCH] Introduce AppError taxonomy with 4 kinds (C6.1 part 1/2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Establishes `src/main/errors/` with: - `AppError` base + `Recoverable|UserInput|Upstream|Fatal` subclasses keyed on a narrow `ErrorKind = "recoverable" | "user_input" | "upstream" | "fatal"` union. Four kinds is deliberate: adding a fifth is a conscious proposal, not an inline decision at a catch site. - `classifyUnknownError(e, fallback)` — required-fallback-meta wrapper that refuses to invent classification. If `e` is already an AppError it is returned untouched; otherwise the call site's intent wins and the raw value is preserved via `cause` (for Error instances) or `context.raw` (for anything else). - `logAppError(logger, raw, fallback, level?)` helper emits one structured log line and returns the classified AppError so the caller can still branch on `err.kind`. Why this module exists: 248 catch blocks in src/main/ today are overwhelmingly `} catch {}` or `log.warn(); return default`. Behaviour is correct case-by-case but the codebase has no machine-readable signal for "is this recoverable vs fatal", making it impossible to write graceful-shutdown logic, telemetry buckets, or catch-site audits. Part 2 migrates the top-20 high-frequency catch sites. Part 3 (tracked as C6.2 proposal) covers the remaining 228. No behavioural change in this commit — it only adds the classifier and exports. --- src/main/errors/handlers.ts | 48 ++++++++ src/main/errors/index.ts | 21 ++++ src/main/errors/taxonomy.ts | 216 ++++++++++++++++++++++++++++++++++++ 3 files changed, 285 insertions(+) create mode 100644 src/main/errors/handlers.ts create mode 100644 src/main/errors/index.ts create mode 100644 src/main/errors/taxonomy.ts diff --git a/src/main/errors/handlers.ts b/src/main/errors/handlers.ts new file mode 100644 index 00000000..1521a16b --- /dev/null +++ b/src/main/errors/handlers.ts @@ -0,0 +1,48 @@ +/** + * Handler helpers that sit on top of the AppError taxonomy (C6.1 part 1/2). + * + * These are thin conveniences, not a framework. Every helper here reduces + * two lines of boilerplate at the call site; it does NOT swap in default + * values or change observable behaviour. + */ + +import { classifyUnknownError, type AppError, type AppErrorMeta } from "./taxonomy.js"; + +export interface Logger { + error: (...args: unknown[]) => void; + warn?: (...args: unknown[]) => void; +} + +/** + * Classify an unknown thrown value and emit a single structured log line. + * Returns the `AppError` so the caller can still branch on `err.kind` + * (e.g. throw for `fatal`, return default for `recoverable`). + * + * No fallback data is invented: this helper only formats a log line; the + * caller is still responsible for what to do next. + */ +export function logAppError( + logger: Logger, + raw: unknown, + fallback: AppErrorMeta, + level: "error" | "warn" = "error", +): AppError { + const err = classifyUnknownError(raw, fallback); + const line = `[${err.code}] ${err.message}`; + const detail = err.context ?? {}; + const causeDetail = + err.appCause instanceof Error + ? err.appCause.message + : err.appCause !== undefined + ? String(err.appCause) + : undefined; + const parts: unknown[] = [line]; + if (Object.keys(detail).length > 0) parts.push(detail); + if (causeDetail) parts.push(`cause=${causeDetail}`); + if (level === "warn" && typeof logger.warn === "function") { + logger.warn(...parts); + } else { + logger.error(...parts); + } + return err; +} diff --git a/src/main/errors/index.ts b/src/main/errors/index.ts new file mode 100644 index 00000000..71983ae6 --- /dev/null +++ b/src/main/errors/index.ts @@ -0,0 +1,21 @@ +/** + * Error taxonomy entry point (C6.1). + * + * Re-exports the AppError hierarchy and helpers. Import from this path + * everywhere inside src/main/ so the rest of the tree never reaches + * directly into `./taxonomy.ts` or `./handlers.ts`. + */ + +export { + AppError, + RecoverableError, + UserInputError, + UpstreamError, + FatalError, + classifyUnknownError, + isAppError, + type AppErrorMeta, + type ErrorKind, +} from "./taxonomy.js"; + +export { logAppError, type Logger } from "./handlers.js"; diff --git a/src/main/errors/taxonomy.ts b/src/main/errors/taxonomy.ts new file mode 100644 index 00000000..46bf3b6e --- /dev/null +++ b/src/main/errors/taxonomy.ts @@ -0,0 +1,216 @@ +/** + * AppError taxonomy (C6.1 part 1/2). + * + * Purpose: give every catch site in src/main/ a stable, auditable + * classification. Today's 248 catch blocks in main/ are overwhelmingly + * untyped `} catch {}` or `} catch (e) { log.warn(...); return default }` + * — behaviour is correct case-by-case but the codebase has no machine + * readable signal for "is this recoverable vs fatal", so: + * + * - graceful-shutdown logic cannot tell a user-input 400 apart from + * an upstream 502 apart from a corrupted SQLite file; + * - telemetry cannot bucket errors by kind; + * - audits of "which catches silently swallow fatal conditions" have + * to be by-hand grep. + * + * Design invariants (Karpathy "let it crash" + Linus "no silent defaults"): + * + * 1. `classifyUnknownError` never invents classification. The caller + * MUST pass a fallback meta describing the *expected* worst case; + * the helper only honours it when the raw error doesn't already + * carry an AppError classification. + * 2. No fallback defaults on construction. `kind` and `code` are + * required; there is no "unknown" catch-all kind because that + * would reintroduce the exact silent-fallthrough pattern this + * module exists to replace. + * 3. The taxonomy is intentionally narrow: 4 kinds. Adding a 5th is + * a conscious proposal, not a one-off at a call site. + * + * Kinds: + * + * - `recoverable`: expected transient failure the caller can work + * around without human involvement. Example: JSON parse of a stale + * cache entry; re-read or skip. + * - `user_input`: caller-supplied input is invalid. Example: user + * pasted a malformed URL; surface to UI, do not retry. + * - `upstream`: downstream service/dependency failed (HTTP 5xx, + * tool provider down, disk IO). Retriable at a higher level or + * must be propagated to the user with context. + * - `fatal`: process-level invariant broken (DB corruption, missing + * required config at runtime). Should abort the current operation + * and ideally the process. + */ + +export type ErrorKind = "recoverable" | "user_input" | "upstream" | "fatal"; + +export interface AppErrorMeta { + kind: ErrorKind; + /** + * Machine-readable stable identifier for this error site. + * Convention: `module.subdomain.condition`, e.g. `db.lock.busy`, + * `ipc.payload.invalid`, `skills.yaml.parse_failed`. + */ + code: string; + /** Human-readable summary suitable for logs. */ + message: string; + /** Original thrown value, preserved for cause-chain inspection. */ + cause?: unknown; + /** Arbitrary structured context captured at the catch site. */ + context?: Record; +} + +/** + * Canonical error carrier for the AgentCompany main process. + * + * Subclasses below exist only so `instanceof RecoverableError` etc. work + * ergonomically at call sites; the behavioural difference lives entirely + * in `kind`. + */ +export class AppError extends Error { + readonly kind: ErrorKind; + readonly code: string; + readonly context?: Record; + // Error has `cause` as optional since ES2022; we mirror it explicitly to + // give a stable accessor regardless of lib version. + readonly appCause: unknown; + + constructor(meta: AppErrorMeta) { + super(meta.message); + this.name = "AppError"; + this.kind = meta.kind; + this.code = meta.code; + this.context = meta.context; + this.appCause = meta.cause; + } + + /** + * Structured envelope suitable for log lines or IPC payloads. + */ + toJSON(): { + name: string; + kind: ErrorKind; + code: string; + message: string; + context?: Record; + cause?: string; + } { + const payload: { + name: string; + kind: ErrorKind; + code: string; + message: string; + context?: Record; + cause?: string; + } = { + name: this.name, + kind: this.kind, + code: this.code, + message: this.message, + }; + if (this.context) payload.context = this.context; + if (this.appCause !== undefined) { + payload.cause = + this.appCause instanceof Error + ? this.appCause.message + : String(this.appCause); + } + return payload; + } +} + +export class RecoverableError extends AppError { + constructor(meta: Omit) { + super({ ...meta, kind: "recoverable" }); + this.name = "RecoverableError"; + } +} + +export class UserInputError extends AppError { + constructor(meta: Omit) { + super({ ...meta, kind: "user_input" }); + this.name = "UserInputError"; + } +} + +export class UpstreamError extends AppError { + constructor(meta: Omit) { + super({ ...meta, kind: "upstream" }); + this.name = "UpstreamError"; + } +} + +export class FatalError extends AppError { + constructor(meta: Omit) { + super({ ...meta, kind: "fatal" }); + this.name = "FatalError"; + } +} + +/** + * Wrap an unknown thrown value into an AppError. + * + * Three cases, resolved in order: + * + * 1. `e` is already an AppError → return it unchanged. The original + * classification (often narrower than the caller's guess) wins. + * 2. `e instanceof Error` → construct an AppError with the caller's + * fallback meta and attach `e` as `cause`. The original + * `Error.message` is preserved in the cause chain; `meta.message` + * wins at the AppError level because it's the site-specific + * description. + * 3. anything else (string, number, plain object, undefined) → + * construct with fallback meta and stash the raw value in + * `context.raw` for debugging. + * + * The `fallback` argument is required on purpose: the call site is the + * only place that knows the semantic intent of the surrounding code, so + * it must commit to a kind/code/message. A silent default here would + * reintroduce the exact "mystery-shaped error" problem this helper is + * designed to eliminate. + */ +export function classifyUnknownError( + e: unknown, + fallback: AppErrorMeta, +): AppError { + if (e instanceof AppError) { + return e; + } + if (e instanceof Error) { + return new AppError({ + ...fallback, + cause: e, + }); + } + return new AppError({ + ...fallback, + context: { + ...(fallback.context ?? {}), + raw: stringifyRaw(e), + }, + }); +} + +function stringifyRaw(value: unknown): string { + if (value === undefined) return "undefined"; + if (value === null) return "null"; + if (typeof value === "string") return value; + if (typeof value === "number" || typeof value === "boolean") { + return String(value); + } + try { + return JSON.stringify(value); + } catch { + // The raw value is a cyclic / un-serialisable object. `String(value)` + // falls back to "[object Object]" which is still better than losing + // the fact that something was thrown. + return String(value); + } +} + +/** + * Convenience narrowing predicate. Intentional: avoids forcing every + * consumer to re-import `AppError` just to write an `instanceof` guard. + */ +export function isAppError(value: unknown): value is AppError { + return value instanceof AppError; +}