feat(instrumentation): close R44 gap — throw-site capture for use cases + controllers
Plan 10 documented R44 (capture at originating-throw layer) but only the R43 repo leg was wired. captureException had zero call sites in any controller or use-case body. This commit closes the gap. Mechanism: - Extract __sentryReported flag helpers into core-shared/instrumentation/ reported-flag.ts. SentryLogger switches to importing them; RecordingLogger carries an inlined copy (tooling → core boundary disallows the import). - Add withCapture(logger, tags, fn) higher-order wrapper paralleling withSpan. On throw: capture-with-tags, mark, re-throw. Bail if the flag was already set — covers the bubbled-from-repo case so each error surfaces in the logger exactly once with the inner-most layer's tags. - Apply withSpan(withCapture(factory)) in every feature's bind-production and bind-dev-seed: auth (3 use cases × 3 controllers), blog (3×3), marketing-pages (2×2), navigation (1×1), media (3×3). Span is outermost so the errored span timing reflects the capture-and-rethrow. - RecordingLogger.captureException now also honours the flag — test capture counts stay honest when both repo and outer layer wrap. Tests: - packages/core-shared/src/instrumentation/with-capture.test.ts — 4 cases covering success, capture-on-throw, mark-on-capture, no-double via the flag. - packages/blog/tests/r44-no-double-capture.test.ts — 3 cases: repo throw → 1 capture with repo tags; controller parse fail → 1 capture with controller tags; success → 0 captures. Verification: pnpm test 26/26, pnpm lint 15/15, pnpm typecheck 14/14. Docs: ADR-014 and the refactor log gain a "Post-merge follow-up" section recording the gap, the fix, and the underlying lesson (don't describe intent as shipped state — grep first). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -12,6 +12,8 @@ export type {
|
||||
export { NoopTracer } from "./noop-tracer";
|
||||
export { NoopLogger } from "./noop-logger";
|
||||
export { withSpan } from "./with-span";
|
||||
export { withCapture } from "./with-capture";
|
||||
export { isReported, markReported } from "./reported-flag";
|
||||
export { INSTRUMENTATION_SYMBOLS } from "./symbols";
|
||||
export { bindNoopInstrumentation } from "./di/bind-noop-instrumentation";
|
||||
export {
|
||||
|
||||
24
packages/core-shared/src/instrumentation/reported-flag.ts
Normal file
24
packages/core-shared/src/instrumentation/reported-flag.ts
Normal file
@@ -0,0 +1,24 @@
|
||||
// Non-enumerable flag used by every ILogger implementation to skip
|
||||
// already-reported errors. The flag is non-enumerable so JSON.stringify
|
||||
// and {...err} spread won't surface it.
|
||||
|
||||
const REPORTED = "__sentryReported" as const;
|
||||
|
||||
export function isReported(err: unknown): boolean {
|
||||
return (
|
||||
err !== null &&
|
||||
typeof err === "object" &&
|
||||
Boolean((err as Record<string, unknown>)[REPORTED])
|
||||
);
|
||||
}
|
||||
|
||||
export function markReported(err: unknown): void {
|
||||
if (err !== null && typeof err === "object" && !isReported(err)) {
|
||||
Object.defineProperty(err, REPORTED, {
|
||||
value: true,
|
||||
enumerable: false,
|
||||
configurable: false,
|
||||
writable: false,
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -1,27 +1,7 @@
|
||||
// packages/core-shared/src/instrumentation/sentry/sentry-logger.ts
|
||||
import * as Sentry from "@sentry/nextjs";
|
||||
import type { ILogger, Breadcrumb, CaptureContext } from "../logger.interface";
|
||||
|
||||
const REPORTED = "__sentryReported" as const;
|
||||
|
||||
function isReported(err: unknown): boolean {
|
||||
return (
|
||||
err !== null &&
|
||||
typeof err === "object" &&
|
||||
Boolean((err as Record<string, unknown>)[REPORTED])
|
||||
);
|
||||
}
|
||||
|
||||
function markReported(err: unknown): void {
|
||||
if (err !== null && typeof err === "object") {
|
||||
Object.defineProperty(err, REPORTED, {
|
||||
value: true,
|
||||
enumerable: false,
|
||||
configurable: false,
|
||||
writable: false,
|
||||
});
|
||||
}
|
||||
}
|
||||
import { isReported, markReported } from "../reported-flag";
|
||||
|
||||
export class SentryLogger implements ILogger {
|
||||
captureException(err: unknown, ctx?: CaptureContext): void {
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { withCapture } from "@/instrumentation/with-capture";
|
||||
import type { ILogger } from "@/instrumentation/logger.interface";
|
||||
import { isReported } from "@/instrumentation/reported-flag";
|
||||
|
||||
function makeLogger(): ILogger & { captureException: ReturnType<typeof vi.fn> } {
|
||||
return {
|
||||
captureException: vi.fn(),
|
||||
captureMessage: vi.fn(),
|
||||
addBreadcrumb: vi.fn(),
|
||||
setUser: vi.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
describe("withCapture", () => {
|
||||
it("does not capture on success", async () => {
|
||||
const logger = makeLogger();
|
||||
const wrapped = withCapture(logger, { layer: "use-case" }, async (x: number) => x + 1);
|
||||
await expect(wrapped(1)).resolves.toBe(2);
|
||||
expect(logger.captureException).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("captures with tags and re-throws on failure", async () => {
|
||||
const logger = makeLogger();
|
||||
const err = new Error("boom");
|
||||
const wrapped = withCapture(logger, { layer: "use-case", name: "blog.x" }, async () => {
|
||||
throw err;
|
||||
});
|
||||
await expect(wrapped()).rejects.toBe(err);
|
||||
expect(logger.captureException).toHaveBeenCalledTimes(1);
|
||||
expect(logger.captureException).toHaveBeenCalledWith(err, {
|
||||
tags: { layer: "use-case", name: "blog.x" },
|
||||
});
|
||||
});
|
||||
|
||||
it("marks the error as reported after first capture", async () => {
|
||||
const logger = makeLogger();
|
||||
const err = new Error("boom");
|
||||
const wrapped = withCapture(logger, { layer: "use-case" }, async () => {
|
||||
throw err;
|
||||
});
|
||||
await expect(wrapped()).rejects.toBe(err);
|
||||
expect(isReported(err)).toBe(true);
|
||||
});
|
||||
|
||||
it("does NOT capture again when the same error already carries the flag", async () => {
|
||||
const logger = makeLogger();
|
||||
const err = new Error("boom");
|
||||
// Simulate an inner layer (repo) having already captured + marked.
|
||||
const inner = withCapture(logger, { layer: "repo" }, async () => {
|
||||
throw err;
|
||||
});
|
||||
const outer = withCapture(logger, { layer: "use-case" }, () => inner());
|
||||
|
||||
await expect(outer()).rejects.toBe(err);
|
||||
// Only the inner layer captured it; outer saw the flag and bailed.
|
||||
expect(logger.captureException).toHaveBeenCalledTimes(1);
|
||||
expect(logger.captureException).toHaveBeenCalledWith(err, {
|
||||
tags: { layer: "repo" },
|
||||
});
|
||||
});
|
||||
});
|
||||
40
packages/core-shared/src/instrumentation/with-capture.ts
Normal file
40
packages/core-shared/src/instrumentation/with-capture.ts
Normal file
@@ -0,0 +1,40 @@
|
||||
import type { ILogger } from "./logger.interface";
|
||||
import { isReported, markReported } from "./reported-flag";
|
||||
|
||||
/**
|
||||
* Higher-order wrapper applied at DI bind time. Mirrors `withSpan`: takes a
|
||||
* factory result `(args) => Promise<R>` and returns the same shape, but any
|
||||
* thrown error is captured via `logger.captureException(err, { tags })` before
|
||||
* being re-thrown.
|
||||
*
|
||||
* Skips capture if the error already carries the `__sentryReported` flag —
|
||||
* this is what prevents double-capture when the same error bubbles through
|
||||
* a wrapped repo → use case → controller chain (the repo's catch site
|
||||
* captures first; outer wrappers see the flag and bail).
|
||||
*
|
||||
* Usage at bind time:
|
||||
*
|
||||
* const captured = withCapture(logger, { feature: "blog", layer: "use-case", name: "blog.getArticles" }, factory(deps));
|
||||
* const wrapped = withSpan(tracer, opts, captured);
|
||||
*
|
||||
* Span wraps capture: the span timing reflects the captured-and-rethrown
|
||||
* failure (errored span gets a duration), and the capture has accurate
|
||||
* tags by the time it fires.
|
||||
*/
|
||||
export function withCapture<Args extends unknown[], R>(
|
||||
logger: ILogger,
|
||||
tags: Record<string, string>,
|
||||
fn: (...args: Args) => Promise<R>,
|
||||
): (...args: Args) => Promise<R> {
|
||||
return async (...args) => {
|
||||
try {
|
||||
return await fn(...args);
|
||||
} catch (err) {
|
||||
if (!isReported(err)) {
|
||||
logger.captureException(err, { tags });
|
||||
markReported(err);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
};
|
||||
}
|
||||
Reference in New Issue
Block a user