docs(agents): bring agent-facing docs in line with the R44 fix
Six docs lagged after the post-merge R44 fix added withCapture + reported-flag.ts. They mentioned withSpan only and described the capture story as if it were inline in use-case / controller bodies. This commit aligns them with what shipped. CLAUDE.md (Key Conventions): - "Spans applied at DI bind time" → "Spans + capture composed at DI bind time" with the withSpan(withCapture(factory)) sandwich and the outermost-span ordering note. - "Capture at throw sites only" expanded to mention the __sentryReported flag, the three flag-checking sites (withCapture, SentryLogger, RecordingLogger), and where the helper lives. AGENTS.md (Instrumentation conventions): - Use case + controller wrapping example shows the full sandwich. - Capture-rules table now explicitly says "via withCapture" for use cases and controllers, and "flag set, withCapture bails" for the bubbled cases. packages/core-shared/AGENTS.md: - "with-span.ts" entry split into a paired with-span + with-capture block, including the actual sandwich code. - New entry for reported-flag.ts explaining the helper and why RecordingLogger inlines the check (boundary rule). - Barrel re-export list updated. docs/architecture/vertical-feature-spec.md (§16): - The bind-production line now describes the withSpan(withCapture(...)) sandwich, the outermost-span rationale, and the bubbled-error bail. docs/architecture/dependency-flow.md (TRACER/LOGGER subsection): - bindAll diagram updated: real repo line annotates inline calls; use case + controller lines show withSpan(withCapture(...)). docs/guides/tdd-workflow.md (Asserting spans and captures): - Direct-injection example shows the binder sandwich. - Capture-assertion example explains the flag-bail behaviour and links to the new tests/r44-no-double-capture.test.ts e2e example. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
20
AGENTS.md
20
AGENTS.md
@@ -385,28 +385,38 @@ return this.tracer.startSpan(
|
|||||||
);
|
);
|
||||||
```
|
```
|
||||||
|
|
||||||
**Use case + controller spans (applied at DI bind time):**
|
**Use case + controller spans + capture (applied at DI bind time):**
|
||||||
|
|
||||||
```ts
|
```ts
|
||||||
const wrappedUC = withSpan(
|
const wrappedUC = withSpan(
|
||||||
tracer,
|
tracer,
|
||||||
{ name: "blog.getArticles", op: "use-case" },
|
{ name: "blog.getArticles", op: "use-case" },
|
||||||
|
withCapture(
|
||||||
|
logger,
|
||||||
|
{ feature: "blog", layer: "use-case", name: "blog.getArticles" },
|
||||||
getArticlesUseCase(repo),
|
getArticlesUseCase(repo),
|
||||||
|
),
|
||||||
);
|
);
|
||||||
const wrappedCtrl = withSpan(
|
const wrappedCtrl = withSpan(
|
||||||
tracer,
|
tracer,
|
||||||
{ name: "blog.getArticles", op: "controller" },
|
{ name: "blog.getArticles", op: "controller" },
|
||||||
|
withCapture(
|
||||||
|
logger,
|
||||||
|
{ feature: "blog", layer: "controller", name: "blog.getArticles" },
|
||||||
getArticlesController(wrappedUC),
|
getArticlesController(wrappedUC),
|
||||||
|
),
|
||||||
);
|
);
|
||||||
```
|
```
|
||||||
|
|
||||||
**Capture rules:**
|
`withSpan` is outermost; `withCapture` is between span and factory so the error is captured before the span closes with error status. Bodies stay vendor-clean — neither use cases nor controllers call `tracer` / `logger` inline.
|
||||||
|
|
||||||
|
**Capture rules** (each error captured exactly once via the `__sentryReported` flag from `core-shared/instrumentation/reported-flag.ts`):
|
||||||
|
|
||||||
| Layer | Captures | Doesn't capture |
|
| Layer | Captures | Doesn't capture |
|
||||||
|---|---|---|
|
|---|---|---|
|
||||||
| Repository | Infra/Payload errors that originate here | Bubbled errors |
|
| Repository | Infra/Payload errors that originate here (inline in catch) | Bubbled errors |
|
||||||
| Use case | Business-rule violations originated in this body | Errors from repos (already captured) |
|
| Use case | Business-rule violations + output-schema failures originated in this body (via `withCapture`) | Errors from repos — flag set, `withCapture` bails |
|
||||||
| Controller | `InputParseError` from safeParse failure | Anything else |
|
| Controller | `InputParseError` from `safeParse` failure (via `withCapture`) | Errors from use cases — flag set, `withCapture` bails |
|
||||||
| `defineErrorMiddleware` | Nothing — maps domain → TRPCError only | — |
|
| `defineErrorMiddleware` | Nothing — maps domain → TRPCError only | — |
|
||||||
|
|
||||||
**Boundary rule (eslint-enforced, R40):**
|
**Boundary rule (eslint-enforced, R40):**
|
||||||
|
|||||||
@@ -51,8 +51,8 @@ Turborepo + pnpm monorepo organized by vertical features. Each feature (`auth`,
|
|||||||
- **Three binding modes per feature** — Each feature exports two binders: `./di/bind-production` (real Payload) and `./di/bind-dev-seed` (populated mock). The app's `bindAll()` dispatcher in `apps/web-next/src/server/bind-production.ts` picks one by env: `USE_DEV_SEED="true"` → dev seed; `NODE_ENV="production"` → production; otherwise → dev seed (developer default so `pnpm dev` boots without Payload). Dev seed lives in `src/__seeds__/dev.ts` as a lazy `buildDev<Entities>()` function that uses the feature's existing factory
|
- **Three binding modes per feature** — Each feature exports two binders: `./di/bind-production` (real Payload) and `./di/bind-dev-seed` (populated mock). The app's `bindAll()` dispatcher in `apps/web-next/src/server/bind-production.ts` picks one by env: `USE_DEV_SEED="true"` → dev seed; `NODE_ENV="production"` → production; otherwise → dev seed (developer default so `pnpm dev` boots without Payload). Dev seed lives in `src/__seeds__/dev.ts` as a lazy `buildDev<Entities>()` function that uses the feature's existing factory
|
||||||
- **App bootstrap** — Each app calls `bindAll()` from a server entry point (page server component, route handler) before resolving any feature controller. The dispatcher is idempotent
|
- **App bootstrap** — Each app calls `bindAll()` from a server entry point (page server component, route handler) before resolving any feature controller. The dispatcher is idempotent
|
||||||
- **Instrumentation lives in `core-shared/instrumentation/`** — Two interfaces (`ITracer`, `ILogger`), three implementations (`NoopTracer`/`NoopLogger`, `SentryTracer`/`SentryLogger`, and `RecordingTracer`/`RecordingLogger` from `core-testing`). Feature packages MUST NOT import `@sentry/*` directly (R40, eslint-enforced)
|
- **Instrumentation lives in `core-shared/instrumentation/`** — Two interfaces (`ITracer`, `ILogger`), three implementations (`NoopTracer`/`NoopLogger`, `SentryTracer`/`SentryLogger`, and `RecordingTracer`/`RecordingLogger` from `core-testing`). Feature packages MUST NOT import `@sentry/*` directly (R40, eslint-enforced)
|
||||||
- **Spans applied at DI bind time** — Use cases + controllers wrapped via `withSpan(tracer, { name: "<feature>.<useCase>", op: "use-case" }, factory(...))` inside `bind-production` / `bind-dev-seed`. Repository methods emit explicit `tracer.startSpan({ name: "<entity>.<method>", op: "repository", attributes: {...} }, ...)` (R41, R42)
|
- **Spans + capture composed at DI bind time** — Use cases + controllers wrapped via `withSpan(tracer, spanOpts, withCapture(logger, tags, factory(deps)))` inside `bind-production` / `bind-dev-seed`. `withSpan` is outermost so an errored span's timing reflects the capture-and-rethrow. Repository methods are different — they call `this.tracer.startSpan(...)` and `this.logger.captureException(...)` inline per method because they own per-call attributes (R41, R42)
|
||||||
- **Capture at throw sites only** — Repository catch blocks call `this.logger.captureException(err, { tags: { feature, repo, method } })`; use cases capture errors they originate; the tRPC error middleware does NOT capture (R43, R44)
|
- **Capture at throw sites only, with double-report guard** — Repos capture infra errors inline; use cases + controllers capture via `withCapture` at bind time; `defineErrorMiddleware` never captures (R43, R44). Each error gets a non-enumerable `__sentryReported` flag the first time it's captured; `withCapture`, `SentryLogger`, and `RecordingLogger` all bail if the flag is set, so a bubbled error surfaces exactly once with the inner-most layer's tags (helper at `core-shared/instrumentation/reported-flag.ts`)
|
||||||
- **PII handling is non-negotiable** — `sendDefaultPii: false` everywhere (R31, CI grep gate); replay default-masks all text/inputs/media (R34, R35, allowlist starts empty); `Sentry.setUser({ id })` only — no email/username (R36); `beforeSend` + `beforeSendTransaction` scrubbers strip emails/passwords/tokens/cookies/auth/IPs (R32, R33)
|
- **PII handling is non-negotiable** — `sendDefaultPii: false` everywhere (R31, CI grep gate); replay default-masks all text/inputs/media (R34, R35, allowlist starts empty); `Sentry.setUser({ id })` only — no email/username (R36); `beforeSend` + `beforeSendTransaction` scrubbers strip emails/passwords/tokens/cookies/auth/IPs (R32, R33)
|
||||||
- **Three apps, three Sentry projects** — `WEB_NEXT_SENTRY_DSN`, `CMS_SENTRY_DSN`, `WEB_TANSTACK_SENTRY_DSN`. Browser DSNs use `NEXT_PUBLIC_` (web-next) and `VITE_` (web-tanstack) prefixes
|
- **Three apps, three Sentry projects** — `WEB_NEXT_SENTRY_DSN`, `CMS_SENTRY_DSN`, `WEB_TANSTACK_SENTRY_DSN`. Browser DSNs use `NEXT_PUBLIC_` (web-next) and `VITE_` (web-tanstack) prefixes
|
||||||
- **Instrumentation binding is orthogonal to repo binding** — `bindAll()`'s Rule 0 (DSN → Sentry vs Noop) is independent of `USE_DEV_SEED` / `NODE_ENV`. Run `pnpm dev` with `WEB_NEXT_SENTRY_DSN` set to test the integration locally
|
- **Instrumentation binding is orthogonal to repo binding** — `bindAll()`'s Rule 0 (DSN → Sentry vs Noop) is independent of `USE_DEV_SEED` / `NODE_ENV`. Run `pnpm dev` with `WEB_NEXT_SENTRY_DSN` set to test the integration locally
|
||||||
|
|||||||
@@ -112,8 +112,9 @@ apps/web-next/src/server/bind-production.ts (bindAll)
|
|||||||
│ ├─ blogContainer.bind(TRACER).toConstantValue(tracer)
|
│ ├─ blogContainer.bind(TRACER).toConstantValue(tracer)
|
||||||
│ ├─ blogContainer.bind(LOGGER).toConstantValue(logger)
|
│ ├─ blogContainer.bind(LOGGER).toConstantValue(logger)
|
||||||
│ ├─ ArticlesRepository(config, tracer, logger) → bound to IArticlesRepository
|
│ ├─ ArticlesRepository(config, tracer, logger) → bound to IArticlesRepository
|
||||||
│ ├─ withSpan(tracer, ...) wraps every use case → bound to UseCase symbol
|
│ │ (real repo: inline this.tracer.startSpan + this.logger.captureException per method)
|
||||||
│ └─ withSpan(tracer, ...) wraps every controller → bound to Controller symbol
|
│ ├─ withSpan(tracer, ..., withCapture(logger, ..., useCase(deps))) → UseCase symbol
|
||||||
|
│ └─ withSpan(tracer, ..., withCapture(logger, ..., controller(uc))) → Controller symbol
|
||||||
│
|
│
|
||||||
└─ (same for auth, marketing-pages, navigation, media)
|
└─ (same for auth, marketing-pages, navigation, media)
|
||||||
```
|
```
|
||||||
|
|||||||
@@ -694,7 +694,7 @@ Invoke the `superpowers:writing-plans` skill to produce a detailed, executable i
|
|||||||
|
|
||||||
- `infrastructure/repositories/<entity>.repository.ts` — constructor takes `(config, tracer, logger)` with Noop defaults; every public method's body is wrapped in `tracer.startSpan(...)` and any `catch` block calls `logger.captureException(err, { tags: { feature, repo, method } })` before re-throwing.
|
- `infrastructure/repositories/<entity>.repository.ts` — constructor takes `(config, tracer, logger)` with Noop defaults; every public method's body is wrapped in `tracer.startSpan(...)` and any `catch` block calls `logger.captureException(err, { tags: { feature, repo, method } })` before re-throwing.
|
||||||
- `infrastructure/repositories/<entity>.repository.mock.ts` — same constructor/wrapping shape (no catch — mocks don't originate infra errors).
|
- `infrastructure/repositories/<entity>.repository.mock.ts` — same constructor/wrapping shape (no catch — mocks don't originate infra errors).
|
||||||
- `di/bind-production.ts` — signature `(config, tracer, logger)`. Binds TRACER + LOGGER to the feature container; constructs the real repo with tracer/logger; wraps every use case + controller via `withSpan` at bind time.
|
- `di/bind-production.ts` — signature `(config, tracer, logger)`. Binds TRACER + LOGGER to the feature container; constructs the real repo with tracer/logger; wraps every use case + controller via `withSpan(withCapture(factory(deps)))` at bind time. `withSpan` is outermost so an errored span's timing reflects the capture-and-rethrow; `withCapture` honours the `__sentryReported` flag so a bubbled error from the repo isn't re-captured.
|
||||||
- `di/bind-dev-seed.ts` — signature `(tracer, logger)`. Same wrapping as bind-production but with the populated mock.
|
- `di/bind-dev-seed.ts` — signature `(tracer, logger)`. Same wrapping as bind-production but with the populated mock.
|
||||||
|
|
||||||
**Required exports (per feature root):** unchanged.
|
**Required exports (per feature root):** unchanged.
|
||||||
|
|||||||
@@ -657,11 +657,11 @@ E2E tests live in `apps/web-next/e2e/`. The `webServer` block in `apps/web-next/
|
|||||||
|
|
||||||
## Asserting spans and captures (Plan 10)
|
## Asserting spans and captures (Plan 10)
|
||||||
|
|
||||||
Use cases, controllers, and repositories emit OpenTelemetry-style spans through the `ITracer` interface. Tests that need to assert span shape inject a `RecordingTracer`:
|
Use cases, controllers, and repositories emit OpenTelemetry-style spans through the `ITracer` interface. Repositories also call `logger.captureException` inline; use cases and controllers get capture composed in via `withCapture` at DI bind time. Tests that need to assert either inject `RecordingTracer` + `RecordingLogger`:
|
||||||
|
|
||||||
```ts
|
```ts
|
||||||
import { RecordingTracer, RecordingLogger } from "@repo/core-testing/instrumentation";
|
import { RecordingTracer, RecordingLogger } from "@repo/core-testing/instrumentation";
|
||||||
import { withSpan } from "@repo/core-shared/instrumentation";
|
import { withSpan, withCapture } from "@repo/core-shared/instrumentation";
|
||||||
import { MockArticlesRepository } from "@/infrastructure/repositories/articles.repository.mock";
|
import { MockArticlesRepository } from "@/infrastructure/repositories/articles.repository.mock";
|
||||||
import { getArticlesUseCase } from "@/application/use-cases/get-articles.use-case";
|
import { getArticlesUseCase } from "@/application/use-cases/get-articles.use-case";
|
||||||
|
|
||||||
@@ -671,11 +671,15 @@ describe("blog.getArticles use case", () => {
|
|||||||
const logger = new RecordingLogger();
|
const logger = new RecordingLogger();
|
||||||
const repo = new MockArticlesRepository(tracer, logger);
|
const repo = new MockArticlesRepository(tracer, logger);
|
||||||
// Use cases are wrapped at DI bind time. For direct-injection tests,
|
// Use cases are wrapped at DI bind time. For direct-injection tests,
|
||||||
// wrap inline:
|
// mirror the binder's sandwich: withSpan(withCapture(factory(deps))).
|
||||||
const wrapped = withSpan(
|
const wrapped = withSpan(
|
||||||
tracer,
|
tracer,
|
||||||
{ name: "blog.getArticles", op: "use-case" },
|
{ name: "blog.getArticles", op: "use-case" },
|
||||||
|
withCapture(
|
||||||
|
logger,
|
||||||
|
{ feature: "blog", layer: "use-case", name: "blog.getArticles" },
|
||||||
getArticlesUseCase(repo),
|
getArticlesUseCase(repo),
|
||||||
|
),
|
||||||
);
|
);
|
||||||
await wrapped({ limit: 10 });
|
await wrapped({ limit: 10 });
|
||||||
expect(tracer.findSpan("blog.getArticles")?.op).toBe("use-case");
|
expect(tracer.findSpan("blog.getArticles")?.op).toBe("use-case");
|
||||||
@@ -684,17 +688,19 @@ describe("blog.getArticles use case", () => {
|
|||||||
});
|
});
|
||||||
```
|
```
|
||||||
|
|
||||||
**Capture assertions** use `RecordingLogger`:
|
**Capture assertions** use `RecordingLogger`. Note: `RecordingLogger.captureException` honours the `__sentryReported` flag — so when a repo's catch block captures and the outer `withCapture` sees the bubbled error, only the inner-most call records:
|
||||||
|
|
||||||
```ts
|
```ts
|
||||||
const logger = new RecordingLogger();
|
const logger = new RecordingLogger();
|
||||||
const repo = new MockArticlesRepository(tracer, logger);
|
const repo = new MockArticlesRepository(tracer, logger);
|
||||||
// Force an infra error in your test setup, then:
|
// Force an infra error in your test setup, then:
|
||||||
expect(logger.captures).toHaveLength(1);
|
expect(logger.captures).toHaveLength(1); // exactly one — flag prevents double
|
||||||
expect(logger.captures[0]).toMatchObject({
|
expect(logger.captures[0]).toMatchObject({
|
||||||
kind: "exception",
|
kind: "exception",
|
||||||
ctx: { tags: { feature: "blog", repo: "articles", method: "getArticles" } },
|
ctx: { tags: { feature: "blog", repo: "articles", method: "getArticles" } },
|
||||||
});
|
});
|
||||||
```
|
```
|
||||||
|
|
||||||
|
For an end-to-end example (controller → use case → repo, all wrapped, asserting no double-capture across layers), see `packages/blog/tests/r44-no-double-capture.test.ts`.
|
||||||
|
|
||||||
**Default mocks** (when you don't need assertions): construct `new MockArticlesRepository()` with no args — the constructor defaults bind `NoopTracer` + `NoopLogger`.
|
**Default mocks** (when you don't need assertions): construct `new MockArticlesRepository()` with no args — the constructor defaults bind `NoopTracer` + `NoopLogger`.
|
||||||
|
|||||||
@@ -46,16 +46,24 @@ Covered areas:
|
|||||||
- `SentryTracer` / `SentryLogger` — adapters over `@sentry/nextjs`. Live in `sentry/` subfolder. **The `sentry/` subfolder is the only path in `packages/` permitted to import `@sentry/*`** (R40), with the additional exception of `instrumentation/di/bind-sentry-instrumentation.{ts,test.ts}`.
|
- `SentryTracer` / `SentryLogger` — adapters over `@sentry/nextjs`. Live in `sentry/` subfolder. **The `sentry/` subfolder is the only path in `packages/` permitted to import `@sentry/*`** (R40), with the additional exception of `instrumentation/di/bind-sentry-instrumentation.{ts,test.ts}`.
|
||||||
- `RecordingTracer` / `RecordingLogger` — in `@repo/core-testing/instrumentation`, not here.
|
- `RecordingTracer` / `RecordingLogger` — in `@repo/core-testing/instrumentation`, not here.
|
||||||
|
|
||||||
**`with-span.ts`:** higher-order helper used at DI binding time to wrap use case + controller factory results in a span. Pattern:
|
**`with-span.ts` + `with-capture.ts`:** two higher-order helpers used at DI binding time to wrap use case + controller factory results. The binders apply them as a sandwich — `withSpan` outermost, `withCapture` between span and factory:
|
||||||
|
|
||||||
```ts
|
```ts
|
||||||
const wrapped = withSpan(
|
const wrapped = withSpan(
|
||||||
tracer,
|
tracer,
|
||||||
{ name: "blog.getArticles", op: "use-case" },
|
{ name: "blog.getArticles", op: "use-case" },
|
||||||
|
withCapture(
|
||||||
|
logger,
|
||||||
|
{ feature: "blog", layer: "use-case", name: "blog.getArticles" },
|
||||||
factory(deps),
|
factory(deps),
|
||||||
|
),
|
||||||
);
|
);
|
||||||
```
|
```
|
||||||
|
|
||||||
|
`withSpan` is pure delegation to `tracer.startSpan`. `withCapture` catches thrown errors, calls `logger.captureException(err, { tags })`, marks the `__sentryReported` flag, and re-throws — but bails if the flag is already set so the inner-most layer wins.
|
||||||
|
|
||||||
|
**`reported-flag.ts`:** small module exporting `markReported(err)` and `isReported(err)`. Used by `withCapture` and `SentryLogger`. `RecordingLogger` carries an inlined copy (tooling → core import is disallowed by the boundary rule).
|
||||||
|
|
||||||
**Symbols:** `INSTRUMENTATION_SYMBOLS.TRACER`, `INSTRUMENTATION_SYMBOLS.LOGGER` (both `Symbol.for(...)` so cross-realm equality holds).
|
**Symbols:** `INSTRUMENTATION_SYMBOLS.TRACER`, `INSTRUMENTATION_SYMBOLS.LOGGER` (both `Symbol.for(...)` so cross-realm equality holds).
|
||||||
|
|
||||||
**`sentry/scrub.ts`:** PII scrubbers used by every `Sentry.init()` call across the monorepo. Substring-based key matching catches derived names (`userEmail`, `accessToken`, `apiKey`, `ipAddress`). IPv4/IPv6 are also redacted from string values via the `[redacted-ip]` token.
|
**`sentry/scrub.ts`:** PII scrubbers used by every `Sentry.init()` call across the monorepo. Substring-based key matching catches derived names (`userEmail`, `accessToken`, `apiKey`, `ipAddress`). IPv4/IPv6 are also redacted from string values via the `[redacted-ip]` token.
|
||||||
@@ -68,7 +76,7 @@ const wrapped = withSpan(
|
|||||||
|
|
||||||
**Subpath exports** (`package.json#exports`):
|
**Subpath exports** (`package.json#exports`):
|
||||||
|
|
||||||
- `./instrumentation` — barrel (interfaces + Noops + withSpan + symbols + binders + node/react init helpers)
|
- `./instrumentation` — barrel (interfaces + Noops + withSpan + withCapture + reported-flag helpers + symbols + binders + node/react init helpers)
|
||||||
- `./instrumentation/sentry/init-server` — Next.js server init helper
|
- `./instrumentation/sentry/init-server` — Next.js server init helper
|
||||||
- `./instrumentation/sentry/init-client` — Next.js client init helper
|
- `./instrumentation/sentry/init-client` — Next.js client init helper
|
||||||
- `./instrumentation/sentry/init-server-node` — `@sentry/node` server init (TanStack Start)
|
- `./instrumentation/sentry/init-server-node` — `@sentry/node` server init (TanStack Start)
|
||||||
|
|||||||
Reference in New Issue
Block a user