From 74dcbd8f62651566ed36e55bca3927227919df80 Mon Sep 17 00:00:00 2001
From: Jonathanchang31 <55106972+jonathanchang31@users.noreply.github.com>
Date: Tue, 30 Jun 2026 18:17:33 +0200
Subject: [PATCH 1/3] feat(selfhost): expand sentry observability context
---
.../routes/docs.self-hosting-operations.tsx | 25 +++++
.../src/routes/docs.self-hosting-security.tsx | 7 ++
src/github/app.ts | 20 ++--
src/queue/processors.ts | 28 +++++-
src/selfhost/ai.ts | 18 +++-
src/selfhost/monitored-work.ts | 28 ++++--
src/selfhost/pg-queue.ts | 14 +++
src/selfhost/sentry.ts | 94 +++++++++++++++++--
src/selfhost/sqlite-queue.ts | 14 +++
src/server.ts | 46 ++++++++-
test/unit/selfhost-monitored-work.test.ts | 16 +++-
test/unit/selfhost-sentry.test.ts | 65 ++++++++++---
12 files changed, 328 insertions(+), 47 deletions(-)
diff --git a/apps/gittensory-ui/src/routes/docs.self-hosting-operations.tsx b/apps/gittensory-ui/src/routes/docs.self-hosting-operations.tsx
index 0711d8cfca..6b357c7c01 100644
--- a/apps/gittensory-ui/src/routes/docs.self-hosting-operations.tsx
+++ b/apps/gittensory-ui/src/routes/docs.self-hosting-operations.tsx
@@ -115,6 +115,31 @@ review_context_fetch_failed`}
log for the same subsystem.
+ Sentry alert classes
+
+
+
Routine checks
Queue pending count is not growing without processing.
diff --git a/apps/gittensory-ui/src/routes/docs.self-hosting-security.tsx b/apps/gittensory-ui/src/routes/docs.self-hosting-security.tsx
index f1f96b91c1..49c91ef6ae 100644
--- a/apps/gittensory-ui/src/routes/docs.self-hosting-security.tsx
+++ b/apps/gittensory-ui/src/routes/docs.self-hosting-security.tsx
@@ -90,6 +90,13 @@ function SelfHostingSecurity() {
private scoring context, or maintainer-only notes. For hosted and self-host boundaries, keep
Privacy and security nearby.
+
+ Sentry data boundary
+
+ Self-host Sentry is opt-in only. When enabled, capture should keep tags low-cardinality and
+ scrub request bodies, raw diffs, prompts, review text, auth headers, private repo config,
+ and local auth paths before an event leaves the box.
+
);
}
diff --git a/src/github/app.ts b/src/github/app.ts
index 2913142265..d610eaab52 100644
--- a/src/github/app.ts
+++ b/src/github/app.ts
@@ -220,13 +220,16 @@ async function mintInstallationToken(
);
return cached.token;
}
- console.error(
- JSON.stringify({
- level: "error",
- event: "orb_broker_unavailable",
- installationId,
- error: errorMessage(error),
- }),
+ console.error(
+ JSON.stringify({
+ level: "error",
+ event: "orb_broker_unavailable",
+ subsystem: "github",
+ operation: "broker_installation_token",
+ reasonCode: "orb_broker_unavailable",
+ installationId,
+ error: errorMessage(error),
+ }),
);
throw error;
}
@@ -733,6 +736,9 @@ async function createOrUpdateNamedCheckRun(
JSON.stringify({
level: "error",
event: "check_run_post_denied",
+ subsystem: "github",
+ operation: "check_run_publish",
+ reasonCode: "permission_denied",
repository: `${owner}/${repo}`,
status: e.status ?? null,
message: (e.message ?? "Resource not accessible by integration").slice(
diff --git a/src/queue/processors.ts b/src/queue/processors.ts
index 24492d633d..376452c44b 100644
--- a/src/queue/processors.ts
+++ b/src/queue/processors.ts
@@ -4252,7 +4252,19 @@ async function auditGateCheckPermissionMissing(
});
// Surface the install-wide Checks:write gap to Sentry — until the scope is granted the required gate check-run
// silently never posts on ANY PR for this install; an operator must SEE this config fault, not just the ledger.
- console.error(JSON.stringify({ level: "error", event: "gate_check_permission_missing", message: warning, repository: repoFullName, pullNumber, deliveryId }));
+ console.error(
+ JSON.stringify({
+ level: "error",
+ event: "gate_check_permission_missing",
+ subsystem: "github",
+ operation: "gate_check_publish",
+ reasonCode: "permission_missing",
+ message: warning,
+ repository: repoFullName,
+ pullNumber,
+ deliveryId,
+ }),
+ );
}
/**
@@ -5149,7 +5161,19 @@ async function maybePublishPrPublicSurface(
detail: checkRunResult.warning,
metadata: { deliveryId: webhook.deliveryId, repoFullName },
});
- console.error(JSON.stringify({ level: "error", event: "check_run_permission_missing", message: checkRunResult.warning, repository: repoFullName, pullNumber: pr.number, deliveryId: webhook.deliveryId }));
+ console.error(
+ JSON.stringify({
+ level: "error",
+ event: "check_run_permission_missing",
+ subsystem: "github",
+ operation: "check_run_publish",
+ reasonCode: "permission_missing",
+ message: checkRunResult.warning,
+ repository: repoFullName,
+ pullNumber: pr.number,
+ deliveryId: webhook.deliveryId,
+ }),
+ );
} else if (checkRunResult?.kind === "published") {
publishedOutputs.push("check_run");
}
diff --git a/src/selfhost/ai.ts b/src/selfhost/ai.ts
index 77c67b0068..85e4fdcb48 100644
--- a/src/selfhost/ai.ts
+++ b/src/selfhost/ai.ts
@@ -483,6 +483,9 @@ function logSelfHostAiProviderFailed(input: {
JSON.stringify({
level: "error",
event: "selfhost_ai_provider_failed",
+ subsystem: "ai",
+ operation: "provider_attempt",
+ reasonCode: "provider_error",
provider: input.provider,
model: input.model || "default",
effort: input.effort,
@@ -599,13 +602,26 @@ export function createChainAi(providers: Array<{ name: string; ai: SelfHostAi }>
} catch (error) {
lastError = error;
failures.push({ provider: p.name, error: errorMessage(error) });
- console.error(JSON.stringify({ level: "warn", event: "selfhost_ai_provider_failed_in_chain", provider: p.name, error: errorMessage(error) }));
+ console.error(
+ JSON.stringify({
+ level: "warn",
+ event: "selfhost_ai_provider_failed_in_chain",
+ subsystem: "ai",
+ operation: "provider_chain",
+ reasonCode: "provider_error",
+ provider: p.name,
+ error: errorMessage(error),
+ }),
+ );
}
}
console.error(
JSON.stringify({
level: "error",
event: "selfhost_ai_providers_exhausted",
+ subsystem: "ai",
+ operation: "provider_chain",
+ reasonCode: "providers_exhausted",
provider: failures.length === 1 ? failures[0]?.provider : undefined,
model: model || "default",
providers: failures.map((failure) => failure.provider),
diff --git a/src/selfhost/monitored-work.ts b/src/selfhost/monitored-work.ts
index d8fa250cbe..ce221ba3fe 100644
--- a/src/selfhost/monitored-work.ts
+++ b/src/selfhost/monitored-work.ts
@@ -22,7 +22,12 @@ export async function runScheduledLoopWithMonitor(
): Promise {
return withSentryMonitor(
"scheduled-loop",
- { jobType: "scheduled-loop", cron },
+ {
+ subsystem: "scheduler",
+ operation: "scheduled_loop",
+ jobType: "scheduled-loop",
+ cron,
+ },
() => Promise.resolve(scheduled()),
);
}
@@ -31,11 +36,15 @@ export async function runOrbExportWithMonitor(
exportBatch: () => Promise,
log: (line: string) => void = console.log,
): Promise {
- await withSentryMonitor("orb-export", { jobType: "orb-export" }, async () => {
- const exported = await exportBatch();
- if (exported > 0)
- log(JSON.stringify({ event: "selfhost_orb_export", exported }));
- });
+ await withSentryMonitor(
+ "orb-export",
+ { subsystem: "orb", operation: "orb_export", jobType: "orb-export" },
+ async () => {
+ const exported = await exportBatch();
+ if (exported > 0)
+ log(JSON.stringify({ event: "selfhost_orb_export", exported }));
+ },
+ );
}
export async function drainOrbRelayWithMonitor(args: {
@@ -53,7 +62,12 @@ export async function drainOrbRelayWithMonitor(args: {
}): Promise {
await withSentryMonitor(
"orb-relay-drain",
- { jobType: "orb-relay-drain", pendingAckCount: args.state.pendingAck.length },
+ {
+ subsystem: "orb",
+ operation: "relay_drain",
+ jobType: "orb-relay-drain",
+ pendingAckCount: args.state.pendingAck.length,
+ },
async () => {
const events = await args.drain(args.relayEnv, args.state.pendingAck);
args.state.pendingAck = [];
diff --git a/src/selfhost/pg-queue.ts b/src/selfhost/pg-queue.ts
index 1aa8ee4491..61c96136ad 100644
--- a/src/selfhost/pg-queue.ts
+++ b/src/selfhost/pg-queue.ts
@@ -306,8 +306,11 @@ export function createPgQueue(
}),
);
captureError(new Error("self-host queue processing lease expired"), {
+ subsystem: "queue",
+ operation: "job_claim",
kind: "job_recovered",
reason: "processing_timeout",
+ queueBackend: "postgres",
recovered,
timeoutMs: processingTimeoutMs,
});
@@ -335,8 +338,11 @@ export function createPgQueue(
error: "unparseable payload",
});
captureError(new Error("unparseable queue payload"), {
+ subsystem: "queue",
+ operation: "job_process",
kind: "job_dead",
reason: "unparseable_payload",
+ queueBackend: "postgres",
jobId: job.id,
});
return true;
@@ -437,6 +443,11 @@ export function createPgQueue(
JSON.stringify({
level: "error",
event: "selfhost_job_dead",
+ subsystem: "queue",
+ operation: "job_process",
+ reasonCode: "max_retries_exhausted",
+ backend: "postgres",
+ jobType: extractPayloadType(job.payload),
id: job.id,
attempts,
error: errMsg,
@@ -452,8 +463,11 @@ export function createPgQueue(
error: errMsg,
}, jobTraceParent);
captureError(error, {
+ subsystem: "queue",
+ operation: "job_process",
kind: "job_dead",
reason: "max_retries_exhausted",
+ queueBackend: "postgres",
jobType: extractPayloadType(job.payload),
jobId: job.id,
attempts,
diff --git a/src/selfhost/sentry.ts b/src/selfhost/sentry.ts
index ecd5957e55..f87eb7f5c9 100644
--- a/src/selfhost/sentry.ts
+++ b/src/selfhost/sentry.ts
@@ -17,6 +17,27 @@ let sentryEnvironment = "production";
const SECRET_KEY =
/(token|secret|key|password|passwd|authorization|auth|dsn|cookie|bearer|credential|private)/i;
+const SENSITIVE_CONTENT_KEY =
+ /(raw_?body|request_?body|body|patch|diff|prompt|messages|review(_|)?(text|body)?|comment(_|)?(text|body)?|private_?repo_?config|repo_?config|auth_?path|auth_?file|codex_?home|claude_?code)/i;
+const PATH_KEY = /(path|file|dir|home)$/i;
+const PATH_SECRET_KEY = /(auth|token|secret|private|config|oauth|codex|claude)/i;
+const REDACTED = "[redacted]";
+const MAX_TAG_LENGTH = 120;
+const SENTRY_CONTEXT_TAGS = [
+ ["subsystem", ["subsystem"]],
+ ["operation", ["operation"]],
+ ["reasonCode", ["reasonCode", "reason"]],
+ ["jobType", ["jobType"]],
+ ["backend", ["backend", "queueBackend"]],
+ ["repo", ["repo", "repository"]],
+ ["pullNumber", ["pullNumber", "pull", "pr"]],
+ ["provider", ["provider"]],
+ ["model", ["model"]],
+ ["effort", ["effort"]],
+ ["monitor", ["monitor"]],
+ ["mode", ["mode"]],
+ ["kind", ["kind"]],
+] as const;
function nonBlank(value: string | undefined): string | undefined {
const trimmed = value?.trim();
@@ -89,6 +110,29 @@ function safeMonitorContext(
return safe;
}
+function tagValue(value: unknown): string | undefined {
+ if (typeof value === "string") {
+ const trimmed = value.trim();
+ return trimmed ? trimmed.slice(0, MAX_TAG_LENGTH) : undefined;
+ }
+ if (typeof value === "number" && Number.isFinite(value)) return String(value);
+ return undefined;
+}
+
+function setSafeContextTags(
+ scope: SentryScope,
+ context: Record,
+): void {
+ for (const [tag, aliases] of SENTRY_CONTEXT_TAGS) {
+ for (const alias of aliases) {
+ const value = tagValue(context[alias]);
+ if (!value) continue;
+ scope.setTag(tag, value);
+ break;
+ }
+ }
+}
+
function setOtelTraceScope(scope: SentryScope): void {
const trace = currentOtelTraceIds();
if (!trace) return;
@@ -110,17 +154,26 @@ export function scrubEvent(event: T): T {
if (!obj || typeof obj !== "object" || depth > 6) return;
for (const key of Object.keys(obj as Record)) {
const rec = obj as Record;
- if (SECRET_KEY.test(key)) rec[key] = "[redacted]";
- else if (typeof rec[key] === "object") redact(rec[key], depth + 1);
+ const value = rec[key];
+ if (
+ SECRET_KEY.test(key) ||
+ SENSITIVE_CONTENT_KEY.test(key) ||
+ (PATH_KEY.test(key) && PATH_SECRET_KEY.test(key))
+ ) {
+ rec[key] = REDACTED;
+ } else if (typeof value === "object") {
+ redact(value, depth + 1);
+ }
}
};
try {
const e = event as {
- request?: { headers?: unknown };
+ request?: { headers?: unknown; body?: unknown };
contexts?: unknown;
extra?: unknown;
};
redact(e.request?.headers, 0);
+ redact(e.request, 0);
redact(e.contexts, 0);
redact(e.extra, 0);
} catch {
@@ -155,7 +208,10 @@ export function captureError(
if (!active || !Sentry) return;
Sentry.withScope((scope) => {
setOtelTraceScope(scope);
- if (context) scope.setContext("gittensory", context);
+ if (context) {
+ scope.setContext("gittensory", context);
+ setSafeContextTags(scope, context);
+ }
Sentry!.captureException(
error instanceof Error ? error : new Error(String(error)),
);
@@ -174,11 +230,7 @@ export function captureReviewFailure(
setOtelTraceScope(scope);
if (context) {
scope.setContext("review", context);
- for (const tag of ["owner", "repo", "pr", "head_sha"]) {
- const value = context[tag];
- if (value !== undefined && value !== null)
- scope.setTag(tag, String(value));
- }
+ setSafeContextTags(scope, context);
}
Sentry!.captureException(
error instanceof Error ? error : new Error(String(error)),
@@ -188,7 +240,29 @@ export function captureReviewFailure(
// The structured-log fields worth indexing as Sentry tags — the dimensions operators filter + group by. Only
// string|number values are tagged; everything else stays in the full "log" context.
-const SENTRY_LOG_TAG_KEYS = ["repo", "repository", "installationId", "installation_id", "pull", "pullNumber", "pr", "project", "kind", "deliveryId", "provider", "model", "effort", "timeoutMs", "trace_id", "span_id"] as const;
+const SENTRY_LOG_TAG_KEYS = [
+ "event",
+ "subsystem",
+ "operation",
+ "reasonCode",
+ "repo",
+ "repository",
+ "pull",
+ "pullNumber",
+ "pr",
+ "project",
+ "kind",
+ "jobType",
+ "backend",
+ "provider",
+ "model",
+ "effort",
+ "mode",
+ "monitor",
+ "timeoutMs",
+ "trace_id",
+ "span_id",
+] as const;
/** A SHORT location suffix — " (repo#pr)" — for a no-message error title, so the issue list shows WHERE without
* dumping every scalar field (which made titles unreadably long, e.g. trailing a full deliveryId). The complete
diff --git a/src/selfhost/sqlite-queue.ts b/src/selfhost/sqlite-queue.ts
index 0a20dcb699..9bbb11dd1d 100644
--- a/src/selfhost/sqlite-queue.ts
+++ b/src/selfhost/sqlite-queue.ts
@@ -249,8 +249,11 @@ export function createSqliteQueue(
}),
);
captureError(new Error("self-host queue processing lease expired"), {
+ subsystem: "queue",
+ operation: "job_claim",
kind: "job_recovered",
reason: "processing_timeout",
+ queueBackend: "sqlite",
recovered,
timeoutMs: processingTimeoutMs,
});
@@ -278,8 +281,11 @@ export function createSqliteQueue(
error: "unparseable payload",
});
captureError(new Error("unparseable queue payload"), {
+ subsystem: "queue",
+ operation: "job_process",
kind: "job_dead",
reason: "unparseable_payload",
+ queueBackend: "sqlite",
jobId: job.id,
});
return true;
@@ -380,6 +386,11 @@ export function createSqliteQueue(
JSON.stringify({
level: "error",
event: "selfhost_job_dead",
+ subsystem: "queue",
+ operation: "job_process",
+ reasonCode: "max_retries_exhausted",
+ backend: "sqlite",
+ jobType: extractPayloadType(job.payload),
id: job.id,
attempts,
error: errMsg,
@@ -395,8 +406,11 @@ export function createSqliteQueue(
error: errMsg,
}, jobTraceParent);
captureError(error, {
+ subsystem: "queue",
+ operation: "job_process",
kind: "job_dead",
reason: "max_retries_exhausted",
+ queueBackend: "sqlite",
jobType: extractPayloadType(job.payload),
jobId: job.id,
attempts,
diff --git a/src/server.ts b/src/server.ts
index 2512655291..92dc7c483b 100644
--- a/src/server.ts
+++ b/src/server.ts
@@ -278,12 +278,22 @@ async function main(): Promise {
}),
);
process.on("uncaughtException", (error) => {
- captureError(error, { kind: "uncaughtException" });
+ captureError(error, {
+ subsystem: "runtime",
+ operation: "process_exception",
+ kind: "uncaughtException",
+ reason: "uncaught_exception",
+ });
console.error(error);
void flushSentry().finally(() => process.exit(1));
});
process.on("unhandledRejection", (reason) => {
- captureError(reason, { kind: "unhandledRejection" });
+ captureError(reason, {
+ subsystem: "runtime",
+ operation: "process_rejection",
+ kind: "unhandledRejection",
+ reason: "unhandled_rejection",
+ });
console.error(reason);
});
// Central error forwarding (#1468): operational failures are structured JSON logs emitted through stdout and
@@ -760,6 +770,9 @@ async function main(): Promise {
JSON.stringify({
level: "error",
event: "selfhost_cron_error",
+ subsystem: "scheduler",
+ operation: "scheduled_loop",
+ reasonCode: "scheduled_loop_failed",
error: error instanceof Error ? error.message : "unknown error",
}),
),
@@ -776,6 +789,9 @@ async function main(): Promise {
JSON.stringify({
level: "error",
event: "selfhost_orb_export_error",
+ subsystem: "orb",
+ operation: "orb_export",
+ reasonCode: "orb_export_failed",
error: error instanceof Error ? error.message : "unknown error",
}),
),
@@ -806,13 +822,23 @@ async function main(): Promise {
JSON.stringify({
level: pull ? "warn" : "error",
event: "selfhost_orb_relay_register_failed",
+ subsystem: "orb",
+ operation: "relay_register",
+ reasonCode: "relay_register_failed",
mode: pull ? "pull" : "push",
error: r.reason ?? "unknown",
}),
);
}
})
- .catch((error) => captureError(error, { kind: "orb_relay_register" }));
+ .catch((error) =>
+ captureError(error, {
+ subsystem: "orb",
+ operation: "relay_register",
+ kind: "orb_relay_register",
+ reason: "relay_register_exception",
+ }),
+ );
// Pull-mode relay drain (#secure-relay): when ORB_RELAY_MODE=pull, the engine DRAINS its events from the Orb on a
// timer instead of exposing an inbound endpoint — the right fit behind NAT/tailnet. Acks the previous batch so the
@@ -839,7 +865,12 @@ async function main(): Promise {
setInterval(
() =>
void drainRelay().catch((error) =>
- captureError(error, { kind: "orb_relay_drain" }),
+ captureError(error, {
+ subsystem: "orb",
+ operation: "relay_drain",
+ kind: "orb_relay_drain",
+ reason: "relay_drain_exception",
+ }),
),
15_000,
);
@@ -865,7 +896,12 @@ async function main(): Promise {
}
main().catch((error) => {
- captureError(error, { kind: "boot" });
+ captureError(error, {
+ subsystem: "runtime",
+ operation: "boot",
+ kind: "boot",
+ reason: "boot_failure",
+ });
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/test/unit/selfhost-monitored-work.test.ts b/test/unit/selfhost-monitored-work.test.ts
index ef03e4d901..457ceef08c 100644
--- a/test/unit/selfhost-monitored-work.test.ts
+++ b/test/unit/selfhost-monitored-work.test.ts
@@ -32,7 +32,12 @@ describe("self-host monitored recurring work", () => {
expect(mocks.withSentryMonitor).toHaveBeenCalledWith(
"scheduled-loop",
- { jobType: "scheduled-loop", cron: "*/2 * * * *" },
+ {
+ subsystem: "scheduler",
+ operation: "scheduled_loop",
+ jobType: "scheduled-loop",
+ cron: "*/2 * * * *",
+ },
expect.any(Function),
);
expect(scheduled).toHaveBeenCalledTimes(1);
@@ -45,7 +50,7 @@ describe("self-host monitored recurring work", () => {
await runOrbExportWithMonitor(exportBatch, log);
expect(mocks.withSentryMonitor).toHaveBeenLastCalledWith(
"orb-export",
- { jobType: "orb-export" },
+ { subsystem: "orb", operation: "orb_export", jobType: "orb-export" },
expect.any(Function),
);
expect(log).toHaveBeenCalledWith(
@@ -112,7 +117,12 @@ describe("self-host monitored recurring work", () => {
expect(mocks.withSentryMonitor).toHaveBeenCalledWith(
"orb-relay-drain",
- { jobType: "orb-relay-drain", pendingAckCount: 1 },
+ {
+ subsystem: "orb",
+ operation: "relay_drain",
+ jobType: "orb-relay-drain",
+ pendingAckCount: 1,
+ },
expect.any(Function),
);
expect(drain).toHaveBeenCalledWith(relayEnv, ["previous-delivery"]);
diff --git a/test/unit/selfhost-sentry.test.ts b/test/unit/selfhost-sentry.test.ts
index a027241de5..ea12f8e8c6 100644
--- a/test/unit/selfhost-sentry.test.ts
+++ b/test/unit/selfhost-sentry.test.ts
@@ -56,22 +56,32 @@ const lastCapturedError = (): Error =>
describe("scrubEvent — redact secrets before an event leaves the box", () => {
it("redacts secret-keyed fields in headers/contexts/extra, recurses, and leaves safe fields", () => {
const ev = scrubEvent({
- request: { headers: { authorization: "Bearer abc", "x-trace": "ok" } },
+ request: {
+ headers: { authorization: "Bearer abc", "x-trace": "ok" },
+ body: "{\"private\":true}",
+ },
contexts: {
gittensory: {
jobId: "j1",
apiKey: "shh",
nested: { secretToken: "deep" },
+ prompt: "private prompt",
+ localAuthPath: "/srv/auth.json",
},
},
- extra: { note: "fine" },
+ extra: { note: "fine", reviewText: "secret review", privateRepoConfig: "x" },
}) as any;
expect(ev.request.headers.authorization).toBe("[redacted]");
expect(ev.request.headers["x-trace"]).toBe("ok");
+ expect(ev.request.body).toBe("[redacted]");
expect(ev.contexts.gittensory.apiKey).toBe("[redacted]");
expect(ev.contexts.gittensory.jobId).toBe("j1");
expect(ev.contexts.gittensory.nested.secretToken).toBe("[redacted]");
+ expect(ev.contexts.gittensory.prompt).toBe("[redacted]");
+ expect(ev.contexts.gittensory.localAuthPath).toBe("[redacted]");
expect(ev.extra.note).toBe("fine");
+ expect(ev.extra.reviewText).toBe("[redacted]");
+ expect(ev.extra.privateRepoConfig).toBe("[redacted]");
});
it("is safe when headers/contexts/extra are absent (the !obj branch)", () => {
@@ -181,12 +191,35 @@ describe("enabled when SENTRY_DSN is set", () => {
).toBe("custom@sha");
});
- it("captureError sends with context, and without context skips setContext", async () => {
+ it("captureError sends with safe context tags, and without context skips setContext", async () => {
await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv);
- captureError(new Error("boom"), { kind: "job_dead" });
+ captureError(new Error("boom"), {
+ subsystem: "queue",
+ operation: "job_process",
+ kind: "job_dead",
+ reason: "max_retries_exhausted",
+ repo: "o/r",
+ pullNumber: 7,
+ installationId: 143010787,
+ deliveryId: "delivery-1",
+ });
expect(mocks.scope.setContext).toHaveBeenCalledWith("gittensory", {
+ subsystem: "queue",
+ operation: "job_process",
kind: "job_dead",
+ reason: "max_retries_exhausted",
+ repo: "o/r",
+ pullNumber: 7,
+ installationId: 143010787,
+ deliveryId: "delivery-1",
});
+ expect(mocks.scope.setTag).toHaveBeenCalledWith("subsystem", "queue");
+ expect(mocks.scope.setTag).toHaveBeenCalledWith("operation", "job_process");
+ expect(mocks.scope.setTag).toHaveBeenCalledWith("reasonCode", "max_retries_exhausted");
+ expect(mocks.scope.setTag).toHaveBeenCalledWith("repo", "o/r");
+ expect(mocks.scope.setTag).toHaveBeenCalledWith("pullNumber", "7");
+ expect(mocks.scope.setTag).not.toHaveBeenCalledWith("installationId", "143010787");
+ expect(mocks.scope.setTag).not.toHaveBeenCalledWith("deliveryId", "delivery-1");
expect(mocks.captureException).toHaveBeenCalledTimes(1);
mocks.scope.setContext.mockClear();
captureError("plain string with no context");
@@ -194,7 +227,7 @@ describe("enabled when SENTRY_DSN is set", () => {
expect(mocks.captureException).toHaveBeenCalledTimes(2);
});
- it("captureReviewFailure sets error level + repo/PR/SHA tags, skipping null/undefined, and works without context", async () => {
+ it("captureReviewFailure sets error level + safe repo/PR tags, skipping high-cardinality tags, and works without context", async () => {
await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv);
captureReviewFailure(new Error("rev"), {
repo: "o/r",
@@ -204,12 +237,8 @@ describe("enabled when SENTRY_DSN is set", () => {
});
expect(mocks.scope.setLevel).toHaveBeenCalledWith("error");
expect(mocks.scope.setTag).toHaveBeenCalledWith("repo", "o/r");
- expect(mocks.scope.setTag).toHaveBeenCalledWith("pr", "7");
- expect(mocks.scope.setTag).toHaveBeenCalledWith("head_sha", "abc");
- expect(mocks.scope.setTag).not.toHaveBeenCalledWith(
- "owner",
- expect.anything(),
- );
+ expect(mocks.scope.setTag).toHaveBeenCalledWith("pullNumber", "7");
+ expect(mocks.scope.setTag).not.toHaveBeenCalledWith("head_sha", "abc");
captureReviewFailure("string failure, no context");
expect(mocks.captureException).toHaveBeenCalledTimes(2);
});
@@ -438,6 +467,9 @@ describe("forwardStructuredLogToSentry — central console.log → Sentry error
JSON.stringify({
level: "error",
event: "orb_broker_unavailable",
+ subsystem: "github",
+ operation: "broker_installation_token",
+ reasonCode: "orb_broker_unavailable",
error: "The operation was aborted due to timeout",
repo: "JSONbored/gittensory",
installationId: 143010787,
@@ -447,8 +479,17 @@ describe("forwardStructuredLogToSentry — central console.log → Sentry error
expect(lastCapturedError().name).toBe("orb_broker_unavailable");
expect(lastCapturedError().message).toBe("The operation was aborted due to timeout");
// The present log dimensions become filterable tags.
+ expect(mocks.scope.setTag).toHaveBeenCalledWith("subsystem", "github");
+ expect(mocks.scope.setTag).toHaveBeenCalledWith(
+ "operation",
+ "broker_installation_token",
+ );
+ expect(mocks.scope.setTag).toHaveBeenCalledWith(
+ "reasonCode",
+ "orb_broker_unavailable",
+ );
expect(mocks.scope.setTag).toHaveBeenCalledWith("repo", "JSONbored/gittensory");
- expect(mocks.scope.setTag).toHaveBeenCalledWith("installationId", "143010787");
+ expect(mocks.scope.setTag).not.toHaveBeenCalledWith("installationId", "143010787");
// Recurrences of one failure group into a single issue by event.
expect(mocks.scope.setFingerprint).toHaveBeenCalledWith(["gittensory-log", "orb_broker_unavailable"]);
});
From 6ddbb875800426fecfecee5cc8c3cc5688de2a78 Mon Sep 17 00:00:00 2001
From: Jonathanchang31 <55106972+jonathanchang31@users.noreply.github.com>
Date: Tue, 30 Jun 2026 19:11:49 +0200
Subject: [PATCH 2/3] fix(github): settle coalesced GET failures
---
src/github/client.ts | 90 ++++++++++++++++++++++++++++++++------------
1 file changed, 66 insertions(+), 24 deletions(-)
diff --git a/src/github/client.ts b/src/github/client.ts
index b7dc0f4955..86eb02bf67 100644
--- a/src/github/client.ts
+++ b/src/github/client.ts
@@ -179,20 +179,27 @@ function responseFromCached(hit: CachedGitHubResponse, replayKind: "hit" | "coal
});
}
-async function fetchWithGitHubRetry(input: RequestInfo | URL, init?: RequestInit): Promise {
+async function fetchWithGitHubRetry(
+ input: RequestInfo | URL,
+ init?: RequestInit,
+): Promise {
let response: Response;
for (let attempt = 0; ; attempt += 1) {
- response = init?.signal
- ? await fetch(input, init)
- : await fetch(input, {
- ...(init ?? {}),
- signal: AbortSignal.timeout(GITHUB_FETCH_TIMEOUT_MS),
- });
+ try {
+ response = init?.signal
+ ? await fetch(input, init)
+ : await fetch(input, {
+ ...(init ?? {}),
+ signal: AbortSignal.timeout(GITHUB_FETCH_TIMEOUT_MS),
+ });
+ } catch (error) {
+ return { ok: false, error };
+ }
// Retry a transient rate-limit (with backoff) instead of surfacing it; stop once exhausted or it's not a limit.
if (attempt >= GITHUB_RATE_LIMIT_MAX_RETRIES || !(await isRateLimitedResponse(response))) break;
await sleep(rateLimitRetryMs(response, attempt));
}
- return response;
+ return { ok: true, response };
}
async function fetchAndMaybeCacheGitHubGet(
@@ -201,9 +208,11 @@ async function fetchAndMaybeCacheGitHubGet(
url: string,
cacheKey: string,
cls: GitHubCacheClass,
-): Promise<{ response: Response; cached: CachedGitHubResponse | null }> {
- const response = await fetchWithGitHubRetry(input, init);
- if (response.status !== 200) return { response, cached: null };
+): Promise {
+ const fetched = await fetchWithGitHubRetry(input, init);
+ if (!fetched.ok) return { ok: false, error: fetched.error };
+ const { response } = fetched;
+ if (response.status !== 200) return { ok: true, response, cached: null };
try {
const cached = {
status: 200,
@@ -215,16 +224,37 @@ async function fetchAndMaybeCacheGitHubGet(
};
await responseCache!.set(cacheKey, cached, githubResponseCacheTtlSeconds(cls));
recordGitHubCacheMetric("set", cls);
- return { response, cached };
+ return { ok: true, response, cached };
} catch {
recordGitHubCacheMetric("error", cls);
- return { response, cached: null };
+ return { ok: true, response, cached: null };
}
}
+type InFlightCacheableGet =
+ | {
+ ok: true;
+ response: Response;
+ cached: CachedGitHubResponse | null;
+ }
+ | {
+ ok: false;
+ error: unknown;
+ };
+
+type GitHubFetchResult =
+ | {
+ ok: true;
+ response: Response;
+ }
+ | {
+ ok: false;
+ error: unknown;
+ };
+
// Single-flight cacheable GETs inside one isolate: a webhook burst often asks for the same metadata
// before Redis has been populated. Join those cold misses so GitHub sees one request, then replay the cached body.
-const inFlightCacheableGets = new Map>();
+const inFlightCacheableGets = new Map>();
// A 12s hard cap on every GitHub request. Centralised here so the app token/installation raw fetches plus comment /
// label / check-run / pr-action Octokit helpers all inherit the cache boundary, retry, and timeout behavior.
@@ -237,7 +267,9 @@ export async function timeoutFetch(input: RequestInfo | URL, init?: RequestInit)
const useCache = responseCache !== null && cls !== null;
if (!useCache) {
recordGitHubCacheMetric("bypassed", cacheBypassClass(method, url, headers));
- return fetchWithGitHubRetry(input, init);
+ const fetched = await fetchWithGitHubRetry(input, init);
+ if (!fetched.ok) throw fetched.error;
+ return fetched.response;
}
const cacheKey = await responseCacheKey(url, headers);
@@ -257,19 +289,29 @@ export async function timeoutFetch(input: RequestInfo | URL, init?: RequestInit)
if (existing) {
recordGitHubCacheMetric("coalesced", cls);
const replay = await existing;
- if (replay) return responseFromCached(replay, "coalesced");
+ if (replay.ok && replay.cached)
+ return responseFromCached(replay.cached, "coalesced");
}
- const request = fetchAndMaybeCacheGitHubGet(input, init, url, cacheKey, cls).then(
- (result) => ({ ok: true as const, result }),
- (error: unknown) => ({ ok: false as const, error }),
- );
- const shared = request.then((settled) => (settled.ok ? settled.result.cached : null));
- const sharedWithCleanup = shared.finally(() => inFlightCacheableGets.delete(cacheKey));
- inFlightCacheableGets.set(cacheKey, sharedWithCleanup);
+ const request = (async (): Promise => {
+ try {
+ return await fetchAndMaybeCacheGitHubGet(
+ input,
+ init,
+ url,
+ cacheKey,
+ cls,
+ );
+ } catch (error) {
+ return { ok: false, error };
+ } finally {
+ inFlightCacheableGets.delete(cacheKey);
+ }
+ })();
+ inFlightCacheableGets.set(cacheKey, request);
const result = await request;
if (!result.ok) throw result.error;
- return result.result.response;
+ return result.response;
}
/** Test-only: reset shared GitHub response cache state between tests. */
From b4b0cc6140216c8f7007f405f47f11d96db74463 Mon Sep 17 00:00:00 2001
From: Jonathanchang31 <55106972+jonathanchang31@users.noreply.github.com>
Date: Tue, 30 Jun 2026 19:33:59 +0200
Subject: [PATCH 3/3] fix(github): preserve shared GET failures
---
src/github/app.ts | 20 ++++++++++----------
src/github/client.ts | 1 +
test/unit/github-client.test.ts | 19 +++++++------------
3 files changed, 18 insertions(+), 22 deletions(-)
diff --git a/src/github/app.ts b/src/github/app.ts
index d610eaab52..3b80f5618b 100644
--- a/src/github/app.ts
+++ b/src/github/app.ts
@@ -220,16 +220,16 @@ async function mintInstallationToken(
);
return cached.token;
}
- console.error(
- JSON.stringify({
- level: "error",
- event: "orb_broker_unavailable",
- subsystem: "github",
- operation: "broker_installation_token",
- reasonCode: "orb_broker_unavailable",
- installationId,
- error: errorMessage(error),
- }),
+ console.error(
+ JSON.stringify({
+ level: "error",
+ event: "orb_broker_unavailable",
+ subsystem: "github",
+ operation: "broker_installation_token",
+ reasonCode: "orb_broker_unavailable",
+ installationId,
+ error: errorMessage(error),
+ }),
);
throw error;
}
diff --git a/src/github/client.ts b/src/github/client.ts
index 86eb02bf67..dfc4f67265 100644
--- a/src/github/client.ts
+++ b/src/github/client.ts
@@ -289,6 +289,7 @@ export async function timeoutFetch(input: RequestInfo | URL, init?: RequestInit)
if (existing) {
recordGitHubCacheMetric("coalesced", cls);
const replay = await existing;
+ if (!replay.ok) throw replay.error;
if (replay.ok && replay.cached)
return responseFromCached(replay.cached, "coalesced");
}
diff --git a/test/unit/github-client.test.ts b/test/unit/github-client.test.ts
index 6a757e7106..e43f6dcfdb 100644
--- a/test/unit/github-client.test.ts
+++ b/test/unit/github-client.test.ts
@@ -511,7 +511,7 @@ describe("timeoutFetch", () => {
expect(getFetches).toBe(2);
});
- it("also falls back when the shared in-flight GET leader throws before a response exists", async () => {
+ it("coalesces the same thrown failure when the shared in-flight GET leader errors before a response exists", async () => {
let cacheReads = 0;
let resolveBothCacheReads!: () => void;
const bothCacheReads = new Promise((resolve) => {
@@ -533,26 +533,21 @@ describe("timeoutFetch", () => {
vi.stubGlobal("fetch", async (input: RequestInfo | URL) => {
if (String(input).includes("/repos/o/r/branches/main/protection/required_status_checks")) {
getFetches += 1;
- if (getFetches === 1) {
- await fetchGate;
- throw new Error("network down");
- }
- return Response.json({ contexts: ["after-throw"] });
+ await fetchGate;
+ throw new Error("network down");
}
return new Response("not found", { status: 404 });
});
const url = "https://api.github.com/repos/o/r/branches/main/protection/required_status_checks";
- const firstRequest = timeoutFetch(url);
- void firstRequest.catch(() => undefined);
- const first = firstRequest.catch((error: Error) => error.message);
- const second = timeoutFetch(url);
+ const first = timeoutFetch(url).catch((error: Error) => error.message);
+ const second = timeoutFetch(url).catch((error: Error) => error.message);
await bothCacheReads;
releaseFetch();
await expect(first).resolves.toContain("network down");
- await expect(second.then((response) => response.json())).resolves.toEqual({ contexts: ["after-throw"] });
- expect(getFetches).toBe(2);
+ await expect(second).resolves.toContain("network down");
+ expect(getFetches).toBe(1);
});
it("fails open when the shared response cache throws on read or write", async () => {