diff --git a/plans/error_handling_middleware.md b/plans/error_handling_middleware.md deleted file mode 100644 index 3fbadfad15..0000000000 --- a/plans/error_handling_middleware.md +++ /dev/null @@ -1,171 +0,0 @@ -# Error Handling Middleware - -Unified error catch → serialize → respond pipeline at all system boundaries (HTTP, CLI, WebSocket). - -**Prerequisite:** [Error codes refactoring](./refactor_error_codes.md) (slug registry + `toRFC9457()` must exist). - ---- - -## Problem - -Error handling is ad-hoc across boundaries: - -- `src/server/universal-handler/index.ts` catches timeouts and returns `JSON.stringify({ error: "Request timeout" })` — no RFC 9457, no slug -- `src/routing/api/error-handler.ts` (`handleAPIError`) returns different shapes in dev vs production -- `src/errors/error-handlers.ts` provides `handleError()`, `wrapError()`, `logAndThrow()` — all log differently, none serialize to RFC 9457 -- `src/errors/user-friendly/error-wrapper.ts` wraps errors for dev overlay — separate from HTTP serialization -- CLI commands catch errors independently with inconsistent formatting -- Plain `Error` throws (289 files) bypass structured handling entirely - ---- - -## Target State - -- Single `errorBoundary()` middleware at each system boundary -- All HTTP responses for errors use `application/problem+json` -- All CLI error output uses structured format: `[slug] title\n detail\n suggestion` -- Plain `Error` throws get auto-wrapped to `unknown-error` slug at boundary -- Dev mode adds stack traces; production omits them -- Existing `handleError()`, `wrapError()`, `logAndThrow()` deprecated, then deleted - ---- - -## Execution Plan - -### Phase 1: HTTP error boundary middleware - -- [ ] **1.1** Create `src/errors/middleware/http-error-boundary.ts` - - Catch all errors from request handlers - - `VeryfrontError` → `toRFC9457()` with `application/problem+json` - - Plain `Error` → wrap as `unknown-error` slug, then serialize - - Dev mode: include `stack` field in response - - Production: omit `stack`, omit `detail` for 5xx errors - -- [ ] **1.2** Create `src/errors/middleware/http-error-boundary.test.ts` - - VeryfrontError → correct RFC 9457 shape - - Plain Error → wrapped as unknown-error - - Dev vs production output differences - - Content-Type header is `application/problem+json` - -> 1.3–1.4 depend on 1.1 - -- [ ] **1.3** Wire into `src/server/universal-handler/index.ts` - - Replace inline catch blocks with `httpErrorBoundary()` - - Replace timeout `JSON.stringify({ error: ... })` with RFC 9457 `timeout-error` slug - -- [ ] **1.4** Wire into `src/routing/api/route-executor.ts` and `error-handler.ts` - - Replace `handleAPIError()` with `httpErrorBoundary()` - - Delete `src/routing/api/error-handler.ts` after migration - -### Phase 2: CLI error boundary - -- [ ] **2.1** Create `src/errors/middleware/cli-error-boundary.ts` - - Format: `[slug] title\n detail\n suggestion` - - Dev mode: include stack trace - - Exit code: 1 for all errors - - Color output when TTY - -- [ ] **2.2** Create `src/errors/middleware/cli-error-boundary.test.ts` - -> 2.3 depends on 2.1 - -- [ ] **2.3** Wire into CLI command handlers - - Replace per-command try/catch with `cliErrorBoundary(handler)` - - Affected: `src/cli/commands/*/command.ts` - -### Phase 3: Error wrapping at boundaries - -- [ ] **3.1** Create `src/errors/middleware/wrap-unknown.ts` - - `wrapUnknownError(error: unknown): VeryfrontError` — wraps any non-VeryfrontError as `unknown-error` slug - - Preserves original error as `cause` - - Extracts message, stack from original - -- [ ] **3.2** Update `src/errors/error-handlers.ts` - - `wrapError()` → use `wrapUnknownError()` internally - - `handleError()` → use structured log format from observability plan - - `logAndThrow()` → deprecate (boundary middleware handles this) - -### Phase 4: Delete legacy error handling - -> 4.1–4.3 are independent. Run as parallel subagents. - -- [ ] **4.1** Delete `src/routing/api/error-handler.ts` (replaced by http-error-boundary) -- [ ] **4.2** Delete `handleError()`, `logAndThrow()` from `src/errors/error-handlers.ts` (replaced by boundary middleware) -- [ ] **4.3** Update `src/errors/index.ts` — remove deleted exports - -### Phase 5: Verify - -- [ ] **5.1** No HTTP responses return `{ error: "..." }` — all use RFC 9457 -- [ ] **5.2** All CLI errors show slug + suggestion -- [ ] **5.3** All tests pass - ---- - -## Code Patterns - -### HTTP error boundary - -```typescript -// src/errors/middleware/http-error-boundary.ts -export function httpErrorBoundary(handler: RequestHandler): RequestHandler { - return async (req, ctx) => { - try { - return await handler(req, ctx); - } catch (error) { - const vfError = error instanceof VeryfrontError - ? error - : wrapUnknownError(error); - - const body = vfError.toRFC9457(); - if (!ctx.isDev) { - delete body.stack; - if (vfError.status >= 500) delete body.detail; - } - - return new Response(JSON.stringify(body), { - status: vfError.status, - headers: { "Content-Type": "application/problem+json" }, - }); - } - }; -} -``` - -### CLI error boundary - -```typescript -// src/errors/middleware/cli-error-boundary.ts -export function cliErrorBoundary(handler: () => Promise): () => Promise { - return async () => { - try { - await handler(); - } catch (error) { - const vfError = error instanceof VeryfrontError - ? error - : wrapUnknownError(error); - - console.error(`[${vfError.slug}] ${vfError.title}`); - if (vfError.detail) console.error(` ${vfError.detail}`); - if (vfError.suggestion) console.error(` ${vfError.suggestion}`); - Deno.exit(1); - } - }; -} -``` - ---- - -## File Changes - -| File | Phase | Change | -|------|-------|--------| -| `src/errors/middleware/http-error-boundary.ts` | 1 | **New** | -| `src/errors/middleware/http-error-boundary.test.ts` | 1 | **New** | -| `src/errors/middleware/cli-error-boundary.ts` | 2 | **New** | -| `src/errors/middleware/cli-error-boundary.test.ts` | 2 | **New** | -| `src/errors/middleware/wrap-unknown.ts` | 3 | **New** | -| `src/server/universal-handler/index.ts` | 1 | Replace inline catches | -| `src/routing/api/route-executor.ts` | 1 | Use http-error-boundary | -| `src/routing/api/error-handler.ts` | 4 | **Delete** | -| `src/errors/error-handlers.ts` | 3–4 | Deprecate → delete legacy fns | -| `src/cli/commands/*/command.ts` | 2 | Wrap with cliErrorBoundary | diff --git a/plans/error_observability.md b/plans/error_observability.md deleted file mode 100644 index 533269335e..0000000000 --- a/plans/error_observability.md +++ /dev/null @@ -1,231 +0,0 @@ -# Error Observability - -Structured error logging, metrics, tracing, and alerting using the slug-based error registry. - -**Prerequisite:** [Error codes refactoring](./refactor_error_codes.md) (slug registry must exist). Pairs with [error handling middleware](./error_handling_middleware.md). - ---- - -## Problem - -Current error observability is fragmented: - -- `ErrorCollector` (`src/observability/error-collector.ts`) uses its own `ErrorType` enum (`compile`, `runtime`, `bundle`, `hmr`, `module`) — disconnected from slug registry categories -- `handleError()` logs `Error: ${message}` — no slug, no category, no structured fields -- Metrics record `recordRenderError()`, `recordRSCError()` as separate counters — no unified error metric -- Tracing exists (`withSpan`, OpenTelemetry) but errors don't attach slug/category as span attributes -- No way to alert on specific slug frequency spikes or new slugs appearing -- Log format is inconsistent: some use `serverLogger.error(message, error)`, others use `console.error` - ---- - -## Target State - -- All errors logged in structured format with `slug`, `category`, `status` fields -- Single `veryfront.error` metric counter with `slug` and `category` labels -- Error spans include `error.slug` and `error.category` attributes -- `ErrorCollector` uses slug registry categories instead of its own enum -- Grafana dashboard: error rate by category, top slugs, new slug detection -- Alert rules: spike detection per-slug, new slug appearance - ---- - -## Execution Plan - -### Phase 1: Structured error logging - -- [ ] **1.1** Create `src/errors/logging.ts` - - `logError(error: VeryfrontError, context?: Record): void` - - Output format: - ``` - [ERROR] {slug} ({category}) — {title} - Detail: {detail} - Suggestion: {suggestion} - Docs: https://veryfront.com/docs/errors/{slug} - ``` - - JSON mode for production (structured logging to stdout): - ```json - {"level":"error","slug":"config-not-found","category":"CONFIG","title":"...","detail":"...","timestamp":"..."} - ``` - -- [ ] **1.2** Create `src/errors/logging.test.ts` - -> 1.3 depends on 1.1 - -- [ ] **1.3** Replace all `handleError()` call sites with `logError()` - - `src/errors/error-handlers.ts` → update `handleError` to delegate to `logError` - - Grep for `serverLogger.error` in error-handling code paths → use `logError` - -### Phase 2: Error metrics - -- [ ] **2.1** Create `src/observability/instruments/error-instruments.ts` - - Counter: `veryfront.error.count` with labels `{slug, category, status}` - - Histogram: `veryfront.error.rate` for error rate tracking - - Function: `recordError(error: VeryfrontError): void` - -- [ ] **2.2** Create `src/observability/instruments/error-instruments.test.ts` - -> 2.3 depends on 2.1 - -- [ ] **2.3** Wire `recordError()` into error boundary middleware - - `httpErrorBoundary` → call `recordError()` before responding - - `cliErrorBoundary` → call `recordError()` before exiting - -### Phase 3: Error tracing integration - -- [ ] **3.1** Create `src/errors/tracing.ts` - - `attachErrorToSpan(error: VeryfrontError, span: Span): void` - - Sets span attributes: `error.slug`, `error.category`, `error.status` - - Sets span status to ERROR - - Adds span event with error detail - -- [ ] **3.2** Create `src/errors/tracing.test.ts` - -> 3.3 depends on 3.1 - -- [ ] **3.3** Wire into existing tracing infrastructure - - `src/observability/auto-instrument/wrappers.ts` → attach error attributes when errors are caught - - `src/server/universal-handler/index.ts` → tracing catch blocks use `attachErrorToSpan` - -### Phase 4: Migrate ErrorCollector - -- [ ] **4.1** Update `src/observability/error-collector.ts` - - Replace `ErrorType` enum with `ErrorCategory` from slug registry - - `DevError.type` → `DevError.category` (use registry categories) - - `DevError.id` → include slug when available - - Keep backward compat for MCP consumers during transition - -- [ ] **4.2** Update `src/observability/error-collector.test.ts` - -- [ ] **4.3** Update all `ErrorCollector.add*()` call sites - - `addCompileError()` → `add({ category: "BUILD", slug: "..." })` - - `addRuntimeError()` → `add({ category: "RUNTIME", slug: "..." })` - - `addBundleError()` → `add({ category: "BUILD", slug: "..." })` - - `addHMRError()` → `add({ category: "DEV", slug: "..." })` - - `addModuleError()` → `add({ category: "MODULE", slug: "..." })` - -### Phase 5: Grafana dashboards and alerts - -- [ ] **5.1** Create error rate dashboard in `veryfront-observability/` - - Panel: Error count by category (stacked bar) - - Panel: Top 10 slugs (table) - - Panel: Error rate over time (line graph) - - Panel: New slugs in last 24h (stat) - -- [ ] **5.2** Create alert rules - - Spike: `rate(veryfront_error_count[5m])` > 2x baseline for any slug - - New slug: slug appears that wasn't seen in last 7 days - - Category threshold: CONFIG/BUILD errors > 10/min (indicates systemic issue) - -### Phase 6: Verify - -- [ ] **6.1** All error log lines include slug and category -- [ ] **6.2** Prometheus `/metrics` endpoint includes `veryfront_error_count` -- [ ] **6.3** Grafana dashboard renders with sample data -- [ ] **6.4** All tests pass - ---- - -## Code Patterns - -### Structured error logging - -```typescript -// src/errors/logging.ts -export function logError(error: VeryfrontError, context?: Record): void { - const entry = { - level: "error", - slug: error.slug, - category: error.category, - title: error.title, - detail: error.detail, - suggestion: error.suggestion, - status: error.status, - docs: `https://veryfront.com/docs/errors/${error.slug}`, - ...context, - timestamp: new Date().toISOString(), - }; - - if (isProductionMode()) { - // Structured JSON for Loki ingestion - console.error(JSON.stringify(entry)); - } else { - // Human-readable for dev - console.error(`[ERROR] ${error.slug} (${error.category}) — ${error.title}`); - if (error.detail) console.error(` Detail: ${error.detail}`); - if (error.suggestion) console.error(` Suggestion: ${error.suggestion}`); - } -} -``` - -### Error metric recording - -```typescript -// src/observability/instruments/error-instruments.ts -import { getMetricsState } from "../metrics/index.ts"; - -const errorCounter = createCounter("veryfront.error.count", { - description: "Total errors by slug and category", -}); - -export function recordError(error: VeryfrontError): void { - errorCounter.add(1, { - slug: error.slug, - category: error.category, - status: String(error.status), - }); -} -``` - -### Error span attributes - -```typescript -// src/errors/tracing.ts -export function attachErrorToSpan(error: VeryfrontError, span: Span): void { - span.setStatus({ code: SpanStatusCode.ERROR, message: error.title }); - span.setAttributes({ - "error.slug": error.slug, - "error.category": error.category, - "error.status": error.status, - }); - span.addEvent("error", { - "error.slug": error.slug, - "error.detail": error.detail ?? "", - }); -} -``` - ---- - -## File Changes - -| File | Phase | Change | -|------|-------|--------| -| `src/errors/logging.ts` | 1 | **New** | -| `src/errors/logging.test.ts` | 1 | **New** | -| `src/observability/instruments/error-instruments.ts` | 2 | **New** | -| `src/observability/instruments/error-instruments.test.ts` | 2 | **New** | -| `src/errors/tracing.ts` | 3 | **New** | -| `src/errors/tracing.test.ts` | 3 | **New** | -| `src/observability/error-collector.ts` | 4 | Migrate to registry categories | -| `src/observability/error-collector.test.ts` | 4 | Update tests | -| `veryfront-observability/dashboards/` | 5 | **New** dashboard JSON | -| `veryfront-observability/alerts/` | 5 | **New** alert rules | - ---- - -## Loki Query Examples (post-migration) - -```logql -# All errors by category -{namespace="veryfront-production"} | json | level="error" | line_format "{{.slug}} ({{.category}})" - -# Config errors specifically -{namespace="veryfront-production"} | json | category="CONFIG" - -# Specific slug frequency -sum(rate({namespace="veryfront-production"} | json | slug="hydration-mismatch" [5m])) - -# New slugs not seen before -{namespace="veryfront-production"} | json | level="error" | slug != "" | slug !~ "config-not-found|build-failed|..." -``` diff --git a/src/routing/registry/registry.test.ts b/src/routing/registry/registry.test.ts index b5cac8a76a..de468d2268 100644 --- a/src/routing/registry/registry.test.ts +++ b/src/routing/registry/registry.test.ts @@ -2,6 +2,7 @@ import { assertEquals } from "#veryfront/testing/assert.ts"; import { describe, it } from "#veryfront/testing/bdd.ts"; import { RouteRegistry } from "./registry.ts"; import type { Handler, HandlerContext, HandlerResult } from "./types.ts"; +import { CONFIG_NOT_FOUND } from "#veryfront/errors/error-registry.ts"; function makeHandler( name: string, @@ -144,7 +145,7 @@ describe("routing/registry/RouteRegistry", () => { assertEquals(await result?.text(), "enabled"); }); - it("should continue on handler errors", async () => { + it("should return RFC 9457 error response when handler throws", async () => { const registry = new RouteRegistry(); const errorHandler: Handler = { metadata: { name: "erroring", priority: 100 }, @@ -159,8 +160,40 @@ describe("routing/registry/RouteRegistry", () => { ); const result = await registry.execute(makeReq(), makeCtx()); - assertEquals(result?.status, 200); - assertEquals(await result?.text(), "fallback"); + + // Should return error response, not continue to fallback handler + assertEquals(result?.status, 500); + assertEquals(result?.headers.get("Content-Type"), "application/problem+json"); + + const body = await result?.json() as { type?: string; title?: string; category?: string }; + assertEquals(body.type?.includes("unknown-error"), true); + assertEquals(body.category, "GENERAL"); + }); + + it("should return RFC 9457 response with correct slug for VeryfrontError", async () => { + const registry = new RouteRegistry(); + const errorHandler: Handler = { + metadata: { name: "config-error", priority: 100 }, + handle: () => Promise.reject(CONFIG_NOT_FOUND.create({ detail: "Test config error" })), + }; + + registry.register(errorHandler); + + const result = await registry.execute(makeReq(), makeCtx()); + + assertEquals(result?.status, 404); + assertEquals(result?.headers.get("Content-Type"), "application/problem+json"); + + const body = await result?.json() as { + type?: string; + detail?: string; + suggestion?: string; + category?: string; + }; + assertEquals(body.type?.includes("config-not-found"), true); + assertEquals(body.category, "CONFIG"); + assertEquals(body.detail, "Test config error"); + assertEquals(body.suggestion?.includes("vf init"), true); }); it("should return null on empty registry", async () => { diff --git a/src/routing/registry/registry.ts b/src/routing/registry/registry.ts index 6ce62185cf..553770c73c 100644 --- a/src/routing/registry/registry.ts +++ b/src/routing/registry/registry.ts @@ -1,6 +1,7 @@ import type { Handler, HandlerContext, RouteRegistryConfig } from "./types.ts"; import { serverLogger } from "#veryfront/utils"; import { withSpan } from "#veryfront/observability/tracing/otlp-setup.ts"; +import { errorToRFC9457Response } from "#veryfront/errors/middleware/http-error-boundary.ts"; export class RouteRegistry { private handlers: Handler[] = []; @@ -101,7 +102,8 @@ export class RouteRegistry { stack: error instanceof Error ? error.stack : undefined, }, ); - // Continue to next handler - a single handler failure shouldn't break the chain + // Convert handler error to RFC 9457 response and return immediately + return errorToRFC9457Response(error, ctx, req); } }