diff --git a/docs/adapters/error-tracking.md b/docs/adapters/error-tracking.md new file mode 100644 index 00000000..829d0698 --- /dev/null +++ b/docs/adapters/error-tracking.md @@ -0,0 +1,81 @@ +# Error-Tracking Adapter + +## Interface + +Source of truth: [`packages/core/src/contracts/adapters/error-tracking.ts`](../../packages/core/src/contracts/adapters/error-tracking.ts) - `ErrorTrackingAdapter`, `ErrorContext`, and the `ERROR_TRACKING` token. + +`captureException` is fire-and-forget (returns `void`) - a reporter failure never breaks the +path that reported the error. Core names **no vendor**: the port is the only seam. Sentry, +PostHog, Rollbar, etc. are consumer overlays. + +## How reporting flows (no per-call-site capture) + +Core does not call `captureException` at each failure site. Instead the logger is the bridge: +every `logger.error({ err, ... }, 'message')` is mirrored to the bound tracker. So one log call +both logs and reports, and **all** error logs are covered - the oRPC unhandled-error +interceptor, EventBus subscriber failures, OutboxRelay drops, and anything else that logs an +error with an `err` field. Error logs without an `err` (validation notices) are not reported. + +Every unhandled error is caught at one of three global seams - never per-call-site in a module: + +- **oRPC `onError` interceptor** - every route/service error; `handler.handle` maps it to a + response, and non-`ORPCError`s are logged (expected `ORPCError`s are not reported). +- **Hono `app.onError`** - anything thrown in the middleware chain before oRPC runs (session + resolution, raw-body capture, etag/cache); returns a 500 and reports non-`HTTPException` errors. +- **EventBus / OutboxRelay** - async subscriber and outbox-drain failures. + +A module never writes tracker-specific code: it throws a typed error (caught by the interceptor) +or logs with `logger.error({ err })`. To opt an expected/noisy error out of forwarding - a caught +third-party timeout you log-and-retry - pass `report: false`: `logger.error({ err, report: false }, +'psp timeout, retrying')` is still logged, just not sent to the tracker (so it doesn't burn quota). + +Wiring, in `createApp`, after plugins load: + +```ts +if (container.has(ERROR_TRACKING)) { + const tracker = container.get(ERROR_TRACKING); + setErrorReporter((error, context) => tracker.captureException(error, context)); +} +``` + +No overlay binds `ERROR_TRACKING` -> no reporter -> error logs stay logs-only. The engine imports +no vendor SDK and has no `SENTRY_DSN`/vendor env of its own. + +The forwarded `ErrorContext` carries `userId`/`traceId` from the active request context plus the +log's structured fields (event topic, path, message) as `extra`. + +## Bind a vendor (consumer overlay) + +Real vendor adapters live in the consumer, bound via a self-disabling overlay plugin - the same +pattern as the payment/KYC/realtime vendor overlays. Ship an `ErrorTrackingAdapter` impl and +provide it; loading the overlay last makes its binding win. + +```ts +// extensions/sentry/plugin.ts (consumer) +import { ERROR_TRACKING } from '@openora/core/contracts'; +import { definePlugin } from '@openora/core/server'; +import * as Sentry from '@sentry/node'; + +export default definePlugin({ + id: 'sentry', + register(ctx) { + const dsn = process.env['SENTRY_DSN']; + if (!dsn) { + return; // self-disabling: no DSN -> platform stays logs-only + } + Sentry.init({ dsn }); + ctx.provide(ERROR_TRACKING, () => ({ + captureException(error, context) { + Sentry.captureException(error, { + ...(context?.userId ? { user: { id: context.userId } } : {}), + tags: { ...context?.tags, ...(context?.traceId ? { trace_id: context.traceId } : {}) }, + ...(context?.extra ? { extra: context.extra } : {}), + }); + }, + })); + }, +}); +``` + +Register it last in `extensions.config.ts` (a `kind: 'infra'` overlay). A PostHog/Rollbar shop +writes the equivalent overlay against the same port - core is unchanged. diff --git a/docs/adapters/kyc.md b/docs/adapters/kyc.md index 73f3c0ff..d7961218 100644 --- a/docs/adapters/kyc.md +++ b/docs/adapters/kyc.md @@ -2,58 +2,13 @@ ## Interface -```ts -// packages/core/src/contracts/adapters/kyc.ts -export type KycAdapter = { - // True when the adapter rubber-stamps every submission (the default MockKycAdapter). - // A boot guard refuses/warns if withdrawals are KYC-gated while this is bound, so an - // operator can't enable the gate yet leave verification a no-op. Real providers omit it. - readonly autoApproves?: boolean; - submit(userId: string, documents: KycDocument[]): Promise; - getStatus(userId: string): Promise; - // Provider-specific normalization of a raw webhook into a vendor decision, or null when - // the body is not a reconcilable decision. Optional - MockKycAdapter omits it. - parseWebhook?( - rawBody: string, - headers: Record, - ): KycResult | null; -}; - -export type KycDocument = { - type: 'passport' | 'drivers_license' | 'national_id'; - frontUrl: string; - backUrl?: string; -}; - -export type KycVendorStatus = 'pending' | 'approved' | 'rejected' | 'not_started'; - -export type KycResult = { - referenceId: string; - status: KycVendorStatus; - // Hosted verification URL to redirect the end user to, for vendors whose flow collects - // documents on their own hosted page rather than accepting them from our backend. - // Omitted by document-forwarding vendors (eg SumSub, MockKycAdapter). - verificationUrl?: string; -}; - -export const KYC_ADAPTER: Token = createToken('KYC_ADAPTER'); -``` +Source of truth: [`packages/core/src/contracts/adapters/kyc.ts`](../../packages/core/src/contracts/adapters/kyc.ts) - `KycAdapter`, `KycDocument`, `KycVendorStatus`, `KycResult`, and the `KYC_ADAPTER` token. -`KYC_ADAPTER` is a `Token` created via `createToken` (`packages/core/src/contracts/adapters/token.ts`), -not a bare `Symbol` - the `Container` uses it to type both `provide` and `get` calls. +Two fields drive behavior beyond their types: `KycAdapter.autoApproves` marks a rubber-stamp adapter (the default `MockKycAdapter`) - a boot guard refuses/warns if withdrawals are KYC-gated while it's bound, so the gate can't be on with verification a no-op; real providers omit it. `KycResult.verificationUrl` is set only by hosted-session vendors whose own page collects documents (see recipe 2), and omitted by document-forwarding vendors. ## Webhook verifier -A second port authenticates the vendor's async webhook before `KycAdapter.parseWebhook` -ever sees the body: - -```ts -export type KycWebhookVerifier = { - verify(rawBody: string, headers: Record): boolean; -}; - -export const KYC_WEBHOOK_VERIFIER: Token = createToken('KYC_WEBHOOK_VERIFIER'); -``` +A second port, `KycWebhookVerifier` + the `KYC_WEBHOOK_VERIFIER` token (same source file), authenticates the vendor's async webhook before `KycAdapter.parseWebhook` ever sees the body. `verify` takes the full headers map, not a single pre-extracted value - the signing header name is vendor-specific (SumSub, a hosted-session vendor, and any future provider each name diff --git a/docs/adapters/notification.md b/docs/adapters/notification.md index cf8e7312..53ed11a9 100644 --- a/docs/adapters/notification.md +++ b/docs/adapters/notification.md @@ -2,14 +2,7 @@ ## Interface -```ts -// packages/core/src/contracts/adapters/src/notification.ts -export interface NotificationDeliveryAdapter { - sendEmail(to: string, subject: string, body: string): Promise; -} - -export const NOTIFICATION_DELIVERY_ADAPTER = Symbol('NOTIFICATION_DELIVERY_ADAPTER'); -``` +Source of truth: [`packages/core/src/contracts/adapters/notification.ts`](../../packages/core/src/contracts/adapters/notification.ts) - `NotificationDeliveryAdapter` and the `NOTIFICATION_DELIVERY_ADAPTER` token. ## Default binding diff --git a/docs/adapters/payment.md b/docs/adapters/payment.md index 9eb80980..ee2e2bdb 100644 --- a/docs/adapters/payment.md +++ b/docs/adapters/payment.md @@ -2,60 +2,9 @@ ## Interface -```ts -// packages/core/src/contracts/adapters/payment.ts -import { createToken, type Token } from './token.js'; - -export type PaymentAdapter = { - processDeposit( - amount: string, - currency: string, - metadata: Record, - ): Promise<{ externalId: string; status: string }>; - - processWithdrawal( - amount: string, - currency: string, - metadata: Record, - ): Promise<{ externalId: string; status: string }>; - - // Address-based deposit vendors only (eg a custody rail). Omit entirely for a - // synchronous PSP. - issueDepositAddress?(userId: string, currency: string): Promise<{ address: string }>; - - // Async vendors only. Normalizes a raw webhook body into a reconcilable event. - parseWebhook?( - rawBody: string, - headers: Record, - ): PaymentWebhookEvent | null; -}; - -export type PaymentWebhookEvent = - | { - kind: 'deposit'; - address: string; - amount: string; - currency: string; - txHash: string; - externalId: string; - } - | { - kind: 'withdrawal'; - externalId: string; - status: 'processing' | 'completed' | 'failed'; - txHash?: string; - }; - -export const PAYMENT_ADAPTER: Token = createToken('PAYMENT_ADAPTER'); - -export type PaymentWebhookVerifier = { - verify(rawBody: string, headers: Record): boolean; -}; - -export const PAYMENT_WEBHOOK_VERIFIER: Token = createToken( - 'PAYMENT_WEBHOOK_VERIFIER', -); -``` +Source of truth: [`packages/core/src/contracts/adapters/payment.ts`](../../packages/core/src/contracts/adapters/payment.ts) - `PaymentAdapter`, `PaymentWebhookEvent`, `PaymentWebhookVerifier`, and the `PAYMENT_ADAPTER` / `PAYMENT_WEBHOOK_VERIFIER` tokens. + +`issueDepositAddress` and `parseWebhook` are optional - present only on address-based/async vendors, omitted by a synchronous PSP (see the two vendor shapes below). ## Default binding diff --git a/docs/catalog.json b/docs/catalog.json index f7c5faef..d8343dc2 100644 --- a/docs/catalog.json +++ b/docs/catalog.json @@ -373,6 +373,15 @@ "packages/core/src/pam/identity/plugin.ts" ] }, + { + "category": "error-tracking", + "interface": "ErrorTrackingAdapter", + "token": "ERROR_TRACKING", + "status": "wired", + "boundIn": [ + "packages/core/src/server/runtime/create-app.ts" + ] + }, { "category": "game", "interface": "GameAdapter", diff --git a/packages/core/src/contracts/adapters/error-tracking.ts b/packages/core/src/contracts/adapters/error-tracking.ts new file mode 100644 index 00000000..3c601fc6 --- /dev/null +++ b/packages/core/src/contracts/adapters/error-tracking.ts @@ -0,0 +1,15 @@ +import { createToken, type Token } from './token.js'; + +export type ErrorContext = { + userId?: string; + traceId?: string; + tags?: Record; + extra?: Record; +}; + +export type ErrorTrackingAdapter = { + captureException(error: unknown, context?: ErrorContext): void; +}; + +export const ERROR_TRACKING: Token = + createToken('ERROR_TRACKING'); diff --git a/packages/core/src/contracts/adapters/index.ts b/packages/core/src/contracts/adapters/index.ts index f4cc7296..6de964f7 100644 --- a/packages/core/src/contracts/adapters/index.ts +++ b/packages/core/src/contracts/adapters/index.ts @@ -108,6 +108,9 @@ export type { } from './email-template.js'; export { EMAIL_TEMPLATE_RENDERER, DEFAULT_EMAIL_TEMPLATES } from './email-template.js'; +export type { ErrorTrackingAdapter, ErrorContext } from './error-tracking.js'; +export { ERROR_TRACKING } from './error-tracking.js'; + export type { AdminGrant, AdminPermissionResolver } from './admin-permission.js'; export { ADMIN_PERMISSION_RESOLVER } from './admin-permission.js'; diff --git a/packages/core/src/server/kernel/__tests__/logger.test.ts b/packages/core/src/server/kernel/__tests__/logger.test.ts new file mode 100644 index 00000000..0941a9d4 --- /dev/null +++ b/packages/core/src/server/kernel/__tests__/logger.test.ts @@ -0,0 +1,64 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { createLogger } from '../logger.js'; +import { setErrorReporter } from '../error-reporter.js'; + +afterEach(() => { + setErrorReporter(undefined); +}); + +describe('createLogger error-reporter hook', () => { + it('forwards an error-level log carrying `err` to the bound reporter', () => { + const reporter = vi.fn(); + setErrorReporter(reporter); + + const err = new Error('boom'); + createLogger('test').error({ err, event: 'x' }, 'something failed'); + + expect(reporter).toHaveBeenCalledTimes(1); + expect(reporter).toHaveBeenCalledWith( + err, + expect.objectContaining({ extra: { event: 'x', message: 'something failed' } }), + ); + }); + + it('does not report error logs without an `err`', () => { + const reporter = vi.fn(); + setErrorReporter(reporter); + + createLogger('test').error({ issues: [] }, 'validation failed'); + + expect(reporter).not.toHaveBeenCalled(); + }); + + it('does not report an error log opted out with `report: false`', () => { + const reporter = vi.fn(); + setErrorReporter(reporter); + + createLogger('test').error({ err: new Error('boom'), report: false }, 'expected, retrying'); + + expect(reporter).not.toHaveBeenCalled(); + }); + + it('does not report non-error levels', () => { + const reporter = vi.fn(); + setErrorReporter(reporter); + + createLogger('test').warn({ err: new Error('x') }, 'a warning'); + + expect(reporter).not.toHaveBeenCalled(); + }); + + it('swallows reporter failures', () => { + setErrorReporter(() => { + throw new Error('reporter failed'); + }); + + expect(() => + createLogger('test').error({ err: new Error('x') }, 'reporter failure'), + ).not.toThrow(); + }); + + it('is inert when no reporter is bound', () => { + expect(() => createLogger('test').error({ err: new Error('x') }, 'no reporter')).not.toThrow(); + }); +}); diff --git a/packages/core/src/server/kernel/error-reporter.ts b/packages/core/src/server/kernel/error-reporter.ts new file mode 100644 index 00000000..037fb53f --- /dev/null +++ b/packages/core/src/server/kernel/error-reporter.ts @@ -0,0 +1,26 @@ +import type { ErrorContext } from '@openora/core/contracts'; + +// The process-wide bridge from the logger to a bound ERROR_TRACKING adapter; +// createApp sets it once after plugins load, when an overlay bound the port. +export type ErrorReporter = (error: unknown, context?: ErrorContext) => void; + +// ponytail: process-global single reporter. Correct under horizontal scaling - each +// replica is its own process with its own reporter, reporting independently (the vendor +// aggregates across instances). The only limit is multiple createApp instances in ONE +// process (multi-tenant hosting, or a test harness booting several apps): they'd share +// this reporter and one app's dispose would clear it for the others. Move it onto the +// container then. See ADR-0026. +let reporter: ErrorReporter | undefined; + +export function setErrorReporter(next: ErrorReporter | undefined): void { + reporter = next; +} + +export function reportError(error: unknown, context?: ErrorContext): void { + // Fire-and-forget: a reporter failure must never break the log call that drove it. + try { + reporter?.(error, context); + } catch { + /* swallow - reporting is best-effort */ + } +} diff --git a/packages/core/src/server/kernel/index.ts b/packages/core/src/server/kernel/index.ts index 01d9363d..00000548 100644 --- a/packages/core/src/server/kernel/index.ts +++ b/packages/core/src/server/kernel/index.ts @@ -24,6 +24,9 @@ export type { Factory } from './container.js'; export { createLogger } from './logger.js'; +export { setErrorReporter, reportError } from './error-reporter.js'; +export type { ErrorReporter } from './error-reporter.js'; + export { getUserId } from './router-utils.js'; export type { OssContext, AuthContext, NodeHeaders } from './router-utils.js'; export { diff --git a/packages/core/src/server/kernel/logger.ts b/packages/core/src/server/kernel/logger.ts index 566882fe..707e8d69 100644 --- a/packages/core/src/server/kernel/logger.ts +++ b/packages/core/src/server/kernel/logger.ts @@ -1,5 +1,42 @@ import { pino, type Logger } from 'pino'; +import { reportError } from './error-reporter.js'; +import { getCurrentRequestContext } from './request-context.js'; +const ERROR_LEVEL = 50; + +// One logger.error({ err }, msg) both logs and reports: this hook forwards every +// error-level log carrying an `err` to the bound error reporter (an ERROR_TRACKING +// overlay - Sentry/PostHog/...), so no call site makes a separate capture call and +// child loggers are covered too. `report: false` opts a noisy/expected error out of +// forwarding (still logged); a log without `err` is never reported. export function createLogger(name: string): Logger { - return pino({ name }); + return pino({ + name, + hooks: { + logMethod(inputArgs, method, level) { + method.apply(this, inputArgs); + if (level < ERROR_LEVEL) { + return; + } + const [first, second] = inputArgs; + if (!first || typeof first !== 'object' || !('err' in first)) { + return; + } + const { err, report, ...rest } = first as { + err?: unknown; + report?: boolean; + } & Record; + if (err === null || err === undefined || report === false) { + return; + } + const ctx = getCurrentRequestContext(); + const message = typeof second === 'string' ? second : undefined; + reportError(err, { + ...(ctx?.userId ? { userId: ctx.userId } : {}), + ...(ctx?.traceId ? { traceId: ctx.traceId } : {}), + extra: message ? { ...rest, message } : rest, + }); + }, + }, + }); } diff --git a/packages/core/src/server/runtime/create-app.ts b/packages/core/src/server/runtime/create-app.ts index e5b6d6e2..d2ad55c3 100644 --- a/packages/core/src/server/runtime/create-app.ts +++ b/packages/core/src/server/runtime/create-app.ts @@ -5,6 +5,7 @@ import type { ContractRouter } from '@orpc/contract'; import { Hono } from 'hono'; import { cors } from 'hono/cors'; import { etag } from 'hono/etag'; +import { HTTPException } from 'hono/http-exception'; import { serve, type ServerType } from '@hono/node-server'; import { resolve } from 'node:path'; import { generateOpenApiSpec } from './openapi.js'; @@ -23,6 +24,7 @@ import { createLogger, createRedisClient, withRequestContext, + setErrorReporter, EVENT_BUS, type OssContext, } from '../kernel/index.js'; @@ -36,6 +38,7 @@ import { ADMIN_PERMISSION_RESOLVER, RATE_LIMITER, CACHE, + ERROR_TRACKING, composeContract, healthContract, IGAMING_CONFIG, @@ -276,6 +279,12 @@ export async function createApp(config: CreateAppConfig): Promise { const registry = await loadPlugins(config.plugins, container); await config.configure?.(container); + if (container.has(ERROR_TRACKING)) { + const tracker = container.get(ERROR_TRACKING); + setErrorReporter((error, context) => tracker.captureException(error, context)); + container.onDispose(() => setErrorReporter(undefined)); + } + const bus = container.get(EVENT_BUS); for (const [event, handlers] of registry.events.getAll()) { for (const handler of handlers) { @@ -327,6 +336,22 @@ export async function createApp(config: CreateAppConfig): Promise { const app = new Hono(); + // Outermost catch for errors thrown in the Hono middleware chain (session + // resolution, raw-body capture, etag/cache) - the one seam oRPC's onError + // interceptor never sees, since handler.handle turns route/service errors into + // responses. logger.error reports via the reporter; HTTPExceptions are deliberate + // outcomes, returned without reporting. + app.onError((err, c) => { + if (err instanceof HTTPException) { + return err.getResponse(); + } + createLogger('http').error( + { err, method: c.req.method, path: c.req.path }, + 'unhandled request error', + ); + return c.json({ error: 'Internal Server Error' }, 500); + }); + if (config.cors !== false) { const origins = config.cors === true || config.cors === undefined ? undefined : config.cors.origins;