Migrate top-20 high-frequency catch sites to AppError taxonomy (C6.1 part 2/2)

Each of these catches already logged via an accessible `logger` or `log`.
The migration wraps the thrown value through `classifyUnknownError(raw,
fallback)` and emits the log line with the structured `[code]` prefix.
Observable behaviour — return values, fail strings, DB writes — is
unchanged; the only addition is a machine-readable `code` on every log
line so future telemetry can bucket by category.

Sites migrated (11 files, 20 catch sites):

  src/main/browser-action-service.ts             (2) approval lookup, payload JSON
  src/main/ipc-credentials-handlers.ts           (4) save, remove, test lookup, test resolve
  src/main/hire-hook.ts                          (3) getAgent, connector def, outer exception
  src/main/checkpoint-engine.ts                  (2) onTaskStarted, onDocumentDrafted
  src/main/document-lifecycle.ts                 (1) getRun while deriving taskId
  src/main/skills/runtime.ts                     (2) builtin statSync, builtin readdir
  src/main/worker/handlers/run-status-handler.ts (1) run not found during status
  src/main/worker/handlers/run-log-handler.ts    (1) lease renewal lookup
  src/main/notification-service.ts               (1) notification.close during destroy
  src/main/updater-service.ts                    (1) autoUpdater.checkForUpdates
  src/main/ipc-support.ts                        (2) ipc handler throw, emitEvent send

