From 18a1bcb42fb96f1b7aff0a8c004646c3b6240a6f Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Thu, 30 Apr 2026 06:09:57 -0400 Subject: [PATCH] docs(logger): document level convention re error->Slack/PostHog hook Add JSDoc context next to the level guide in server/src/logger.ts explaining that logger.error / logger.fatal trigger the #aao-errors Slack alert and PostHog $exception capture via the pino hook, so they should be used only for unexpected, page-worthy failures. Expected failures with a graceful fallback (third-party 4xx, validation, etc.) should be logger.warn so the alert path is not taken. Same anti-pattern surfaced as ":rotating_light: System error: unknown" noise in PRs #3578, #3622, #3648. Co-Authored-By: Claude Opus 4.7 (1M context) --- .changeset/docs-logger-level-convention.md | 6 ++++++ server/src/logger.ts | 17 +++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 .changeset/docs-logger-level-convention.md diff --git a/.changeset/docs-logger-level-convention.md b/.changeset/docs-logger-level-convention.md new file mode 100644 index 0000000000..0641dbd25c --- /dev/null +++ b/.changeset/docs-logger-level-convention.md @@ -0,0 +1,6 @@ +--- +--- + +Document the level convention in `server/src/logger.ts` JSDoc: `error`/`fatal` (level >= 50) trigger Slack `#aao-errors` alerts and PostHog `$exception` capture via the pino hook, so they should be reserved for unexpected, page-worthy failures. For *expected* failures with a graceful user-facing fallback (third-party 4xx, validation errors, etc.), use `warn` so the alert path is not taken. + +This is a docs-only change to give future authors and reviewers a clear reference for the `logger.error` vs `logger.warn` decision. The same anti-pattern triggered the original `:rotating_light: System error: unknown` Slack noise that PRs #3578 / #3622 / #3648 fixed. diff --git a/server/src/logger.ts b/server/src/logger.ts index 96f16d6303..bf6503f64c 100644 --- a/server/src/logger.ts +++ b/server/src/logger.ts @@ -193,7 +193,20 @@ export function createLogger(context: string | Record) { * - trace: Very detailed, low-level information (usually disabled) * - debug: Debugging information (enabled in development) * - info: Normal operation messages (default in production) - * - warn: Warning messages (potential issues) - * - error: Error messages (operation failed but app continues) + * - warn: Warning messages (potential issues, expected failures with a graceful fallback) + * - error: Unexpected failures — operation failed but app continues * - fatal: Critical errors (app cannot continue) + * + * IMPORTANT: `logger.error()` and `logger.fatal()` (level >= 50) trigger the + * Slack `#aao-errors` alert and a PostHog `$exception` capture via the pino + * hook in this file (and `notifyErrorChannel` in `utils/posthog.ts`). Use + * `error` for failures that should page an operator. For *expected* failures + * — third-party 4xx, validation errors, anything where the caller already + * returns a friendly user-facing message — use `warn` so the alert path is + * not taken. + * + * Tool handlers in `addie/mcp/` are the most common source of confusion: + * if your handler catches a failure and returns a "Failed to do X" string + * to the user, that should be `logger.warn`, not `logger.error`. Reserve + * `error` for catch blocks of network/parse failures that "shouldn't happen". */