mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-16 15:15:35 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The server and the browser need clear error reports when an operator enables external monitoring. > - Paperclip already uses an opt-in OpenTelemetry pattern for server traces. > - Sentry can provide error reports for both runtime paths when the operator sets one data source name. > - This pull request adds one opt-in Sentry gate for the server and the browser. > - The benefit is faster diagnosis while the default setup sends no Sentry data. ## Linked Issues or Issue Description **What is improved?** Paperclip gains optional error monitoring for server and browser failures. **Subsystem affected** Cross-cutting (server, UI, and shared authentication data). **Current behavior** Paperclip has no built-in Sentry error capture for server failures or browser boundary failures. Operators must inspect local logs and browser tools. **Proposed behavior** When the operator sets `SENTRY_DSN`, the server and authenticated browser use the same Sentry project. When the variable is absent, both paths stay inactive. The server loads Sentry dynamically and fails open when the optional package is absent. **Reason and benefit** Operators can inspect runtime errors in one Sentry project. The default setup remains local and sends no monitoring data. **Breaking changes** None when `SENTRY_DSN` remains unset. Authenticated session responses add the optional `sentryDsn` field. **Additional context** The implementation uses built-in Sentry privacy options. It disables default HTTP context and breadcrumb integrations and keeps `sendDefaultPii` false. ## What Changed - Add an opt-in server Sentry gate with dynamic package loading and fail-open behavior. - Add the Sentry data source name to the authenticated session response. - Add an authenticated browser Sentry gate and React error boundary capture. - Add tests for server, browser, route, and application error paths. - Document activation, installation, privacy settings, capture behavior, and operator controls. ## Verification - Run `npx vitest run server/src/__tests__/sentry.test.ts`. - Run `npx vitest run ui/src/lib/sentry.test.ts`. - Run `npx vitest run server/src/__tests__/auth-routes.test.ts server/src/__tests__/shutdown.test.ts`. - Confirm that the full continuous integration suite passes on this pull request. - Leave `SENTRY_DSN` unset and confirm that the server and browser gates stay inactive. - Set `SENTRY_DSN` and install the optional Sentry packages before a manual capture check. ## Risks The operator controls the Sentry project and accepts the data risk when the operator enables the feature. Error objects can contain messages, stacks, or cause chains with private values. The default configuration sends no data because the feature stays off without `SENTRY_DSN`. A missing optional server package does not stop server boot. ## Model Used OpenAI Codex, GPT-5, with tool use, repository inspection, GitHub CLI operations, and code review support. ## 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>
329 lines
11 KiB
TypeScript
329 lines
11 KiB
TypeScript
import { EventEmitter } from "node:events";
|
|
import { describe, expect, it, vi } from "vitest";
|
|
import {
|
|
coordinateHeartbeatSchedulerShutdown,
|
|
finalizeServerShutdown,
|
|
loadWithoutCoordinatedShutdownSignalHooks,
|
|
} from "./shutdown.js";
|
|
|
|
function deferred<T = void>() {
|
|
let resolve!: (value: T) => void;
|
|
let reject!: (reason?: unknown) => void;
|
|
const promise = new Promise<T>((res, rej) => {
|
|
resolve = res;
|
|
reject = rej;
|
|
});
|
|
return { promise, resolve, reject };
|
|
}
|
|
|
|
function stubLogger() {
|
|
return { info: vi.fn(), error: vi.fn() };
|
|
}
|
|
|
|
describe("finalizeServerShutdown", () => {
|
|
it("awaits the setup-token cleanup before the database stop and the process exit", async () => {
|
|
const order: string[] = [];
|
|
// The held promise models the setup-token session cancellation and its
|
|
// sandbox lease release. The teardown must not continue while it is pending.
|
|
const release = deferred();
|
|
const shutdownAppServices = vi.fn(async () => {
|
|
order.push("appServices:start");
|
|
await release.promise;
|
|
order.push("appServices:settled");
|
|
});
|
|
const stopEmbeddedPostgres = vi.fn(async () => {
|
|
order.push("postgres:stop");
|
|
});
|
|
const shutdownInstrumentation = vi.fn(async () => {
|
|
order.push("instrumentation:flush");
|
|
});
|
|
const shutdownSentry = vi.fn(async () => {
|
|
order.push("sentry:flush");
|
|
});
|
|
|
|
let exited = false;
|
|
const finalize = finalizeServerShutdown({
|
|
signal: "SIGTERM",
|
|
shutdownAppServices,
|
|
stopEmbeddedPostgres,
|
|
shutdownInstrumentation,
|
|
shutdownSentry,
|
|
log: stubLogger(),
|
|
}).then(() => {
|
|
// This models the caller's `process.exit(0)` continuation.
|
|
exited = true;
|
|
order.push("exit");
|
|
});
|
|
|
|
// The cleanup is in flight. The database stop, the instrumentation flush,
|
|
// and the process exit continuation must all wait for it to settle.
|
|
await vi.waitFor(() => expect(shutdownAppServices).toHaveBeenCalledOnce());
|
|
expect(stopEmbeddedPostgres).not.toHaveBeenCalled();
|
|
expect(shutdownInstrumentation).not.toHaveBeenCalled();
|
|
expect(exited).toBe(false);
|
|
|
|
release.resolve();
|
|
await finalize;
|
|
|
|
expect(exited).toBe(true);
|
|
expect(order).toEqual([
|
|
"appServices:start",
|
|
"appServices:settled",
|
|
"postgres:stop",
|
|
"instrumentation:flush",
|
|
"sentry:flush",
|
|
"exit",
|
|
]);
|
|
});
|
|
|
|
it("keeps the teardown durable and still exits when the setup-token release fails", async () => {
|
|
const order: string[] = [];
|
|
// The held promise rejects, which models a lease release that failed. The
|
|
// reaper owns the durable retry, so the teardown must log the failure and
|
|
// continue rather than swallow it or block the exit.
|
|
const release = deferred();
|
|
const releaseError = new Error("lease release failed");
|
|
const shutdownAppServices = vi.fn(async () => {
|
|
await release.promise;
|
|
});
|
|
const stopEmbeddedPostgres = vi.fn(async () => {
|
|
order.push("postgres:stop");
|
|
});
|
|
const shutdownInstrumentation = vi.fn(async () => {
|
|
order.push("instrumentation:flush");
|
|
});
|
|
const shutdownSentry = vi.fn(async () => undefined);
|
|
const log = stubLogger();
|
|
|
|
let exited = false;
|
|
const finalize = finalizeServerShutdown({
|
|
signal: "SIGTERM",
|
|
shutdownAppServices,
|
|
stopEmbeddedPostgres,
|
|
shutdownInstrumentation,
|
|
shutdownSentry,
|
|
log,
|
|
}).then(() => {
|
|
exited = true;
|
|
});
|
|
|
|
await vi.waitFor(() => expect(shutdownAppServices).toHaveBeenCalledOnce());
|
|
expect(stopEmbeddedPostgres).not.toHaveBeenCalled();
|
|
expect(exited).toBe(false);
|
|
|
|
release.reject(releaseError);
|
|
await finalize;
|
|
|
|
// The teardown surfaced the failure in the log, then finished the ordered
|
|
// teardown and reached the exit continuation.
|
|
expect(log.error).toHaveBeenCalledWith(
|
|
expect.objectContaining({ err: releaseError, signal: "SIGTERM" }),
|
|
expect.any(String),
|
|
);
|
|
expect(order).toEqual(["postgres:stop", "instrumentation:flush"]);
|
|
expect(exited).toBe(true);
|
|
});
|
|
|
|
it("skips the database stop when no embedded PostgreSQL runs in this process", async () => {
|
|
const shutdownAppServices = vi.fn(async () => undefined);
|
|
const shutdownInstrumentation = vi.fn(async () => undefined);
|
|
const shutdownSentry = vi.fn(async () => undefined);
|
|
const log = stubLogger();
|
|
|
|
await finalizeServerShutdown({
|
|
signal: "SIGINT",
|
|
shutdownAppServices,
|
|
stopEmbeddedPostgres: null,
|
|
shutdownInstrumentation,
|
|
shutdownSentry,
|
|
log,
|
|
});
|
|
|
|
expect(shutdownAppServices).toHaveBeenCalledOnce();
|
|
expect(shutdownInstrumentation).toHaveBeenCalledOnce();
|
|
expect(log.info).not.toHaveBeenCalled();
|
|
});
|
|
});
|
|
|
|
describe("loadWithoutCoordinatedShutdownSignalHooks", () => {
|
|
it("removes the eager signal handlers from the real embedded-postgres import", async () => {
|
|
const before = {
|
|
SIGINT: process.rawListeners("SIGINT"),
|
|
SIGTERM: process.rawListeners("SIGTERM"),
|
|
};
|
|
const moduleName = "embedded-postgres";
|
|
|
|
await loadWithoutCoordinatedShutdownSignalHooks(() => import(moduleName));
|
|
|
|
expect(process.rawListeners("SIGINT")).toEqual(before.SIGINT);
|
|
expect(process.rawListeners("SIGTERM")).toEqual(before.SIGTERM);
|
|
});
|
|
|
|
it("keeps the database available for a marker-backed SIGTERM snapshot", async () => {
|
|
const signalTarget = new EventEmitter();
|
|
const preexistingSignalListener = vi.fn();
|
|
signalTarget.on("SIGTERM", preexistingSignalListener);
|
|
|
|
let databaseAvailable = true;
|
|
const embeddedPostgresExitHook = vi.fn(() => {
|
|
databaseAvailable = false;
|
|
});
|
|
await loadWithoutCoordinatedShutdownSignalHooks(
|
|
async () => {
|
|
signalTarget.on("SIGINT", embeddedPostgresExitHook);
|
|
signalTarget.on("SIGTERM", embeddedPostgresExitHook);
|
|
return { default: class EmbeddedPostgres {} };
|
|
},
|
|
signalTarget,
|
|
);
|
|
|
|
let shutdown: Promise<unknown> | null = null;
|
|
let snapshotCaptured = false;
|
|
signalTarget.once("SIGTERM", () => {
|
|
shutdown = coordinateHeartbeatSchedulerShutdown({
|
|
signal: "SIGTERM",
|
|
prepareHotRestartShutdown: async () => {
|
|
// This models the real failure path: a valid intent exists, and the
|
|
// snapshot must query embedded PostgreSQL after SIGTERM is delivered.
|
|
expect(databaseAvailable).toBe(true);
|
|
snapshotCaptured = true;
|
|
return { mode: "hot_restart" as const, skipDrain: true };
|
|
},
|
|
waitForHeartbeatSchedulerIdle: vi.fn(async () => undefined),
|
|
});
|
|
});
|
|
|
|
signalTarget.emit("SIGTERM");
|
|
await shutdown;
|
|
|
|
expect(preexistingSignalListener).toHaveBeenCalledOnce();
|
|
expect(embeddedPostgresExitHook).not.toHaveBeenCalled();
|
|
expect(snapshotCaptured).toBe(true);
|
|
});
|
|
});
|
|
|
|
describe("coordinateHeartbeatSchedulerShutdown", () => {
|
|
it("quiesces active scheduler work before capturing a hot-restart snapshot", async () => {
|
|
let snapshotCaptured = false;
|
|
let releaseScheduler!: () => void;
|
|
const schedulerIdle = new Promise<void>((resolve) => {
|
|
releaseScheduler = resolve;
|
|
});
|
|
const waitForHeartbeatSchedulerIdle = vi.fn(() => schedulerIdle);
|
|
|
|
const shutdown = coordinateHeartbeatSchedulerShutdown({
|
|
signal: "SIGTERM",
|
|
prepareHotRestartShutdown: vi.fn(async () => {
|
|
snapshotCaptured = true;
|
|
return { mode: "prepared" as const, skipDrain: true };
|
|
}),
|
|
waitForHeartbeatSchedulerIdle,
|
|
});
|
|
|
|
await vi.waitFor(() => expect(waitForHeartbeatSchedulerIdle).toHaveBeenCalledOnce());
|
|
expect(snapshotCaptured).toBe(false);
|
|
releaseScheduler();
|
|
|
|
const result = await shutdown;
|
|
expect(snapshotCaptured).toBe(true);
|
|
expect(result).toEqual({
|
|
hotRestart: { mode: "prepared", skipDrain: true },
|
|
preparationError: null,
|
|
waitedForSchedulerIdle: true,
|
|
});
|
|
});
|
|
|
|
it("quiesces scheduler work before selecting server-stdio runs to drain", async () => {
|
|
const waitForHeartbeatSchedulerIdle = vi.fn(async () => undefined);
|
|
|
|
const result = await coordinateHeartbeatSchedulerShutdown({
|
|
signal: "SIGTERM",
|
|
prepareHotRestartShutdown: vi.fn(async () => ({
|
|
mode: "acp_drain_required" as const,
|
|
skipDrain: false,
|
|
drainRunIds: ["acp-run"],
|
|
})),
|
|
waitForHeartbeatSchedulerIdle,
|
|
});
|
|
|
|
expect(waitForHeartbeatSchedulerIdle).toHaveBeenCalledOnce();
|
|
expect(result).toEqual({
|
|
hotRestart: {
|
|
mode: "acp_drain_required",
|
|
skipDrain: false,
|
|
drainRunIds: ["acp-run"],
|
|
},
|
|
preparationError: null,
|
|
waitedForSchedulerIdle: true,
|
|
});
|
|
});
|
|
|
|
it("preserves the scheduler idle wait for normal graceful shutdown", async () => {
|
|
let releaseScheduler!: () => void;
|
|
const schedulerIdle = new Promise<void>((resolve) => {
|
|
releaseScheduler = resolve;
|
|
});
|
|
const waitForHeartbeatSchedulerIdle = vi.fn(() => schedulerIdle);
|
|
let settled = false;
|
|
|
|
const shutdown = coordinateHeartbeatSchedulerShutdown({
|
|
signal: "SIGTERM",
|
|
prepareHotRestartShutdown: vi.fn(async () => ({
|
|
mode: "not_requested" as const,
|
|
skipDrain: false,
|
|
})),
|
|
waitForHeartbeatSchedulerIdle,
|
|
}).finally(() => {
|
|
settled = true;
|
|
});
|
|
|
|
await vi.waitFor(() => expect(waitForHeartbeatSchedulerIdle).toHaveBeenCalledOnce());
|
|
expect(settled).toBe(false);
|
|
|
|
releaseScheduler();
|
|
|
|
await expect(shutdown).resolves.toEqual({
|
|
hotRestart: { mode: "not_requested", skipDrain: false },
|
|
preparationError: null,
|
|
waitedForSchedulerIdle: true,
|
|
});
|
|
});
|
|
|
|
it("waits for scheduler idle when hot-restart preparation is unavailable", async () => {
|
|
const waitForHeartbeatSchedulerIdle = vi.fn(async () => undefined);
|
|
|
|
const result = await coordinateHeartbeatSchedulerShutdown({
|
|
signal: "SIGTERM",
|
|
prepareHotRestartShutdown: null,
|
|
waitForHeartbeatSchedulerIdle,
|
|
});
|
|
|
|
expect(waitForHeartbeatSchedulerIdle).toHaveBeenCalledOnce();
|
|
expect(result).toEqual({
|
|
hotRestart: null,
|
|
preparationError: null,
|
|
waitedForSchedulerIdle: true,
|
|
});
|
|
});
|
|
|
|
it("falls back to the scheduler idle wait when hot-restart preparation fails", async () => {
|
|
const preparationError = new Error("snapshot failed");
|
|
const waitForHeartbeatSchedulerIdle = vi.fn(async () => undefined);
|
|
|
|
const result = await coordinateHeartbeatSchedulerShutdown({
|
|
signal: "SIGTERM",
|
|
prepareHotRestartShutdown: vi.fn(async () => {
|
|
throw preparationError;
|
|
}),
|
|
waitForHeartbeatSchedulerIdle,
|
|
});
|
|
|
|
expect(waitForHeartbeatSchedulerIdle).toHaveBeenCalledOnce();
|
|
expect(result).toEqual({
|
|
hotRestart: null,
|
|
preparationError,
|
|
waitedForSchedulerIdle: true,
|
|
});
|
|
});
|
|
});
|