mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 01:57:31 +01:00
fix(server): split run-failure Sentry groups that have no usable root code (#19308)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server sends each unexpected terminal run failure to Sentry. A custom fingerprint groups these events. > - The fingerprint is `[errorCode, agentAdapter, failurePhase?, rootErrorCode?]`. The root code is used only when it is an identifier. > - Many setup failures have no identifier root code. Some have a numeric JSON-RPC code (`-32002`). Some have only an HTTP status (`409`). Some have no code. > - All of these failures get the same fingerprint, `setup_failed / paperclip_runner / setup`. One Sentry group now holds about ten unrelated failure types: a controller lease loss, Daytona create timeouts, sandbox resume timeouts, remote-workspace 409 errors, S3 throttling and more. The group title changes with each new event. A fix for one cause cannot close the group. > - This pull request adds a bounded fallback when there is no usable root code. The fallback is the root exception class name plus its HTTP status or numeric code. > - The benefit is that each failure type gets its own Sentry group, so a fix can be verified and resolved for each cause. ## Linked Issues or Issue Description Refs #16359 Refs #16802 Refs #19156 **What happened?** Unrelated `paperclip_runner` setup failures go into one Sentry issue. The root exceptions include `LegacyControllerLeaseLostError`, `JsonRpcCallError` with code `-32002` (Daytona create timeout), `JsonRpcCallError` with code `-32003` (sandbox resume timeout), plain `Error` with status `409` (remote workspace writer still pending), and AWS `SlowDown`. None of them has an identifier root code, so the fingerprint drops the root element. **Expected behavior** Failures with different root exceptions go into different Sentry issues. Failures with the same root exception and status still group together. The fingerprint does not contain message text. **Steps to reproduce** 1. Report two setup failures for the `paperclip_runner` adapter with error code `setup_failed`. 2. Give the first failure the root exception `LegacyControllerLeaseLostError` with no code. 3. Give the second failure the root exception `JsonRpcCallError` with code `-32002`. 4. Read the two fingerprints. Before this change, they are the same. **Additional context** Only events without an identifier root code get a new fingerprint. Events with an identifier root code (for example `ECONNRESET`) keep their current fingerprint and their current Sentry group. ## What Changed - `server/src/sentry.ts`: when there is no identifier root code, `captureRunFailure` adds fallback fingerprint elements from the deepest recorded exception: - `exception:<Name>` when the name is an identifier. - `status:<n>` when the exception has an HTTP status (already validated as 100–599 by the diagnostics collector), or else `code:<n>` when the code is a number of up to 10 digits. - Names or codes with other shapes are not used, so message text and secrets cannot go into the fingerprint. - The `root_error_code` tag does not change. - `server/src/__tests__/run-failure-sentry.test.ts`: new tests for the fallback groups, for an identifier root code that keeps the current fingerprint, and for rejected unbounded names and codes. ## Verification - `cd server && npx vitest run src/__tests__/run-failure-sentry.test.ts src/__tests__/run-failure-sentry-real-sdk.test.ts src/__tests__/sentry-links.test.ts src/services/__tests__/run-failure-report.test.ts`: 358 tests pass. The real-SDK suite ran against `@sentry/node@10.71.0`. - I reverted only `server/src/sentry.ts` and ran the new grouping test. It failed, because all six fingerprints were the same. With the change, it passes. - `cd server && npx tsc --noEmit -p tsconfig.json`: no errors. - `src/__tests__/sentry.test.ts` did not load in my local environment (`AI_InvalidArgumentError: baseURL must be a non-empty string`). It fails in the same way on unchanged `master`, so I depend on CI for that suite. ## Risks - Events without an identifier root code get a new fingerprint. Sentry puts them in new issues. The old mixed issues stop getting these events. This is the intended result. Alerts or saved searches that point to an old mixed issue must move to the new issues. - The number of groups increases by a small, bounded amount. Exception class names, HTTP statuses and numeric codes have low cardinality. No message, ID or free text goes into the fingerprint. - No change to what the server sends in tags or contexts, and no change to run control flow. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edits, test runs, GitHub and Sentry connectors). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
d361f92071
commit
5d2fb8f737
2 files changed
+70
-1
No files matched your search
@@ -190,6 +190,60 @@ describe("captureRunFailure", () => {
|
||||
expect(captureException.mock.calls[1]![1]?.fingerprint).toEqual(["timeout", "codex"]);
|
||||
});
|
||||
|
||||
it("separates setup failures without a usable root code by the root exception", async () => {
|
||||
const { sentryModule, captureException } = await importFreshSentryWithGateOpen();
|
||||
const setupFailure = (exceptions: NonNullable<RunFailureEvent["diagnostics"]>["exceptions"]) => baseEvent({
|
||||
errorCode: "setup_failed", agentAdapter: "paperclip_runner",
|
||||
diagnostics: { execution: { failurePhase: "setup" }, adapter: {}, provider: {}, exceptions, truncatedFields: [] },
|
||||
});
|
||||
|
||||
sentryModule.captureRunFailure(setupFailure([{ name: "LegacyControllerLeaseLostError", message: "Legacy controller lease lost" }]));
|
||||
sentryModule.captureRunFailure(setupFailure([{ name: "Error", message: "The workspace still has an unfinished writer", status: 409 }]));
|
||||
sentryModule.captureRunFailure(setupFailure([{ name: "Error", message: "Workspace ownership changed", status: 409 }]));
|
||||
sentryModule.captureRunFailure(setupFailure([
|
||||
{ name: "EnvironmentRunError", message: "Failed to acquire lease", code: "lease_acquire_failed" },
|
||||
{ name: "JsonRpcCallError", message: "Daytona lease acquisition timed out", code: "-32002" },
|
||||
]));
|
||||
sentryModule.captureRunFailure(setupFailure([{ name: "JsonRpcCallError", message: "RPC call timed out", code: "-32003" }]));
|
||||
sentryModule.captureRunFailure(setupFailure([{ name: "Error", message: "plain failure" }]));
|
||||
|
||||
const fingerprints = captureException.mock.calls.map(([, context]) => context?.fingerprint);
|
||||
const prefix = ["setup_failed", "paperclip_runner", "setup"];
|
||||
expect(fingerprints).toEqual([
|
||||
[...prefix, "exception:LegacyControllerLeaseLostError"],
|
||||
[...prefix, "exception:Error", "status:409"],
|
||||
[...prefix, "exception:Error", "status:409"],
|
||||
[...prefix, "exception:JsonRpcCallError", "code:-32002"],
|
||||
[...prefix, "exception:JsonRpcCallError", "code:-32003"],
|
||||
[...prefix, "exception:Error"],
|
||||
]);
|
||||
expect(JSON.stringify(fingerprints)).not.toMatch(/workspace|timed out|plain failure/i);
|
||||
});
|
||||
|
||||
it("keeps an identifier root code as the only root grouping element", async () => {
|
||||
const { sentryModule, captureException } = await importFreshSentryWithGateOpen();
|
||||
|
||||
sentryModule.captureRunFailure(baseEvent({
|
||||
errorCode: "setup_failed", agentAdapter: "paperclip_runner",
|
||||
diagnostics: { execution: { failurePhase: "setup" }, adapter: {}, provider: {},
|
||||
exceptions: [{ name: "Error", code: "ECONNRESET", status: 503 }], truncatedFields: [] },
|
||||
}));
|
||||
|
||||
expect(captureException.mock.calls[0]![1]?.fingerprint).toEqual(["setup_failed", "paperclip_runner", "setup", "ECONNRESET"]);
|
||||
});
|
||||
|
||||
it("drops unbounded root exception names and codes from the fallback", async () => {
|
||||
const { sentryModule, captureException } = await importFreshSentryWithGateOpen();
|
||||
|
||||
sentryModule.captureRunFailure(baseEvent({
|
||||
errorCode: "setup_failed", agentAdapter: "paperclip_runner",
|
||||
diagnostics: { execution: { failurePhase: "setup" }, adapter: {}, provider: {},
|
||||
exceptions: [{ name: "Error: token=secret value", code: "not a code 123" }], truncatedFields: [] },
|
||||
}));
|
||||
|
||||
expect(captureException.mock.calls[0]![1]?.fingerprint).toEqual(["setup_failed", "paperclip_runner", "setup"]);
|
||||
});
|
||||
|
||||
it("sets the fingerprint element \"unknown\" when the error code is absent", async () => {
|
||||
const { sentryModule, captureException } = await importFreshSentryWithGateOpen();
|
||||
|
||||
|
||||
+16
-1
@@ -229,6 +229,21 @@ function captureRunFailureEvent(event: RunFailureEvent): string | null {
|
||||
diagnostics?.exceptions.at(-1)?.code;
|
||||
const rootErrorCode = typeof rootCodeCandidate === "string" && /^[a-zA-Z][a-zA-Z0-9_]{1,99}$/.test(rootCodeCandidate)
|
||||
? rootCodeCandidate : undefined;
|
||||
// Without a usable root code, unrelated failures that share the error code,
|
||||
// adapter and phase would collapse into one group. Fall back to the root
|
||||
// exception's class name plus its HTTP status or numeric code. These are
|
||||
// bounded values; the message stays out of the fingerprint.
|
||||
const rootFallback: string[] = [];
|
||||
const rootException = diagnostics?.exceptions.at(-1);
|
||||
if (!rootErrorCode && rootException) {
|
||||
if (typeof rootException.name === "string" && /^[A-Za-z_$][A-Za-z0-9_$]{0,99}$/.test(rootException.name)) {
|
||||
rootFallback.push(`exception:${rootException.name}`);
|
||||
}
|
||||
if (rootException.status !== undefined) rootFallback.push(`status:${rootException.status}`);
|
||||
else if (typeof rootException.code === "string" && /^-?[0-9]{1,10}$/.test(rootException.code)) {
|
||||
rootFallback.push(`code:${rootException.code}`);
|
||||
}
|
||||
}
|
||||
const phase = diagnostics?.execution.failurePhase;
|
||||
const failurePhase = phase === "setup" || phase === "execute" ? phase : undefined;
|
||||
const contexts: Record<string, Record<string, unknown>> = {};
|
||||
@@ -303,7 +318,7 @@ function captureRunFailureEvent(event: RunFailureEvent): string | null {
|
||||
signal: normalizeRunSignal(event.signal),
|
||||
},
|
||||
},
|
||||
fingerprint: [errorCode, event.agentAdapter, ...(failurePhase ? [failurePhase] : []), ...(rootErrorCode ? [rootErrorCode] : [])],
|
||||
fingerprint: [errorCode, event.agentAdapter, ...(failurePhase ? [failurePhase] : []), ...(rootErrorCode ? [rootErrorCode] : rootFallback)],
|
||||
});
|
||||
} catch (err) {
|
||||
// eslint-disable-next-line no-console
|
||||
|
||||
Reference in new issue
Block a user