Commit Graph
2 Commits
Author SHA1 Message Date
Devin Foley a59f5a8adc fix(server): stop reporting expected managed-cloud transients to Sentry (#13323)
## 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
2026-09-12 11:15:43 -07:00
Devin Foley 174e35a144 fix(server): stop paging Sentry for supervised boot races in managed cloud (#12772)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - The server refuses to boot when its database is not migrated, or
when an authenticated public deployment has no `DATABASE_URL`. These
refusals are deliberate and correct.
> - In managed cloud, a supervisor creates each stack, migrates its
fresh database, applies configuration, and restarts the app. The app
container often boots before those steps finish.
> - Each early boot hits one of the two refusals, exits, and captures
the refusal to Sentry. One fleet build batch produces hundreds of
identical expected events. Real errors get buried.
> - This pull request classifies exactly those two refusals as expected
transients when `PAPERCLIP_CLOUD_API_ORIGIN` marks a supervised
deployment, and skips only the Sentry capture for them.
> - The benefit is a clean error signal: expected provisioning noise
stops, and every real failure still reports.

## Linked Issues or Issue Description

**What happened?**

A managed-cloud stack boots its app container before the supervisor
migrates the empty database or finishes applying configuration. The
container refuses to start, crash-loops briefly, and converges after the
supervisor restarts it. Every refused boot sends an error event to
Sentry. A batch of new stacks produces hundreds of these expected
events.

**Expected behavior**

The refusal logs and exits nonzero, so the supervisor can act. Sentry
receives no event for an expected provisioning transient. Sentry still
receives events for real failures: schema drift, malformed
configuration, and every refusal outside managed cloud.

**Steps to reproduce**

1. Set `PAPERCLIP_MIGRATION_AUTO_APPLY=false`,
`PAPERCLIP_MIGRATION_PROMPT=never`, `SENTRY_DSN`, and
`PAPERCLIP_CLOUD_API_ORIGIN`.
2. Point `DATABASE_URL` at an empty database and start the server.
3. The server refuses to start. Before this change it also captures the
refusal to Sentry on every boot.

**Deployment mode**

Authenticated public (managed cloud).

## What Changed

- New `server/src/startup-refusals.ts`: a `StartupRefusalError` class
for refusals whose remedy belongs to the deployment supervisor,
`migrationRefusalError()` to classify a pending-migrations refusal (zero
applied migrations = never migrated = supervised transient; any applied
history = drift = plain always-reported `Error`), and
`shouldReportStartupFailure()` for the capture decision.
- `server/src/index.ts`: the pending-migrations refusal uses the
classifier; the missing-`DATABASE_URL` refusal under the
authenticated-public contract becomes a `StartupRefusalError` (the
malformed-URL refusal stays a plain `Error`); the startup crash handler
consults `shouldReportStartupFailure()` before `captureException`.
Logging and the nonzero exit are unchanged.
- New `server/src/__tests__/startup-refusals.test.ts` covering the
classification and decision matrix, including the unchanged self-hosted
paths.

## Verification

- `pnpm vitest run src/__tests__/startup-refusals.test.ts` — 7 passed.
- Review the decision matrix in the test file: refusals report when
`PAPERCLIP_CLOUD_API_ORIGIN` is absent or blank; non-refusal errors and
non-`Error` throwables always report; drift always reports.

## Risks

- Low risk. The change only skips a Sentry capture in one narrow,
marker-gated case. Boot behavior, logging, and the exit code do not
change.
- Self-hosted deployments do not set `PAPERCLIP_CLOUD_API_ORIGIN`, so
their reporting is unchanged, and the tests pin that.
- A supervised deployment with a genuinely stuck migration runner loses
per-boot Sentry events for that stack. The supervisor's own health
checks and monitoring own that signal, and the container logs still
carry the refusal.

## Model Used

Claude Fable 5 (claude-fable-5) via Claude Code, 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 (none found for startup Sentry suppression)
- [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
(module doc comment; no user-facing docs affected)
- [x] I have considered and documented any risks above
2026-09-03 10:02:42 -07:00