mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-13 23:39:21 +02:00
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The server reports crashes to Sentry so operators can find real
faults
> - Three expected conditions report as crashes: a client that closes
the connection mid-request, one stale pooled database socket after a
pooled endpoint recycles, and the short boot window where a supervised
cloud stack runs a new app image before its migration runner has caught
up
> - These events arrive in the hundreds and bury real errors
> - This pull request classifies each condition as expected and stops
the Sentry capture for exactly that condition, with behavior unchanged
everywhere else
> - The benefit is a Sentry feed where each event is a real fault
## Linked Issues or Issue Description
**What happened?**
Three noise classes fill the backend Sentry project on managed cloud
fleets:
1. `Error: aborted` (ECONNRESET) reports as a 500 crash when a client
closes the tab or loses its network mid-request. Observed 18 times in
one week from routine client disconnects.
2. `Error: write CONNECTION_CLOSED ...` reports from many query paths
after a pooled Postgres endpoint suspends. The existing single retry in
cloud actor resolution still fails, because a suspended endpoint kills
every pooled socket at once and the one replay draws another dead
socket.
3. `Error: PostgreSQL has pending migrations (...). Refusing to start`
reports from every supervised stack during a fleet upgrade. The
supervisor delivers the new app image before it runs the migration
runner, so each stack crash-loops briefly by design. One fleet roll
produced 329 events (11 per container).
**Expected behavior**
A client disconnect ends the request quietly. A transient dead socket is
replayed until a live socket answers. A supervised mid-upgrade boot
refusal logs and exits nonzero without a Sentry capture, while the same
refusal on a self-hosted deployment keeps reporting.
**Steps to reproduce**
1. Abort an HTTP request mid-flight: the error handler reports a crash
to Sentry.
2. Suspend a pooled Postgres endpoint under an idle server, then issue
two quick requests: the first replay can draw a second dead socket and
surface `CONNECTION_CLOSED`.
3. On a deployment with `PAPERCLIP_CLOUD_API_ORIGIN` set, add a
migration file without running the migration runner and boot: the
refusal reports to Sentry.
**Paperclip version or commit**
master (0e14c61da)
## What Changed
- `server/src/middleware/error-handler.ts`: a request abort (`Error:
aborted` with `ECONNRESET`) ends the response with status 499 and skips
crash reporting and telemetry.
- `server/src/middleware/auth.ts`: `retryOnTransientDbConnectionError`
replays up to twice with a short pause, so a pool-wide recycle does not
defeat the retry.
- `server/src/startup-refusals.ts`: pending migrations on a database
with applied history classify as a supervised-transient refusal
(`schema-migration-pending`). The capture skip applies only when
`PAPERCLIP_CLOUD_API_ORIGIN` is set. A wiped journal beside real tables
keeps reporting. Self-hosted behavior is unchanged.
- Tests updated and added for all three behaviors.
## Verification
- `pnpm vitest run src/__tests__/startup-refusals.test.ts
src/__tests__/cloud-tenant-transient-db-retry.test.ts
src/__tests__/error-handler.test.ts` in `server/` — 25 tests, all pass.
- The abort test asserts no `captureException` and no telemetry crash
track.
- The refusal tests pin all three classifications: never-migrated,
pending-with-history, wiped journal.
## Risks
- Low risk. The abort path only triggers on the exact `aborted` +
`ECONNRESET` pair; every other error keeps reporting.
- The refusal reclassification suppresses a capture only under a cloud
supervisor. If an operator breaks a migration runner, the supervisor's
own deploy gates surface it; self-hosted deployments still report.
- The retry widening adds at most ~150 ms before a genuine connection
fault surfaces.
## Model Used
Claude (Anthropic) — claude-fable-5 (Claude Fable 5), Claude Code
harness, extended thinking with tool use.
## 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
89 lines
3.8 KiB
TypeScript
89 lines
3.8 KiB
TypeScript
/**
|
|
* Deliberate boot refusals and whether they should page Sentry.
|
|
*
|
|
* Some startup preconditions the server must not repair itself: an
|
|
* unmigrated schema when auto-apply is off (the operator's migration
|
|
* runner owns schema), or an unmet database contract for authenticated
|
|
* public deployments. The server logs the refusal and exits nonzero so
|
|
* whatever supervises the deployment can act.
|
|
*
|
|
* In supervised managed-cloud deployments (`PAPERCLIP_CLOUD_API_ORIGIN`
|
|
* set), two of these refusals are a routine provisioning phase rather
|
|
* than an incident: a freshly created stack's app container boots
|
|
* before the harness has migrated the empty database or finished
|
|
* applying its committed configuration, crash-loops briefly, and is
|
|
* restarted by the harness once the precondition holds. Reporting every
|
|
* such boot to Sentry buries real errors under hundreds of expected
|
|
* events per fleet build batch, so the crash handler skips the capture
|
|
* for exactly this class — the refusal still logs and still exits
|
|
* nonzero. Everywhere else (self-hosted, local dev) reporting is
|
|
* unchanged.
|
|
*/
|
|
|
|
export type StartupRefusalKind =
|
|
| "schema-not-yet-migrated"
|
|
| "schema-migration-pending"
|
|
| "database-contract-unmet";
|
|
|
|
/**
|
|
* A boot refusal whose remedy belongs to the deployment's supervisor.
|
|
* Only refusals that are *expected transients* under managed-cloud
|
|
* provisioning use this class; refusals that always indicate operator
|
|
* error (schema drift, a malformed DATABASE_URL) stay plain `Error`s.
|
|
*/
|
|
export class StartupRefusalError extends Error {
|
|
readonly kind: StartupRefusalKind;
|
|
|
|
constructor(kind: StartupRefusalKind, message: string) {
|
|
super(message);
|
|
this.name = "StartupRefusalError";
|
|
this.kind = kind;
|
|
}
|
|
}
|
|
|
|
/**
|
|
* Chooses the error class for a pending-migrations refusal. A database
|
|
* with zero applied migrations AND zero tables is not stale — it has
|
|
* never been migrated at all, which under a supervisor means "not yet"
|
|
* rather than "drifted". Any applied history, or any pre-existing
|
|
* tables beside an empty or wiped migration journal, makes pending
|
|
* migrations a drift signal that must keep reporting.
|
|
*/
|
|
export function migrationRefusalError(
|
|
state: { appliedMigrations: string[]; tableCount: number },
|
|
message: string,
|
|
): Error {
|
|
const neverMigrated = state.appliedMigrations.length === 0 && state.tableCount === 0;
|
|
if (neverMigrated) return new StartupRefusalError("schema-not-yet-migrated", message);
|
|
// A database with applied HISTORY and newer pending files is normal
|
|
// mid-upgrade under a supervisor: managed fleet rolls deliver the new
|
|
// app image before the migration runner, so every upgraded stack
|
|
// briefly boots ahead of its schema and crash-loops until the
|
|
// supervisor migrates and restarts it (observed: ~11 events per
|
|
// container, hundreds per fleet roll). It still refuses, logs, and
|
|
// exits nonzero; only the Sentry capture is skipped — and only when
|
|
// `PAPERCLIP_CLOUD_API_ORIGIN` marks the deployment as supervised
|
|
// (`shouldReportStartupFailure`). Self-hosted deployments keep
|
|
// reporting.
|
|
if (state.appliedMigrations.length > 0) {
|
|
return new StartupRefusalError("schema-migration-pending", message);
|
|
}
|
|
// An empty or wiped migration journal beside real tables is genuine
|
|
// drift with no supervisor remedy on the way; it must keep reporting.
|
|
return new Error(message);
|
|
}
|
|
|
|
/**
|
|
* Whether a startup failure should be captured to Sentry. Everything
|
|
* reports except a supervised-transient refusal in a managed-cloud
|
|
* deployment.
|
|
*/
|
|
export function shouldReportStartupFailure(
|
|
error: unknown,
|
|
env: NodeJS.ProcessEnv = process.env,
|
|
): boolean {
|
|
if (!(error instanceof StartupRefusalError)) return true;
|
|
const cloudOrigin = env.PAPERCLIP_CLOUD_API_ORIGIN?.trim();
|
|
return !cloudOrigin;
|
|
}
|