mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-12 08:19:54 +02:00
## Thinking Path > - Paperclip manages work for AI agents and their companies. > - Cloud instances proxy a trusted user's portfolio through the control plane. > - A failed request returns a generic error to the client. > - That replacement error loses the failure phase and network code in Sentry. > - Operators need bounded evidence without upstream messages or credentials. > - This PR adds safe diagnostics while preserving the existing request behavior. ## Linked Issues or Issue Description Refs #10850, which added the portfolio proxy. No open PR for this diagnostic gap was found. **What happened?** A rejected portfolio fetch becomes a generic 502 in Sentry. The event cannot distinguish a connection reset, deadline, HTTP response failure, or body failure. The route's previous warning also included the original error and a stack identifier. **Steps to reproduce** Make the portfolio proxy's fetch reject with a TypeError whose cause has `code: ECONNRESET`. The client correctly receives the generic 502, but the captured replacement error loses that code. **Expected behavior** Keep the existing client response. Attach only bounded server-side diagnostic fields to the failure event. Do not retry the request or expose the original error. ## What Changed - Add a typed portfolio error with a private frozen diagnostic record: phase, upstream HTTP status, elapsed milliseconds, and an allowlisted network code. - Read at most four error/cause objects through own data properties. Unknown codes, messages, getters, and out-of-range values do not enter the record. - Replace the route's raw-error warnings with safe fields. Send a plain error plus event-local context through the existing optional Sentry gate. Keep the route callsite and default fingerprint policy. - Preserve authentication, trusted headers, cookies, exact HTTP error bodies, cache behavior, the ten-second deadline, and one fetch per request. Public responses receive no diagnostic fields. - Document the fields and test HTTP behavior, privacy, and event isolation with the real Sentry SDK. ## Verification - `PAPERCLIP_REQUIRE_SENTRY_TEST_SDK=1 pnpm exec vitest run server/src/__tests__/cloud-portfolio-error.test.ts server/src/__tests__/cloud-routes.test.ts server/src/__tests__/sentry.test.ts server/src/__tests__/run-failure-sentry-real-sdk.test.ts`: 65 passed, with the audited optional SDK installed. - Full local `pnpm -r typecheck` and `pnpm build` passed on Node 24.21.0 and pnpm 9.15.4. - Independent review found no blockers and independently passed all 65 tests, including the real SDK checks, on this exact commit. - Full Linux CI passed on this exact commit and provides aggregate suite coverage (54 successful checks, 2 intentional skips). A duplicate full local aggregate was not run. - The first SDK contract job failed before tests when npm could not resolve an OpenTelemetry transitive package. A subsequent empty-cache install first encountered a missing tarball, then succeeded after the registry artifact became available. All 6 real-SDK tests passed against that fresh install; the single unchanged-head CI retry passed. The SDK pin, workflow, and dependency files are unchanged. - The first browser shard 8 run timed out waiting for the inbox retry reply after 45 seconds. The unchanged isolated case passed (1/1), and the test, UI handler, fixture, and recovery files match the base commit. The failed log contains no wakeup POST before the test's immediate navigation; a navigation/request timing race is suspected but unproven without a trace. The single unchanged-head shard retry passed (20 passed, 1 skipped); no timeout or source change was made. - Greptile reviewed this exact commit at 5/5 with no unresolved review threads. - No live portfolio request was replayed. Route tests use controlled local upstream responses. - The added diff passed the secret and PII scan and `git diff --check`. ## Risks This is a diagnostic change. It does not identify or repair the origin of a connection reset. Unknown transport failures remain `unknown`. Elapsed values outside 0–60,000 ms become null. Default Sentry fingerprinting remains enabled; exact historical group membership is not guaranteed. No retry, migration, deployment, or configuration change is included. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository editing, code execution, and independent agent review. The exact deployment model ID and context window are not exposed in this session. ## 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 - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
380 lines
16 KiB
TypeScript
380 lines
16 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 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";
|
|
|
|
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): void {
|
|
if (!sentryHandle) return;
|
|
try {
|
|
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: { 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: { error_code: "cloud_portfolio_failure" },
|
|
contexts: { cloud_portfolio: { ...CloudPortfolioError.diagnosticsFor(error) } },
|
|
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 {
|
|
/** The task UUID the run belongs to. */
|
|
taskId: 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.
|
|
*
|
|
* Sets the fingerprint to `[errorCode, agentAdapter]`, in that order, so
|
|
* Sentry groups events by error code and adapter. The error message stays
|
|
* out of the fingerprint — it still travels as the exception message and as
|
|
* a field of the `run_failure` context.
|
|
*/
|
|
export function captureRunFailure(event: RunFailureEvent): void {
|
|
if (!sentryHandle) 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 contexts: Record<string, Record<string, unknown>> = {};
|
|
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: {
|
|
run_id: event.runId,
|
|
task_id: event.taskId,
|
|
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],
|
|
});
|
|
} 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 }>;
|
|
}
|
|
|
|
/**
|
|
* 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,
|
|
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,
|
|
);
|
|
}
|
|
}
|