Skip to content

chore: plans for error handling middleware and error observability - #248

Merged
kojiwakayama merged 9 commits into
mainfrom
chore/error-handling-observability-plans
Feb 9, 2026
Merged

kojiwakayama merged 9 commits into
mainfrom
chore/error-handling-observability-plans

Conversation

@kojiwakayama

@kojiwakayama kojiwakayama commented Feb 6, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up plans to #246 (error codes refactoring) and #247 (centralize scattered errors).

Execution Order

#246 Slug registry + RFC 9457     ← foundation
  ├── #247 Centralize scattered errors
  ├── This PR: Plan A (error handling middleware)  ← depends on #246
  └── This PR: Plan B (error observability)        ← depends on #246 + Plan A

Plan A: Error Handling Middleware

Unified catch → serialize → respond pipeline at all boundaries.

Full plan: plans/error_handling_middleware.md

Tasks

Phase 1: HTTP error boundary

  • 1.1 Create http-error-boundary.ts — catch all, serialize RFC 9457
  • 1.2 Tests for boundary middleware
  • 1.3 Wire into runtime-handler/index.ts (env-aware errorToRFC9457Response)
  • 1.4 Wire into routing/api/route-executor.ts

Phase 2: CLI error boundary

  • 2.1 Create cli-error-boundary.ts — structured terminal output
  • 2.2 Tests
  • 2.3 Wire into CLI router (adds error metrics + OTel tracing)

Phase 3: Error wrapping

  • 3.1 wrapUnknownError() — wrap plain Error as unknown-error slug
  • 3.2 Migrate wrapError() callers to wrapWithContext(), delete wrapper

Phase 4: Delete legacy

  • 4.1 Delete routing/api/error-handler.ts (zero imports after dedup)
  • 4.2 Delete handleError(), logAndThrow(), wrapError() (zero callers)
  • 4.3 Clean up barrel exports

Phase 5: Verify

  • 5.1 Ad-hoc { error } responses: only dev-only dashboard API and input-validation remain (breaking change to convert)
  • 5.2 CLI errors show slug + suggestion via cliErrorBoundary
  • 5.3 All tests pass (1083 passed)

Plan B: Error Observability

Structured logging, metrics, tracing, and alerting with slug-based errors.

Full plan: plans/error_observability.md

Tasks

Phase 1: Structured error logging

  • 1.1 Create src/errors/logging.ts — JSON in production, human-readable in dev
  • 1.2 Tests
  • 1.3 handleError deleted; remaining handleErrorWithFallback is a different pattern (catch-and-continue)

Phase 2: Error metrics

  • 2.1 Create error-instruments.ts — veryfront.error.count{slug,category,status}
  • 2.2 Tests
  • 2.3 Wire into error boundary middleware

Phase 3: Error tracing

  • 3.1 Create src/errors/tracing.ts — attachErrorToSpan()
  • 3.2 Tests
  • 3.3 Wire into existing tracing infrastructure

Phase 4: Migrate ErrorCollector

  • 4.1 Replace ErrorType enum with registry categories
  • 4.2 Update tests
  • 4.3 All add*Error() callers use new category system

Phase 5: Verify

  • 5.1 Error boundary logs include slug + category
  • 5.2 Prometheus metrics endpoint includes veryfront_error_count
  • 5.3 All tests pass (1083 passed)

Fix Commits (review feedback)

  • b071194 — Deduplicate RFC 9457 response logic (handleAPIError → errorToRFC9457Response)
  • fb2c5dc — Fix logError context merge (preserve both error.context and caller context)
  • 95c445f — Use shared isProduction() in CLI error boundary (remove Node.js fallbacks)
  • 0d40b16 — Remove unused errorRate histogram and clean up re-exports
  • d532459 — Fix error handling hardening and remove collector compat paths
  • 86235af — Strip error details from production responses in runtime-handler (P1 fix)
  • ebbf627 — Delete deprecated handleError/logAndThrow, wire cliErrorBoundary into CLI router
  • 3332b00 — Replace wrapError with wrapWithContext, delete legacy wrapper

@ariskemper ariskemper self-assigned this Feb 6, 2026
@ariskemper
ariskemper marked this pull request as draft February 6, 2026 16:53
@ariskemper
ariskemper force-pushed the chore/error-handling-observability-plans branch from 7fd65bc to 5f86714 Compare February 9, 2026 10:37
Comment thread src/routing/api/route-executor.ts Fixed
- HTTP error boundary with RFC 9457 Problem Details responses
- CLI error boundary with slug-based formatting
- Error wrapping at boundaries (unknown errors → unknown-error slug)
- Structured error logging (JSON in production, human-readable in dev)
- Error metrics via OpenTelemetry counter with slug/category/status labels
- Error tracing with span attributes and events
- Migrate ErrorCollector to use slug registry categories
- Deprecate legacy handleError/logAndThrow in favor of boundaries
- Add plans for error handling middleware and error observability
@ariskemper
ariskemper force-pushed the chore/error-handling-observability-plans branch from 29f9d27 to 7403c08 Compare February 9, 2026 11:22
@ariskemper
ariskemper requested a review from kwakayama February 9, 2026 11:25
@ariskemper
ariskemper marked this pull request as ready for review February 9, 2026 11:25

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7403c08fe0

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/server/runtime-handler/index.ts Outdated
Comment thread src/server/handlers/request/snippet.handler.ts Outdated
Extract errorToRFC9457Response as a shared export from http-error-boundary.ts
and reuse it in route-executor.ts instead of duplicating the entire function.
Also remove the duplicate wrapAsUnknownError and import wrapUnknownError from
wrap-unknown.ts.
Previously, providing a context argument would silently drop error.context
due to using `||` instead of merging. Now both are merged with caller context
taking precedence.
Remove process.* fallbacks from isTTY/isDevelopment — this is a Deno project.
Use isProduction() from build/config/environment.ts for consistency with
logging.ts and the rest of the codebase.
Remove the errorRate histogram from error-instruments since it was created
but never wired up anywhere. Also remove the awkward isVeryfrontErrorMiddleware
alias from the errors barrel — isVeryfrontError is already exported from
http-error.ts and the middleware version is identical.
Comment thread src/errors/middleware/http-error-boundary.ts Fixed
Replace createErrorResponse with errorToRFC9457Response for env-aware
filtering in the runtime-handler catch blocks. In production, detail is
stripped from 5xx responses and stack traces are omitted, preventing
internal error messages from leaking to clients.
@kojiwakayama
kojiwakayama force-pushed the chore/error-handling-observability-plans branch from 13c3f3d to 86235af Compare February 9, 2026 12:24
…ndary

- Delete `handleError()` and `logAndThrow()` from error-handlers.ts (zero callers)
- Remove from barrel exports and tests
- Wire `cliErrorBoundary` into cli/router.ts (adds error metrics + OTel tracing)
- Keep `wrapError`, `handleErrorWithFallback`, `retryWithBackoff` (still used)
Callers now use wrapWithContext directly from middleware/wrap-unknown.
Removes the thin wrapError wrapper from error-handlers.ts, its tests,
and barrel export. The 3 MDX callers updated to direct imports.
@kojiwakayama
kojiwakayama merged commit 6f2a519 into main Feb 9, 2026
12 checks passed
@kojiwakayama
kojiwakayama deleted the chore/error-handling-observability-plans branch February 9, 2026 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants