diff --git a/src/queue/ai-review-orchestration.ts b/src/queue/ai-review-orchestration.ts index c0fb71962c..2c6078d248 100644 --- a/src/queue/ai-review-orchestration.ts +++ b/src/queue/ai-review-orchestration.ts @@ -752,7 +752,7 @@ export async function runAiReviewForAdvisory( public_notes: hasPublicReviewAssessment(result.advisoryNotes), /* v8 ignore next -- current review runner always supplies diagnostics for completed AI attempts. */ review_diagnostics: result.reviewDiagnostics ?? [], - }); + }, "ai_review_inconclusive"); } args.advisory.findings.push(...findings); const metadataFor = ( @@ -828,6 +828,7 @@ export async function runAiReviewForAdvisory( null, combine: env.AI_REVIEW_PLAN?.combine ?? null, }, + "ai_review_public_summary_missing", ); return { notes: @@ -848,13 +849,16 @@ export async function runAiReviewForAdvisory( error: errorMessage(error), }), ); + // error is a genuinely caught exception here (unlike the two captures above, which construct their own + // Error to report a known condition) -- named to mirror the structured log's own "event" field just above, + // not the exception's native class, so every unexpected review crash groups under one readable title. captureReviewFailure(error, { kind: "review", installationId: args.installationId, repo: args.repoFullName, pr: args.pr.number, head_sha: args.advisory.headSha, - }); + }, "ai_review_failed"); return undefined; } finally { // #regate-dup-prep: only release a lock THIS call actually claimed. A caller-supplied diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 6afb265eb0..fb07b94dee 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -7941,7 +7941,7 @@ async function maybePublishPrPublicSurface( pr: pr.number, head_sha: advisory.headSha, failedOutputs: failedOutputs.map((failure) => failure.output), - }); + }, "pr_public_surface_publish_failed"); // At least one output failed for a reason that can plausibly clear on its own (rate limit / 5xx / momentary // token issue) — retry the whole job instead of leaving the review permanently unposted. A mix of transient // and permanent failures still retries: the permanent one re-fails identically next pass and re-audits, but @@ -9039,7 +9039,7 @@ async function maybePublishPrPublicSurface( head_sha: advisory.headSha, reviewer_count: aiReview?.reviewerCount ?? 0, public_notes: hasPublicReviewAssessment(aiReview?.notes), - }); + }, "ai_review_public_summary_missing"); } // Secrets-scan (#audit-3.4): always scans the REAL resolved diff and, on a CONCRETE credential hit, appends a diff --git a/src/selfhost/pg-queue.ts b/src/selfhost/pg-queue.ts index 932a422514..84f513b69f 100644 --- a/src/selfhost/pg-queue.ts +++ b/src/selfhost/pg-queue.ts @@ -632,7 +632,7 @@ export function createPgQueue( error: errorMessageWithCause(error), }), ); - captureError(error, { kind: "queue_dead_letter_revive_crashed" }); + captureError(error, { kind: "queue_dead_letter_revive_crashed" }, "queue_dead_letter_revive_crashed"); } } @@ -758,7 +758,7 @@ export function createPgQueue( error: errorMessageWithCause(error), }), ); - captureError(error, { kind: "queue_foreground_liveness_release_crashed" }); + captureError(error, { kind: "queue_foreground_liveness_release_crashed" }, "queue_foreground_liveness_release_crashed"); } } @@ -1088,7 +1088,7 @@ export function createPgQueue( reason: "processing_timeout", recovered, timeoutMs: processingTimeoutMs, - }); + }, "processing_timeout"); } const job = await claimNext(); if (!job) return false; @@ -1116,7 +1116,7 @@ export function createPgQueue( kind: "job_dead", reason: "unparseable_payload", jobId: job.id, - }); + }, "unparseable_payload"); return true; } const jobTraceParent = message.type === "github-webhook" ? message.traceParent : undefined; @@ -1427,7 +1427,7 @@ export function createPgQueue( jobType: extractPayloadType(job.payload), jobId: job.id, attempts, - }); + }, "job_dead"); } else { const retryDelayMs = consumingRetryDelayMs(error, backoff(attempts)); await pool.query( @@ -1474,7 +1474,7 @@ export function createPgQueue( error: errorMessageWithCause(error), }), ); - captureError(error, { kind: "queue_pump_crashed" }); + captureError(error, { kind: "queue_pump_crashed" }, "queue_pump_crashed"); } finally { active--; } diff --git a/src/selfhost/sentry.ts b/src/selfhost/sentry.ts index c28a332e38..31114452b3 100644 --- a/src/selfhost/sentry.ts +++ b/src/selfhost/sentry.ts @@ -431,26 +431,40 @@ export async function buildSentryOpenTelemetryBridge(): Promise, + eventName?: string, ): void { if (!active || !Sentry) return; Sentry.withScope((scope) => { setOtelTraceScope(scope); if (context) { const safeContext = hashedInstallationContext(context); scope.setContext("gittensory", safeContext); applyOperationalTags(scope, safeContext); } - Sentry!.captureException( - error instanceof Error ? error : new Error(String(error)), - ); + Sentry!.captureException(namedCaptureError(error, eventName)); }); } /** Capture a failed review at ERROR level, tagged by repo/PR/SHA for triage. A review that cannot be produced is a - * real failure the maintainer must SEE — not a warning that hides in the noise. No-op when off. */ + * real failure the maintainer must SEE — not a warning that hides in the noise. No-op when off. `eventName`, when + * given, becomes the Sentry issue title's prefix (see {@link namedCaptureError}) instead of the generic "Error". */ export function captureReviewFailure( error: unknown, context?: Record, + eventName?: string, ): void { if (!active || !Sentry) return; Sentry.withScope((scope) => { @@ -461,9 +475,7 @@ export function captureReviewFailure( scope.setContext("review", safeContext); applyOperationalTags(scope, safeContext); } - Sentry!.captureException( - error instanceof Error ? error : new Error(String(error)), - ); + Sentry!.captureException(namedCaptureError(error, eventName)); }); } diff --git a/src/selfhost/sqlite-queue.ts b/src/selfhost/sqlite-queue.ts index 19eb5071fd..2bff63908b 100644 --- a/src/selfhost/sqlite-queue.ts +++ b/src/selfhost/sqlite-queue.ts @@ -320,7 +320,7 @@ export function createSqliteQueue( error: errorMessageWithCause(error), }), ); - captureError(error, { kind: "queue_dead_letter_revive_crashed" }); + captureError(error, { kind: "queue_dead_letter_revive_crashed" }, "queue_dead_letter_revive_crashed"); } } @@ -441,7 +441,7 @@ export function createSqliteQueue( error: errorMessageWithCause(error), }), ); - captureError(error, { kind: "queue_foreground_liveness_release_crashed" }); + captureError(error, { kind: "queue_foreground_liveness_release_crashed" }, "queue_foreground_liveness_release_crashed"); } } @@ -826,7 +826,7 @@ export function createSqliteQueue( reason: "processing_timeout", recovered, timeoutMs: processingTimeoutMs, - }); + }, "processing_timeout"); } const job = claimNext(); if (!job) return false; @@ -854,7 +854,7 @@ export function createSqliteQueue( kind: "job_dead", reason: "unparseable_payload", jobId: job.id, - }); + }, "unparseable_payload"); return true; } const jobTraceParent = message.type === "github-webhook" ? message.traceParent : undefined; @@ -1113,7 +1113,7 @@ export function createSqliteQueue( jobType: extractPayloadType(job.payload), jobId: job.id, attempts, - }); + }, "job_dead"); } else { const retryDelayMs = consumingRetryDelayMs(error, backoff(attempts)); driver.query( @@ -1163,7 +1163,7 @@ export function createSqliteQueue( error: errorMessageWithCause(error), }), ); - captureError(error, { kind: "queue_pump_crashed" }); + captureError(error, { kind: "queue_pump_crashed" }, "queue_pump_crashed"); } finally { active--; } diff --git a/src/server.ts b/src/server.ts index 00a7a7deda..5d1fed9a3c 100644 --- a/src/server.ts +++ b/src/server.ts @@ -317,12 +317,12 @@ async function main(): Promise { }), ); process.on("uncaughtException", (error) => { - captureError(error, { kind: "uncaughtException" }); + captureError(error, { kind: "uncaughtException" }, "uncaughtException"); console.error(error); void flushSentry().finally(() => process.exit(1)); }); process.on("unhandledRejection", (reason) => { - captureError(reason, { kind: "unhandledRejection" }); + captureError(reason, { kind: "unhandledRejection" }, "unhandledRejection"); console.error(reason); }); // Central error forwarding (#1468): operational failures are structured JSON logs emitted through stdout and @@ -1069,7 +1069,7 @@ async function main(): Promise { state: orbRelayRegistrationState, register: registerOrbRelayTargetWithRetry, ...(relayDrainState ? { drainState: relayDrainState } : {}), - }).catch((error) => captureError(error, { kind: "orb_relay_register" })); + }).catch((error) => captureError(error, { kind: "orb_relay_register" }, "orb_relay_register")); void attemptOrbRelayRegistration(); setInterval(() => void attemptOrbRelayRegistration(), 60_000); // Dashboard-visible counterparts to the streak/no-progress alert gate in isOrbRelayRegistrationAlerting: @@ -1093,7 +1093,7 @@ async function main(): Promise { }; if (isD1SizeProbeEnabled(d1ProbeEnv)) { /* v8 ignore start -- self-host entrypoint timer; probe logic itself is unit-tested in d1-size-probe.test.ts. */ - const runD1Probe = () => runD1SizeProbe(d1ProbeEnv).catch((error) => captureError(error, { kind: "d1_size_probe" })); + const runD1Probe = () => runD1SizeProbe(d1ProbeEnv).catch((error) => captureError(error, { kind: "d1_size_probe" }, "d1_size_probe")); void runD1Probe(); setInterval(runD1Probe, 900_000); /* v8 ignore stop */ @@ -1127,14 +1127,14 @@ async function main(): Promise { } }; void drainRelay().catch((error) => - captureError(error, { kind: "orb_relay_drain" }), + captureError(error, { kind: "orb_relay_drain" }, "orb_relay_drain"), ); // 30s matches broker-client's request timeout so a slow/degraded broker's in-flight drain has fully // timed out (or completed) before the next tick would otherwise pile another request on top of it. setInterval( () => void drainRelay().catch((error) => - captureError(error, { kind: "orb_relay_drain" }), + captureError(error, { kind: "orb_relay_drain" }, "orb_relay_drain"), ), 30_000, ); @@ -1160,7 +1160,7 @@ async function main(): Promise { } main().catch((error) => { - captureError(error, { kind: "boot" }); + captureError(error, { kind: "boot" }, "boot"); console.error(error); /* v8 ignore next -- boot failure exits the process; shutdown helper is covered independently. */ void Promise.all([shutdownOpenTelemetry(), flushSentry()]).finally(() => process.exit(1)); diff --git a/src/services/agent-action-executor.ts b/src/services/agent-action-executor.ts index 71fae8e225..3ed53a2bb6 100644 --- a/src/services/agent-action-executor.ts +++ b/src/services/agent-action-executor.ts @@ -571,7 +571,7 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE // terminal-hold capture below and the "a real failure the maintainer must see" convention already used // for review-pass failures (selfhost/sentry.ts's captureReviewFailure, queue/processors.ts). Previously // this class of failure was audit-log-only, invisible without a manual audit_events query. - captureError(error, { kind: "agent_action_execution_failed", repo: ctx.repoFullName, pr: ctx.pullNumber, installationId: ctx.installationId, actionClass: action.actionClass }); + captureError(error, { kind: "agent_action_execution_failed", repo: ctx.repoFullName, pr: ctx.pullNumber, installationId: ctx.installationId, actionClass: action.actionClass }, "agent_action_execution_failed"); } // #2265: a permission-looking 403 on a PR-write mutation can mean the LOCAL installations.permissions // snapshot is stale after a maintainer-initiated downgrade (GitHub sends no downgrade webhook). Rate-limit @@ -817,7 +817,7 @@ export async function executeIssueMaintenanceActions(env: Env, ctx: IssueActionE await audit("error", errorMessage(error)); // Mirrors executeAgentMaintenanceActions's non-merge capture below -- issue-side label/close has no retry // loop either, so a single failure here is already this pass's terminal outcome. - captureError(error, { kind: "agent_issue_action_execution_failed", repo: ctx.repoFullName, issue: ctx.issueNumber, installationId: ctx.installationId, actionClass: action.actionClass }); + captureError(error, { kind: "agent_issue_action_execution_failed", repo: ctx.repoFullName, issue: ctx.issueNumber, installationId: ctx.installationId, actionClass: action.actionClass }, "agent_issue_action_execution_failed"); } } @@ -850,7 +850,10 @@ async function handleMergeFailure(env: Env, ctx: AgentActionExecutionContext, er // failure the maintainer must see" case captureReviewFailure already covers for an exhausted AI review pass. // Fires once per hold (not per retry attempt), so a transient failure that resolves within MERGE_RETRY_CAP // never reaches Sentry at all. - captureError(error, { kind: "agent_merge_blocked", repo: ctx.repoFullName, pr: ctx.pullNumber, installationId: ctx.installationId, reason: reason.slice(0, 280) }); + // Named "agent_merge_blocked" (not the caught exception's own class, e.g. "HttpError") so every terminal + // merge hold groups under one readable title regardless of which HTTP status caused it -- the specific + // status/reason stays in the message and the "review" context object either way. + captureError(error, { kind: "agent_merge_blocked", repo: ctx.repoFullName, pr: ctx.pullNumber, installationId: ctx.installationId, reason: reason.slice(0, 280) }, "agent_merge_blocked"); await recordAuditEvent(env, { eventType: "agent.action.merge_blocked", actor: AGENT_ACTOR, diff --git a/test/unit/agent-action-executor.test.ts b/test/unit/agent-action-executor.test.ts index 30a0d937a4..9cbe46fe6d 100644 --- a/test/unit/agent-action-executor.test.ts +++ b/test/unit/agent-action-executor.test.ts @@ -1405,7 +1405,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { expect((await auditFor(env, "merge"))?.outcome).toBe("error"); // "not mergeable" is immediately terminal (classifyMergeFailure), so this held-for-human outcome must be // Sentry-visible, not just an audit_events row a maintainer has to go looking for (#3862/#3863 gap sweep). - expect(captureSpy).toHaveBeenCalledWith(expect.any(Error), expect.objectContaining({ kind: "agent_merge_blocked", repo: "owner/repo", pr: 7 })); + expect(captureSpy).toHaveBeenCalledWith(expect.any(Error), expect.objectContaining({ kind: "agent_merge_blocked", repo: "owner/repo", pr: 7 }), "agent_merge_blocked"); captureSpy.mockRestore(); }); @@ -1444,7 +1444,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { expect(refreshInstallationHealthForInstallation).toHaveBeenCalledWith(env, 123); // Non-merge action classes have no retry loop, so a single failure is already this pass's terminal outcome // and must be Sentry-visible immediately (#3862/#3863 gap sweep). - expect(captureSpy).toHaveBeenCalledWith(expect.any(Error), expect.objectContaining({ kind: "agent_action_execution_failed", actionClass: "close" })); + expect(captureSpy).toHaveBeenCalledWith(expect.any(Error), expect.objectContaining({ kind: "agent_action_execution_failed", actionClass: "close" }), "agent_action_execution_failed"); captureSpy.mockRestore(); }); @@ -1875,7 +1875,7 @@ describe("executeIssueMaintenanceActions (#2270 issue-side actuation)", () => { const outcomes = await executeIssueMaintenanceActions(env, issueCtx(), [issueClose]); expect(outcomes[0]?.outcome).toBe("error"); expect((await auditFor(env, "close"))?.outcome).toBe("error"); - expect(captureSpy).toHaveBeenCalledWith(expect.any(Error), expect.objectContaining({ kind: "agent_issue_action_execution_failed", actionClass: "close" })); + expect(captureSpy).toHaveBeenCalledWith(expect.any(Error), expect.objectContaining({ kind: "agent_issue_action_execution_failed", actionClass: "close" }), "agent_issue_action_execution_failed"); captureSpy.mockRestore(); }); diff --git a/test/unit/ai-review-advisory.test.ts b/test/unit/ai-review-advisory.test.ts index 3db7da3ee4..d445d00c06 100644 --- a/test/unit/ai-review-advisory.test.ts +++ b/test/unit/ai-review-advisory.test.ts @@ -392,6 +392,7 @@ describe("runAiReviewForAdvisory", () => { expect.objectContaining({ status: "unparseable_output" }), ]), }), + "ai_review_inconclusive", ); captureSpy.mockRestore(); }); @@ -413,7 +414,7 @@ describe("runAiReviewForAdvisory", () => { confirmedContributor: true, }); expect(adv.findings.map((f) => f.code)).toEqual(["ai_review_inconclusive"]); - expect(captureSpy).toHaveBeenCalledWith(expect.any(Error), expect.objectContaining({ reason: "ai_review_inconclusive", repo: "acme/widgets", head_sha: "sha3" })); + expect(captureSpy).toHaveBeenCalledWith(expect.any(Error), expect.objectContaining({ reason: "ai_review_inconclusive", repo: "acme/widgets", head_sha: "sha3" }), "ai_review_inconclusive"); captureSpy.mockRestore(); }); @@ -684,6 +685,7 @@ describe("runAiReviewForAdvisory", () => { head_sha: "sha3", reviewer_count: 0, }), + "ai_review_public_summary_missing", ); captureSpy.mockRestore(); }); diff --git a/test/unit/queue-2.test.ts b/test/unit/queue-2.test.ts index 3bd6c03541..c13ac3f7f2 100644 --- a/test/unit/queue-2.test.ts +++ b/test/unit/queue-2.test.ts @@ -1489,6 +1489,7 @@ describe("queue processors", () => { reviewer_count: 0, public_notes: false, }), + "ai_review_public_summary_missing", ); captureSpy.mockRestore(); }); diff --git a/test/unit/queue-4.test.ts b/test/unit/queue-4.test.ts index f5bcff8cd6..1a7fe17253 100644 --- a/test/unit/queue-4.test.ts +++ b/test/unit/queue-4.test.ts @@ -5597,7 +5597,7 @@ describe("queue processors", () => { expect(aggregate?.metadata_json).toContain('"transient":true'); // The total publish failure (nothing reached the PR) escalates to Sentry at error level, not just the ledger — // this still fires BEFORE the retryable throw, so the failure stays observable even though the job also retries. - expect(captureSpy).toHaveBeenCalledWith(expect.any(Error), expect.objectContaining({ kind: "publish", repo: "JSONbored/gittensory" })); + expect(captureSpy).toHaveBeenCalledWith(expect.any(Error), expect.objectContaining({ kind: "publish", repo: "JSONbored/gittensory" }), "pr_public_surface_publish_failed"); captureSpy.mockRestore(); }); diff --git a/test/unit/selfhost-sentry.test.ts b/test/unit/selfhost-sentry.test.ts index b7f5a15e48..323becf4fe 100644 --- a/test/unit/selfhost-sentry.test.ts +++ b/test/unit/selfhost-sentry.test.ts @@ -578,6 +578,40 @@ describe("enabled when SENTRY_DSN is set", () => { expect(mocks.captureException).toHaveBeenCalledTimes(2); }); + it("captureError with an eventName renames the captured Error so the Sentry title isn't the generic 'Error'", async () => { + await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); + captureError(new Error("self-host queue processing lease expired"), { kind: "job_dead" }, "processing_timeout"); + expect(lastCapturedError().name).toBe("processing_timeout"); + expect(lastCapturedError().message).toBe("self-host queue processing lease expired"); + }); + + it("captureError without an eventName leaves a caught exception's own name untouched", async () => { + await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); + class HttpError extends Error { + constructor(message: string) { + super(message); + this.name = "HttpError"; + } + } + captureError(new HttpError("merge already in progress"), { kind: "agent_merge_blocked" }); + expect(lastCapturedError().name).toBe("HttpError"); + }); + + it("captureReviewFailure with an eventName renames the captured Error the same way captureError does", async () => { + await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); + captureReviewFailure(new Error("AI review inconclusive — no usable verdict for the PR head"), { repo: "o/r" }, "ai_review_inconclusive"); + expect(lastCapturedError().name).toBe("ai_review_inconclusive"); + }); + + it("captureError/captureReviewFailure with an eventName still names a non-Error value's synthesized Error", async () => { + await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); + captureError("plain string failure", undefined, "boot"); + expect(lastCapturedError().name).toBe("boot"); + expect(lastCapturedError().message).toBe("plain string failure"); + captureReviewFailure("plain string review failure", undefined, "ai_review_failed"); + expect(lastCapturedError().name).toBe("ai_review_failed"); + }); + it("adds active OTEL trace ids to captured Sentry events", async () => { await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); otelMocks.currentOtelTraceIds.mockReturnValue({