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". */