Files
PaperClipAI/server/src/sentry.ts
T
515ff0f284 fix(server): report which S3 call failed in storage error events (#17424)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - The server keeps attachments and other files in a storage provider.
Hosted instances use the S3 provider.
> - Sentry received `SignatureDoesNotMatch` errors from the S3 provider
on two staging hosts.
> - The events have only AWS SDK stack frames. They do not show which S3
call failed, the HTTP status, or the S3 request ID.
> - Without these facts, we cannot find the cause or make a safe fix.
> - This pull request records a small, fixed-shape failure record on the
S3 error and sends it with the Sentry event.
> - The benefit is that the next event shows the failed operation and
the provider request ID, and no private data goes to Sentry.

## Linked Issues or Issue Description

**What happened?**
The S3 storage provider threw `SignatureDoesNotMatch` four times on
staging. The Sentry events do not identify the S3 operation or the
caller.

**Expected behavior**
A storage provider failure in Sentry shows which storage call failed and
the provider's correlation fields.

**Steps to reproduce**
1. Configure the S3 storage provider against an endpoint that rejects
the request signature.
2. Read, write, check, or delete a stored file.
3. Look at the Sentry event. It has no first-party stack frame and no
operation name.

No public GitHub issue exists. I searched open and closed pull requests
for `SignatureDoesNotMatch` and storage failure diagnostics. No pull
request covers this.

## What Changed

- Add `server/src/storage/failure-diagnostics.ts`. It keeps a frozen,
fixed-shape record for each failed storage error in a `WeakMap`.
- Add `server/src/storage/s3-failure.ts`. It wraps each S3 call and
records the operation, error code, HTTP status, request ID, and SDK
attempt count. It reads only data properties. A value that does not
match its bounded format becomes `unknown` or `null`.
- Wrap `PutObject`, `GetObject`, `HeadObject`, `DeleteObject`,
`CreateMultipartUpload`, `UploadPart`, and `CompleteMultipartUpload`.
- In `captureException`, add the `storage_operation` and
`storage_error_code` tags and a `storage_failure` context to the event
of that error only. The original error and its default grouping do not
change.
- Document the new context in `doc/observability.md`.

## Verification

- `pnpm exec vitest run
src/__tests__/storage-s3-failure-diagnostics.test.ts
src/__tests__/sentry.test.ts` (in `server/`): 37 passed, 5 skipped. The
skipped tests are the existing real-SDK tests, which need an environment
flag.
- `pnpm exec vitest run src/__tests__/storage-s3-multipart.test.ts
src/__tests__/storage-local-provider.test.ts`: 8 passed.
- The new tests use a local HTTP server that returns a real S3
`SignatureDoesNotMatch` XML body with a `StringToSign` field. The tests
make sure that the record has the operation, code, status and request
ID, and that it does not contain the private string.
- I typechecked the changed source files with a scoped `tsc` project.
They have no errors. I could not run the full `pnpm -r typecheck` and
`pnpm build` in my sandbox: it has a 4 GiB memory limit and no `cargo`
for the runner binary. CI runs the full checks.

## Risks

- Low risk. The change adds a `try`/`catch` that records data and throws
the same error again. Storage behavior, not-found handling, and HTTP
responses do not change.
- Secret-sensitive routes replace the error before reporting, so those
events do not get the record. This is intended.
- The record never contains the bucket, object key, endpoint,
credentials, error message, or provider response body.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic. Used through Claude
Code with tool use (shell, file edit) and extended reasoning.

## 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: devinfoley <139239+devinfoley@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-24 05:08:56 -07:00

430 lines
18 KiB
TypeScript

// Optional Sentry error monitoring for the server process.
//
// Activated only when the backend DSN resolves to a value — see
// `resolveSentryDsns` in `sentry-dsn.ts` for the precedence between
// `SENTRY_DSN_BACKEND` and the legacy `SENTRY_DSN` fallback. When it
// resolves to `null`, no Sentry package is loaded at all.
//
// The import is dynamic and the package is an optional runtime dependency —
// operators who want server-side error monitoring install `@sentry/node`
// themselves. That keeps Sentry off the default dependency graph and avoids
// forcing a lockfile bump for an opt-in feature. This gate mirrors the
// OpenTelemetry gate in `instrumentation.ts`.
//
// OpenTelemetry keeps ownership of trace setup: the initializer passes
// `skipOpenTelemetrySetup: true` and `tracesSampleRate: 0`, so this module
// adds error monitoring only and starts no span or trace behavior of its
// own.
//
// `serverName`: the initializer sets this to the host name of the process,
// with `os.hostname()`. The `@sentry/node` client already falls back to the
// same host name when the caller omits this option, so this line makes an
// existing default explicit instead of changing captured event content. An
// explicit value stays correct if a future SDK version changes its default.
//
// An operator can still send a different value in place of the host name.
// When the environment variable `SENTRY_NAME` holds a non-empty string, the
// initializer uses that value instead. This keeps the same order the
// `@sentry/node` client itself uses when the caller omits `serverName`.
//
// Default-integration privacy note: `sendDefaultPii: false` filters values
// by name, inside the `RequestData` integration only. Three other default
// integrations copy raw values past that filter, so the initializer removes
// or narrows them with built-in Sentry options — no custom filter code:
// - `Console` turns a `console.*` call into a breadcrumb with the raw
// arguments. The initializer drops it.
// - `ContextLines` reads local source lines around each stack frame off
// the host disk. The initializer drops it.
// - `Http` records a breadcrumb for each outbound request, with its URL
// and query string. The initializer keeps the integration (`RequestData`
// and request isolation need it) and turns the breadcrumb off with the
// integration's own `breadcrumbs` option.
//
// `onUnhandledRejectionIntegration` defaults to `mode: "warn"`, which
// registers a `process.on("unhandledRejection")` listener. Node cancels its
// own crash-on-unhandled-rejection behavior when any listener is registered.
// The server relies on that crash today, so the initializer passes
// `mode: "strict"`: Sentry still captures the event, then exits the process,
// so the existing crash-and-restart behavior stays.
//
// Before it imports the package, the bootstrap checks the installed
// `@sentry/node` version against the exact version this manifest's
// `peerDependencies` declares — the same audited version documented in
// `doc/observability.md`. A missing or a mismatched version logs one
// diagnostic and leaves the server running without error monitoring; it
// never throws. This gate mirrors the OpenTelemetry gate in
// `instrumentation.ts`.
import os from "node:os";
import { AdapterStopTimeoutError } from "./services/adapter-stop-timeout.js";
import { CloudPortfolioError } from "./services/cloud-portfolio-error.js";
import { storageFailureDiagnosticsFor } from "./storage/failure-diagnostics.js";
import type { RunFailureDiagnostics } from "./services/run-failure-diagnostics.js";
import { readBuildCommit } from "./build-commit.js";
import { checkExactPeerVersions } from "./peer-version-check.js";
import { resolveSentryDsns } from "./sentry-dsn.js";
import { readRunFailureReportingMode } from "./sentry-run-failure-policy.js";
import { addSentryRunLinks, type SentryRunReference } from "./sentry-links.js";
const { backend: dsn, legacyFallbackUsed } = resolveSentryDsns();
if (legacyFallbackUsed) {
// eslint-disable-next-line no-console
console.warn(
"[paperclip] SENTRY_DSN_FRONTEND or SENTRY_DSN_BACKEND is not set. " +
"The server uses the legacy SENTRY_DSN value for the affected " +
"component. Set SENTRY_DSN_FRONTEND and SENTRY_DSN_BACKEND to send " +
"each component to its own Sentry project.",
);
}
/** Event-local context accepted by the optional Sentry package. */
interface SentryCaptureContext {
tags: Record<string, string>;
contexts: Record<string, Record<string, unknown>>;
fingerprint: string[];
}
/** The subset of the `@sentry/node` client surface this gate calls. */
interface SentryHandle {
captureException(error: unknown, context?: SentryCaptureContext): string;
close(timeout?: number): Promise<boolean>;
}
let sentryHandle: SentryHandle | null = null;
let shutdownPromise: Promise<void> | null = null;
/**
* Resolves once the Sentry SDK has started, or once bootstrap has failed and
* logged, or at once when the backend DSN resolves to `null`. No caller
* needs to await this before calling `captureException` — it is a no-op
* until ready — but `index.ts` awaits it at startup so the first real error
* has a live client.
*/
export const sentryReady: Promise<void> = dsn ? bootstrapSentry(dsn) : Promise.resolve();
/**
* Report an error to Sentry. A no-op before the gate opens, when the gate
* never opens (the backend DSN resolves to `null`), or when bootstrap
* failed. Never throws — observability must not change control flow.
*/
export function captureException(error: unknown, reference?: SentryRunReference): void {
if (!sentryHandle) return;
try {
const tags = reference ? {
run_id: reference.runId,
...(reference.companyId ? { company_id: reference.companyId } : {}),
...(reference.agentId ? { agent_id: reference.agentId } : {}),
} : {};
const storageFailure = storageFailureDiagnosticsFor(error);
if (error instanceof AdapterStopTimeoutError) {
// Event-local, fixed-shape context: no ambient scope or raw error fields.
const exception = new Error(error.message);
exception.stack = error.stack;
sentryHandle.captureException(exception, {
tags: { ...tags, error_code: "adapter_stop_unconfirmed" },
contexts: { adapter_stop: { ...error.diagnostics } },
fingerprint: ["{{ default }}"],
});
} else if (error instanceof CloudPortfolioError) {
const exception = new Error(error.message);
exception.stack = error.stack;
sentryHandle.captureException(exception, {
tags: { ...tags, error_code: "cloud_portfolio_failure" },
contexts: { cloud_portfolio: { ...CloudPortfolioError.diagnosticsFor(error) } },
fingerprint: ["{{ default }}"],
});
} else if (storageFailure) {
// The original error keeps its default grouping; only the normalized
// record is added, never the provider's request or response.
sentryHandle.captureException(error, {
tags: { ...tags, storage_operation: storageFailure.operation, storage_error_code: storageFailure.errorCode },
contexts: { storage_failure: { ...storageFailure } },
fingerprint: ["{{ default }}"],
});
} else if (reference) {
sentryHandle.captureException(error, { tags, contexts: {}, fingerprint: ["{{ default }}"] });
} else {
sentryHandle.captureException(error);
}
} catch (err) {
// eslint-disable-next-line no-console
console.error("[paperclip] Sentry captureException failed", err);
}
}
/** The run status values that mark a run as a genuine terminal failure. */
export type RunFailureStatus = "failed" | "timed_out" | "cancelled";
/**
* The diagnostic values `captureRunFailure` sends with a terminal-failure
* event. `errorCode` is `null` when the run holds no error code.
*/
export interface RunFailureEvent {
taskId: string | null;
companyId?: string;
agentId?: string;
/** The `heartbeat_runs` row id. */
runId: string;
/** The redacted error message. */
errorMessage: string;
/** The run's error code, or `null` when the run holds none. */
errorCode: string | null;
/** The agent's adapter type, or `"unknown"` when the agent row is absent. */
agentAdapter: string;
/** The run status that triggered this report. */
runStatus: RunFailureStatus;
/** Bounded, redacted diagnostics selected by the run failure reporter. */
diagnostics?: RunFailureDiagnostics;
/** Recorded process exit evidence, when available. Validated before capture. */
exitCode?: number | null;
signal?: string | null;
}
function normalizeRunExitCode(value: unknown): number | null {
// Match the persisted PostgreSQL integer. Do not coerce adapter-supplied text.
return typeof value === "number" &&
Number.isInteger(value) &&
value >= -2147483648 &&
value <= 2147483647
? value
: null;
}
function normalizeRunSignal(value: unknown): string | null {
if (value == null) return null;
// Only host signal constants may leave the process; arbitrary adapter text
// can contain output or credentials even when stored in the signal column.
return typeof value === "string" && Object.hasOwn(os.constants.signals, value)
? value
: "unknown";
}
/**
* Report one terminal run failure to Sentry. A no-op before the gate opens
* or when the gate never opens. Never throws — observability must not
* change run control flow.
*
*/
export function captureRunFailure(event: RunFailureEvent): void {
if (!sentryHandle || readRunFailureReportingMode() === "off") return;
const handle = sentryHandle;
try {
const errorCode = event.errorCode ?? "unknown";
// Sentry's async scope isolation is absent when OTel setup is skipped.
// A withScope mutation can then persist into unrelated later captures.
// Pass these fields on this event only; do not mutate the ambient scope.
const diagnostics = event.diagnostics;
const rootCodeCandidate = diagnostics?.execution.originalFailureCode ??
diagnostics?.exceptions.at(-1)?.code;
const rootErrorCode = typeof rootCodeCandidate === "string" && /^[a-zA-Z][a-zA-Z0-9_]{1,99}$/.test(rootCodeCandidate)
? rootCodeCandidate : undefined;
const phase = diagnostics?.execution.failurePhase;
const failurePhase = phase === "setup" || phase === "execute" ? phase : undefined;
const contexts: Record<string, Record<string, unknown>> = {};
if (event.companyId && event.agentId) {
const runPath = `/api/heartbeat-runs/${encodeURIComponent(event.runId)}`;
contexts.paperclip = {
schemaVersion: 1,
companyId: event.companyId,
agentId: event.agentId,
runPath,
eventsPath: `${runPath}/events`,
logPath: `${runPath}/log`,
...(event.taskId ? { taskPath: `/api/issues/${encodeURIComponent(event.taskId)}` } : {}),
};
try {
const base = new URL(process.env.PAPERCLIP_PUBLIC_URL ?? "");
if ((base.protocol === "https:" || base.protocol === "http:") &&
!base.username && !base.password && !base.search && !base.hash && base.pathname === "/") {
contexts.paperclip.runUrl = new URL(runPath, base).href;
contexts.paperclip.eventsUrl = new URL(`${runPath}/events`, base).href;
contexts.paperclip.logUrl = new URL(`${runPath}/log`, base).href;
}
} catch {}
}
if (diagnostics) {
contexts.run_execution = { ...diagnostics.execution, truncatedFields: diagnostics.truncatedFields };
if (Object.keys(diagnostics.adapter).length) contexts.adapter_failure = diagnostics.adapter;
if (Object.keys(diagnostics.provider).length) contexts.provider_failure = diagnostics.provider;
diagnostics.exceptions.forEach(({ name, code, status, requestId }, index) => {
contexts[`run_exception_${index}`] = { name, code, status, requestId };
});
}
// Rebuild only sanitized exception fields. Passing a raw SDK Error can
// serialize its request/response, headers, or other enumerable properties.
let cause: Error | undefined;
for (const entry of [...(diagnostics?.exceptions ?? [])].reverse()) {
const error: Error = new Error(entry.message ?? event.errorMessage, cause ? { cause } : undefined);
error.name = entry.name ?? "Error";
error.stack = entry.stack;
cause = error;
}
const exception = cause ?? new Error(event.errorMessage);
if (!cause) {
// A saved adapter result is not a thrown Error. Do not pretend that the
// reporter's own stack is the failure location.
exception.stack = typeof diagnostics?.adapter.stackPreview === "string"
? diagnostics.adapter.stackPreview : undefined;
}
handle.captureException(exception, {
tags: {
event_kind: "run_failure",
...(rootErrorCode ? { root_error_code: rootErrorCode } : {}),
...(failurePhase ? { failure_phase: failurePhase } : {}),
run_id: event.runId,
...(event.taskId ? { task_id: event.taskId } : {}),
...(event.companyId ? { company_id: event.companyId } : {}),
...(event.agentId ? { agent_id: event.agentId } : {}),
error_code: errorCode,
agent_adapter: event.agentAdapter,
run_status: event.runStatus,
},
contexts: {
...contexts,
run_failure: {
taskId: event.taskId,
runId: event.runId,
errorMessage: event.errorMessage,
errorCode,
agentAdapter: event.agentAdapter,
exitCode: normalizeRunExitCode(event.exitCode),
signal: normalizeRunSignal(event.signal),
},
},
fingerprint: [errorCode, event.agentAdapter, ...(failurePhase ? [failurePhase] : []), ...(rootErrorCode ? [rootErrorCode] : [])],
});
} catch (err) {
// eslint-disable-next-line no-console
console.error("[paperclip] Sentry captureRunFailure failed", err);
}
}
/**
* Flush buffered events and close the Sentry client. Idempotent — concurrent
* callers share one shutdown. A no-op when monitoring is off or bootstrap
* failed.
*/
export function shutdownSentry(): Promise<void> {
shutdownPromise ??= (async () => {
await sentryReady;
if (!sentryHandle) return;
try {
// Awaiting matters: the client flushes buffered events to Sentry
// during close; exiting before it settles silently drops them.
await sentryHandle.close(5_000);
} catch (err) {
// eslint-disable-next-line no-console
console.error("[paperclip] Sentry shutdown failed", err);
}
})();
return shutdownPromise;
}
/**
* The subset of the `@sentry/node` module surface the initializer needs to
* build its options object. A structural type, not the real Sentry type —
* the real type is unavailable at compile time because the package is an
* optional runtime dependency (see the module comment above).
*/
interface SentryModuleLike {
httpIntegration(options: { breadcrumbs: boolean }): { name: string };
onUnhandledRejectionIntegration(options: { mode: "strict" }): { name: string };
}
/** The `Sentry.init` options this gate builds. */
export interface SentryInitOptions {
dsn: string;
release?: string;
skipOpenTelemetrySetup: boolean;
tracesSampleRate: number;
sendDefaultPii: boolean;
serverName: string;
integrations: (defaults: Array<{ name: string }>) => Array<{ name: string }>;
beforeSend: typeof addSentryRunLinks;
}
/**
* Build the `Sentry.init` options object. A pure function, split out from
* `bootstrapSentry` so a test can call it with a real `@sentry/node` module
* and assert the resolved integration list and the captured-event shape
* against the true SDK, not a stand-in.
*/
export function buildSentryInitOptions(
dsn: string,
Sentry: SentryModuleLike,
): SentryInitOptions {
return {
dsn,
release: process.env.SENTRY_RELEASE?.trim() || readBuildCommit() || undefined,
skipOpenTelemetrySetup: true,
tracesSampleRate: 0,
sendDefaultPii: false,
beforeSend: addSentryRunLinks,
serverName: process.env.SENTRY_NAME || os.hostname(),
integrations: (defaults: Array<{ name: string }>) => {
const kept = defaults.filter(
(integration) =>
integration.name !== "Console" &&
integration.name !== "ContextLines" &&
integration.name !== "Http" &&
integration.name !== "OnUnhandledRejection",
);
return [
...kept,
// Keep the rest of the Http integration — RequestData and request
// isolation need it — but turn the outbound breadcrumb off.
Sentry.httpIntegration({ breadcrumbs: false }),
// Keep today's crash-on-unhandled-rejection behavior. See the
// module comment above for why the default mode cannot stay.
Sentry.onUnhandledRejectionIntegration({ mode: "strict" }),
];
},
};
}
async function bootstrapSentry(dsn: string): Promise<void> {
// Gate on the exact peer version before touching the dynamic import: a
// package installed at the wrong version can still load and start, which
// would silently invalidate the privacy audit `doc/observability.md`
// records against one exact version. Checking first turns that into one
// precise, fail-open diagnostic.
const versionCheck = checkExactPeerVersions(["@sentry/node"]);
if (!versionCheck.ok) {
// eslint-disable-next-line no-console
console.warn(
"[paperclip] The backend Sentry DSN is set, but the @sentry/node " +
"package is not installed, or is installed at an unsupported " +
"version. Install the declared version of @sentry/node to enable " +
"server error monitoring. Continuing without it.",
versionCheck.detail,
);
return;
}
try {
// Dynamic import so type-resolution doesn't require the package to be
// installed unless the operator actually opts in.
// @ts-ignore optional peer dep
const Sentry = await import("@sentry/node");
Sentry.init(buildSentryInitOptions(dsn, Sentry));
sentryHandle = {
captureException: (...args) => Sentry.captureException(...args),
close: (timeout) => Sentry.close(timeout),
};
} catch (err) {
// The exact-version gate above already confirmed @sentry/node is
// installed at the declared version, so only a load or init failure
// after that point reaches this block.
// eslint-disable-next-line no-console
console.warn(
"[paperclip] The backend Sentry DSN is set, and @sentry/node passed " +
"the version check, but it failed to load or initialize. " +
"Continuing without error monitoring.",
err,
);
}
}