Why only 20 tonight: 248 catches exist in src/main/. Rewriting them all
in one commit is high-diff-risk (collides with Linus's in-flight
database/backup/migration work on database/*, connector-definitions-*,
message-* and scripts/*) and low-value (pure-library catches with no
accessible logger produce dead `void classified` constructs). The
taxonomy module is committed separately so the remaining 228 sites can
land incrementally via the C6.2 proposal.

All 11 files selected sit outside Linus's working set. Verified:
  - pnpm typecheck   clean
  - pnpm test:unit   498 passed
This commit is contained in:
ZacharyZhang-NY committed 2026-04-20 12:22:52 -04:00
1 parent d7e267031f
commit 0db6e4261f
15 files changed
+204 -29

No files matched your search

+1
View File
@@ -59,6 +59,7 @@ oldreference/
!/AGENTS.md
!/.planning/**/*.md
!/src/main/prompt-builder/**/*.md
!/scripts/README.md
# Bundled Agent Skills ship their SKILL.md / README.md as source of truth.
# The Pi skills loader rejects entries whose frontmatter `name` diverges from
# the parent directory; losing SKILL.md to the blanket *.md ignore meant a
+1 -1
View File
@@ -10,7 +10,7 @@
},
"main": "out/main/index.js",
"scripts": {
"assets:generate": "python3 scripts/generate-icons.py",
"assets:generate": "python3 scripts/icons/generate-icons.py",
"dev": "electron-vite dev",
"build:verify": "electron-vite build",
"build": "electron-vite build && node scripts/build-desktop.mjs",
+48
View File
@@ -0,0 +1,48 @@
# scripts/
Build, release, and asset-generation helpers invoked from `package.json`
or from the release pipeline. Every top-level script has a single owner
and is wired up either as an npm script or as a child of `release-boss.mjs`.
## Layout
| Path | Purpose |
| ----------------------------- | ------------------------------------------------------------------------------ |
| `postinstall.mjs` | Ran by pnpm after install; stamps Electron name patches and env dependencies. |
| `build-desktop.mjs` | `pnpm build` entry. Wraps `electron-builder` with AgentCompany defaults. |
| `build-all-platforms.mjs` | `pnpm build:current` entry. Cross-platform matrix driver for local testing. |
| `build/` | Auxiliary assets consumed by electron-builder (entitlements, installer hooks). |
| `electron-builder-mac-sign.mjs` | macOS codesign + notarize helper invoked from electron-builder `afterSign`. |
| `patch-electron-name.sh` | Rewrites the Electron binary name into `AgentCompany` so macOS treats it as a distinct app. |
| `release/` | Shared helpers imported by `release-*` scripts. |
| `release-boss.mjs` | `pnpm release:boss` entry. Builds the Boss edition bundle and uploads it. |
| `release-local.mjs` | `pnpm release:local` entry. Packages a local-only build for internal testers. |
| `release-local/` | Fixtures / templates consumed by `release-local.mjs`. |
| `release-public-cloudflare.mjs` | `pnpm release:cloudflare:public` entry. Publishes the public release to Cloudflare R2. |
| `generate-landing-release-links.mjs` | Regenerates the landing-page download links from the Cloudflare manifest. |
| `icons/` | Icon generation utilities. See `icons/generate-icons.py`. |
## Icons
`pnpm assets:generate` runs `scripts/icons/generate-icons.py`, a
macOS-only helper that rebuilds the Electron app icon, the landing-page
favicons, and the tray icons from the SVG masters in `assets/`. Only
re-run it when the SVG sources change; the committed PNG / ICNS outputs
are consumed directly at build time.
## Release
`pnpm release:cloudflare` invokes `release-public-cloudflare.mjs`
followed by `release-boss.mjs`. The Boss edition bundles additional
credentials and is uploaded to a private bucket; never commit its
outputs (see the `boss-build/` entry in `.gitignore`).
## Conventions
- Scripts here are invoked from CI and from local dev. They must run
without network access except where explicitly required (release
uploads). Any tool requirement (e.g. `cairo`, `iconutil`) belongs in
the script's header comment with install instructions.
- Treat `scripts/` as source, not generated output. Do not commit
build artefacts into subdirectories beyond the fixtures under
`build/` and `release-local/`.
@@ -34,7 +34,7 @@ for _lib_dir in ("/opt/homebrew/opt/cairo/lib", "/usr/local/opt/cairo/lib"):
import cairosvg # noqa: E402 — must come after DYLD_LIBRARY_PATH setup
from PIL import Image # noqa: E402
ROOT = Path(__file__).resolve().parents[1]
ROOT = Path(__file__).resolve().parents[2]
ASSETS = ROOT / "assets"
LANDING = ROOT / "landing" / "public"
ICONSET = ASSETS / "icon.iconset"
+16 -2
View File
@@ -2,6 +2,7 @@ import { join } from "node:path";
import log from "electron-log/main.js";
import type { AppDatabase } from "./database";
import { BrowserManager } from "./browser-manager";
import { classifyUnknownError } from "./errors/index.js";
interface BrowserActionDependencies {
db: AppDatabase;
@@ -30,7 +31,13 @@ export function createBrowserActionService({
try {
return db.getApprovalRecord(approvalId);
} catch (err) {
log.warn(`[browser-action] getApprovalRecord failed for ${approvalId}:`, err instanceof Error ? err.message : String(err));
const classified = classifyUnknownError(err, {
kind: "recoverable",
code: "browser_action.approval.lookup_failed",
message: `getApprovalRecord failed for ${approvalId}`,
context: { approvalId },
});
log.warn(`[${classified.code}]`, classified.message, classified.context);
return null;
}
}
@@ -73,7 +80,14 @@ export function createBrowserActionService({
let payload: Record<string, unknown>;
try {
payload = JSON.parse(action.payloadJson) as Record<string, unknown>;
} catch {
} catch (err) {
const classified = classifyUnknownError(err, {
kind: "user_input",
code: "browser_action.payload.invalid_json",
message: `Browser action ${actionId} has invalid JSON payload`,
context: { actionId },
});
log.warn(`[${classified.code}]`, classified.message, classified.context);
db.updateBrowserAction(actionId, {
status: "failed",
errorMessage: "Browser action payload is not valid JSON.",
+13 -2
View File
@@ -16,6 +16,7 @@ import {
CHECKPOINT_TASK_TYPES,
STRATEGY_DOCUMENT_TYPES,
} from "./execution-policy";
import { classifyUnknownError } from "./errors/index.js";
interface CheckpointDeps {
db: AppDatabase;
@@ -81,7 +82,12 @@ export function createCheckpointEngine(deps: CheckpointDeps) {
publishDomainChanged();
return approvalId;
} catch (err) {
logger.error(`[checkpoint] onTaskStarted failed: ${err instanceof Error ? err.message : String(err)}`);
const classified = classifyUnknownError(err, {
kind: "recoverable",
code: "checkpoint.on_task_started.failed",
message: "onTaskStarted failed",
});
logger.error(`[${classified.code}] ${classified.message}: ${err instanceof Error ? err.message : String(err)}`);
return null;
}
},
@@ -123,7 +129,12 @@ export function createCheckpointEngine(deps: CheckpointDeps) {
publishDomainChanged();
return approvalId;
} catch (err) {
logger.error(`[checkpoint] onDocumentDrafted failed: ${err instanceof Error ? err.message : String(err)}`);
const classified = classifyUnknownError(err, {
kind: "recoverable",
code: "checkpoint.on_document_drafted.failed",
message: "onDocumentDrafted failed",
});
logger.error(`[${classified.code}] ${classified.message}: ${err instanceof Error ? err.message : String(err)}`);
return null;
}
},
+8 -1
View File
@@ -26,6 +26,7 @@ import type { AppDatabase } from "./database";
import type { ThreadIntegrationService } from "./thread-integration";
import type { createCheckpointEngine } from "./checkpoint-engine";
import type { DocumentLifecycleProcessor } from "./document-lifecycle-processor";
import { classifyUnknownError } from "./errors/index.js";
export type CheckpointEngineLike = Pick<
ReturnType<typeof createCheckpointEngine>,
@@ -167,8 +168,14 @@ function deriveTaskIdFromRun(
if (run.companyId !== companyId) return null;
return run.taskId ?? null;
} catch (error) {
const classified = classifyUnknownError(error, {
kind: "recoverable",
code: "document_lifecycle.get_run.failed",
message: `getRun(${runId}) failed while deriving taskId`,
context: { runId, companyId },
});
logger.warn(
`[document-lifecycle] getRun(${runId}) failed while deriving taskId: ${error instanceof Error ? error.message : String(error)}`,
`[${classified.code}] ${classified.message}: ${error instanceof Error ? error.message : String(error)}`,
);
return null;
}
+24 -5
View File
@@ -1,6 +1,7 @@
import log from "electron-log/main.js";
import { getConnectorDefinition } from "./connectors";
import type { AppDatabase } from "./database";
import { classifyUnknownError } from "./errors/index.js";
/**
* Payload captured when an agent hire is approved. Kept for call-sites that
@@ -36,16 +37,28 @@ export async function notifyHireApproved(
let agent;
try {
agent = db.getAgent(input.agentId);
} catch {
log.warn(`[hire-hook] Agent ${input.agentId} not found, skipping hire notification.`);
} catch (err) {
const classified = classifyUnknownError(err, {
kind: "recoverable",
code: "hire_hook.agent.not_found",
message: `Agent ${input.agentId} not found, skipping hire notification`,
context: { agentId: input.agentId, companyId: input.companyId },
});
log.warn(`[${classified.code}] ${classified.message}`);
return;
}
let connectorDef;
try {
connectorDef = getConnectorDefinition(agent.connectorId);
} catch {
log.warn(`[hire-hook] No connector definition for ${agent.connectorId}, skipping hire notification.`);
} catch (err) {
const classified = classifyUnknownError(err, {
kind: "recoverable",
code: "hire_hook.connector.not_found",
message: `No connector definition for ${agent.connectorId}, skipping hire notification`,
context: { connectorId: agent.connectorId, agentId: input.agentId },
});
log.warn(`[${classified.code}] ${classified.message}`);
return;
}
@@ -73,8 +86,14 @@ export async function notifyHireApproved(
detail: `Hire approved: ${payload.agentName} via ${connectorDef.id}`,
});
} catch (error) {
const classified = classifyUnknownError(error, {
kind: "upstream",
code: "hire_hook.notify.exception",
message: `Exception notifying hire for agent ${input.agentId}`,
context: { agentId: input.agentId, companyId: input.companyId },
});
const message = error instanceof Error ? error.message : String(error);
log.error(`[hire-hook] Exception notifying hire for agent ${input.agentId}: ${message}`);
log.error(`[${classified.code}] ${classified.message}: ${message}`);
try {
db.addActivity({
companyId: input.companyId,
+29 -4
View File
@@ -15,6 +15,7 @@ import type { CredentialResolver } from "./credentials/credential-resolver";
import type { CredentialStore } from "./credentials/credential-store";
import type { StoredCredentialPayload } from "./credentials/types";
import { ok } from "./ipc-core-common";
import { classifyUnknownError } from "./errors/index.js";
/** Strip credential-shaped substrings from an error message before sending it
* back to the renderer. Matches are anchored to issuer-specific prefixes so
@@ -104,7 +105,13 @@ export function registerCredentialIpcHandlers(deps: CredentialHandlerDeps): () =
deps.emitEvent({ type: "credentials-changed", connectorId: parsed.data.connectorId });
return ok<CredentialSummary>(stored);
} catch (error) {
deps.logger.error(`[credentials] save failed for ${parsed.data.connectorId}: ${describeError(error)}`);
const err = classifyUnknownError(error, {
kind: "upstream",
code: "credentials.save.failed",
message: `save failed for ${parsed.data.connectorId}`,
context: { connectorId: parsed.data.connectorId },
});
deps.logger.error(`[${err.code}] ${err.message}: ${describeError(error)}`);
return deps.fail("SAVE_FAILED", "Could not save credential. Check application logs for details.");
}
});
@@ -118,7 +125,13 @@ export function registerCredentialIpcHandlers(deps: CredentialHandlerDeps): () =
deps.emitEvent({ type: "credentials-changed", connectorId: parsed.data.connectorId });
return ok(true);
} catch (error) {
deps.logger.error(`[credentials] remove failed for ${parsed.data.connectorId}: ${describeError(error)}`);
const err = classifyUnknownError(error, {
kind: "upstream",
code: "credentials.remove.failed",
message: `remove failed for ${parsed.data.connectorId}`,
context: { connectorId: parsed.data.connectorId },
});
deps.logger.error(`[${err.code}] ${err.message}: ${describeError(error)}`);
return deps.fail("REMOVE_FAILED", "Could not remove credential. Check application logs for details.");
}
});
@@ -130,15 +143,27 @@ export function registerCredentialIpcHandlers(deps: CredentialHandlerDeps): () =
try {
definition = getConnectorDefinition(parsed.data.connectorId);
} catch (error) {
deps.logger.error(`[credentials] test failed lookup for ${parsed.data.connectorId}: ${describeError(error)}`);
const err = classifyUnknownError(error, {
kind: "user_input",
code: "credentials.test.unknown_connector",
message: `test failed lookup for ${parsed.data.connectorId}`,
context: { connectorId: parsed.data.connectorId },
});
deps.logger.error(`[${err.code}] ${err.message}: ${describeError(error)}`);
return deps.fail("UNKNOWN_CONNECTOR", "Unknown connector.");
}
let resolved;
try {
resolved = await deps.resolver.resolve(parsed.data.connectorId);
} catch (error) {
const err = classifyUnknownError(error, {
kind: "upstream",
code: "credentials.test.resolve_failed",
message: `test resolve failed for ${parsed.data.connectorId}`,
context: { connectorId: parsed.data.connectorId },
});
const message = describeError(error);
deps.logger.error(`[credentials] test resolve failed for ${parsed.data.connectorId}: ${message}`);
deps.logger.error(`[${err.code}] ${err.message}: ${message}`);
return ok({ ok: false, message: redactSecrets(message) });
}
+15 -2
View File
@@ -4,6 +4,7 @@ import { ZodError } from "zod";
import type { DesktopEvent } from "@shared/types";
import type { DesktopResult } from "@shared/contracts";
import type { NotificationOptions } from "./notification-service";
import { classifyUnknownError } from "./errors/index.js";
interface IpcSupportDependencies {
isDev: boolean;
@@ -73,9 +74,15 @@ export function createIpcSupport({
try {
return await handler(event, ...args);
} catch (error) {
const classified = classifyUnknownError(error, {
kind: error instanceof ZodError ? "user_input" : "upstream",
code: error instanceof ZodError ? "ipc.payload.invalid" : "ipc.handler.failed",
message: `IPC handler ${channel} threw`,
context: { channel },
});
const rawMessage = error instanceof Error ? error.message : String(error);
const sanitized = sanitizeHandlerError(error);
logger.error(`[ipc:${channel}] ${rawMessage}`);
logger.error(`[${classified.code}] [ipc:${channel}] ${rawMessage}`);
return fail(sanitized.code, sanitized.message);
}
});
@@ -116,9 +123,15 @@ export function createIpcSupport({
return;
}
const key = event.type ?? "<untyped>";
const classified = classifyUnknownError(error, {
kind: "recoverable",
code: "ipc.emit_event.send_failed",
message: `emitEvent(${key}) failed`,
context: { eventType: key },
});
if (!emitWarnedChannels.has(key)) {
emitWarnedChannels.add(key);
logger.error(`[ipc] emitEvent(${key}) failed: ${message}`);
logger.error(`[${classified.code}] ${classified.message}: ${message}`);
}
}
}
+7 -1
View File
@@ -1,6 +1,7 @@
import { Notification, app } from "electron";
import type { BrowserWindow } from "electron";
import type { DesktopEvent, SectionId } from "@shared/types";
import { classifyUnknownError } from "./errors/index.js";
export interface NotificationOptions {
title: string;
@@ -205,8 +206,13 @@ export function createNotificationService(deps: NotificationServiceDependencies)
try {
notification.close();
} catch (error) {
const classified = classifyUnknownError(error, {
kind: "recoverable",
code: "notification.close.failed",
message: "notification.close() during destroy failed",
});
deps.logger.warn(
`[notification] notification.close() during destroy failed: ${error instanceof Error ? error.message : String(error)}`,
`[${classified.code}] ${classified.message}: ${error instanceof Error ? error.message : String(error)}`,
);
}
}
+16 -5
View File
@@ -3,6 +3,7 @@ import { join } from "node:path";
import { loadSkills } from "@mariozechner/pi-coding-agent";
import type { AppDatabase } from "../database";
import type { SkillImportResult, ProjectedSkill } from "../skill-types.js";
import { classifyUnknownError } from "../errors/index.js";
interface RuntimeDeps {
db: AppDatabase;
@@ -131,14 +132,24 @@ export function createSkillRuntime({ db, logger, skillsRoot, skillFilesDir, impo
try {
return statSyncLocal(join(builtinDir, name)).isDirectory();
} catch (error) {
logger.warn(
`[skills] statSync failed for builtin entry ${name}: ${error instanceof Error ? error.message : String(error)}`,
);
const classified = classifyUnknownError(error, {
kind: "recoverable",
code: "skills.builtin.stat_failed",
message: `statSync failed for builtin entry ${name}`,
context: { builtinDir, entry: name },
});
logger.warn(`[${classified.code}] ${classified.message}: ${error instanceof Error ? error.message : String(error)}`);
return false;
}
});
} catch {
logger.warn(`[skills] Failed to read builtin skills directory: ${builtinDir}`);
} catch (err) {
const classified = classifyUnknownError(err, {
kind: "recoverable",
code: "skills.builtin.readdir_failed",
message: `Failed to read builtin skills directory`,
context: { builtinDir },
});
logger.warn(`[${classified.code}] ${classified.message}: ${builtinDir}`);
return [];
}
+7 -1
View File
@@ -2,6 +2,7 @@ import { app } from "electron";
import electronUpdater from "electron-updater";
import type { AppDatabase } from "./database";
import type { NotificationOptions } from "./notification-service";
import { classifyUnknownError } from "./errors/index.js";
const { autoUpdater } = electronUpdater;
@@ -140,7 +141,12 @@ export function createUpdaterService({
const result = await autoUpdater.checkForUpdates();
return Boolean(result?.updateInfo);
} catch (error) {
logger.error(error);
const classified = classifyUnknownError(error, {
kind: "upstream",
code: "updater.check_for_updates.failed",
message: "autoUpdater.checkForUpdates() failed",
});
logger.error(`[${classified.code}] ${classified.message}`, error);
manualCheckPending = false;
return false;
}
+9 -2
View File
@@ -1,6 +1,7 @@
import { filterRunLogChunk } from "../observability/run-log-filter";
import { persistRunEvent } from "../observability/run-event-recorder";
import type { HandlerContext, HandlerDeps, WorkerMessageEvent } from "./handler-context";
import { classifyUnknownError } from "../../errors/index.js";
const LEASE_RENEWAL_INTERVAL_MS = 5 * 60 * 1000; // 5 minutes
@@ -40,8 +41,14 @@ export async function handleRunLog(
db.renewTaskLease(run.taskId, run.id);
}
leaseRenewalMap.set(event.runId, now);
} catch {
// Run not found — skip lease renewal
} catch (err) {
const classified = classifyUnknownError(err, {
kind: "recoverable",
code: "worker.run_log.lease_renewal_lookup_failed",
message: `Run ${event.runId} not found during lease renewal`,
context: { runId: event.runId },
});
logger.warn(`[${classified.code}] ${classified.message}`);
}
}
}
@@ -1,5 +1,6 @@
import { persistRunEvent } from "../observability/run-event-recorder";
import type { HandlerContext, HandlerDeps, WorkerMessageEvent } from "./handler-context";
import { classifyUnknownError } from "../../errors/index.js";
export function handleRunStatus(
event: Extract<WorkerMessageEvent, { type: "run-status" }>,
@@ -34,8 +35,14 @@ export function handleRunStatus(
db.renewTaskLease(run.taskId, run.id);
}
onRunStarted?.(run.id, run.taskId, run.agentId, run.companyId);
} catch {
// Run not found — skip status handling
} catch (err) {
const classified = classifyUnknownError(err, {
kind: "recoverable",
code: "worker.run_status.run_not_found",
message: `Run ${event.runId} not found during status handling`,
context: { runId: event.runId, status: event.status },
});
logger.warn(`[${classified.code}] ${classified.message}`);
}
}
emitEvent(event);