Files
PaperClipAI/server/src/middleware/error-handler.ts
T
9cc979116e fix(server): stop reporting queries that outlive the database shutdown (#16243)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - The server reports unexpected route errors to Sentry through the
shared error handler.
> - An orderly shutdown drains the HTTP listener, and then it ends the
database pool.
> - `server.close()` completes when the sockets close. A route handler
whose client disconnected, or whose socket the drain timeout closed, can
still run.
> - The next query of that handler fails with postgres.js
`CONNECTION_ENDED`, because this process ended the pool. The error
handler reports this as a 500 crash.
> - This pull request marks the pool as closing just before it ends. The
error handler then skips the error sinks for that one error code during
shutdown.
> - Public MCP calls run in their own internal Express app, so the flag
is also shared with that app.
> - The benefit is that a normal restart does not create false Sentry
crash reports. Real errors are still reported.

## Linked Issues or Issue Description

No public GitHub issue exists. This is the bug description.

**What happened?**

A staging server restarted. During the restart, `GET
/api/companies/:companyId/heartbeat-runs` failed in the per-row run
privacy check (`canActorReadHeartbeatRun`). The error was `Failed query:
select "id" from "issues" ...`. Its cause was `write CONNECTION_ENDED
<host>:5432`. postgres.js raises this code only for a query that starts
after the process called `sql.end()`. The error handler sent the error
to Sentry as a crash.

**Expected behavior**

A query that runs after the shutdown ends the pool is an expected part
of the shutdown. The server must not report it as a crash. If the
response is still open, the server answers `503`. The server must
continue to report all other errors.

**Steps to reproduce**

1. Start the server, and send a slow request (for example, a large
heartbeat run list).
2. Close the client connection, or let the 5 second drain timeout close
it, while the handler still runs.
3. Send SIGTERM. The shutdown drains HTTP and ends the pool.
4. The handler's next query fails with `CONNECTION_ENDED`. Before this
change, the error goes to Sentry.

## What Changed

- `server/src/database-retry.ts`: Add `isEndedDbPoolError`. It finds the
postgres.js `CONNECTION_ENDED` code anywhere in the error `cause` chain.
`isTransientDbConnectionError` now uses the same chain walk. Its
behavior does not change.
- `server/src/index.ts`: The orderly shutdown sets
`app.locals.paperclipDatabaseClosing = true` immediately before it ends
the database pools.
- `server/src/middleware/error-handler.ts`: When the flag is set and the
error is `CONNECTION_ENDED`, the handler answers `503 { error: "Server
is shutting down" }`, or it ends the response if the headers are already
sent. It does not call Sentry or crash telemetry. This follows the
existing client-abort (`ECONNRESET`) case.
- `server/src/services/public-mcp/capabilities.ts` and
`server/src/app.ts`: `createMcpApiDispatch` takes the main app's locals.
Each per-call dispatch app reads `paperclipDatabaseClosing` through a
live getter, so a public MCP call that outlives the pool is handled the
same way (Greptile review finding).
- `server/src/__tests__/error-handler.test.ts`: Add tests for the 503
path, the headers-sent path, `CONNECTION_ENDED` outside shutdown (still
reported), and `CONNECTION_CLOSED` during shutdown (still reported).
- `server/src/__tests__/public-mcp-dispatch-shutdown.test.ts`: Send an
ended-pool error through the real MCP dispatch, both during shutdown
(503, not reported) and outside shutdown (reported).

## Verification

- `cd server && vitest run
src/__tests__/public-mcp-dispatch-shutdown.test.ts
src/__tests__/error-handler.test.ts
src/__tests__/cloud-tenant-transient-db-retry.test.ts`: 25 tests pass.
- With only the error-handler change reverted, the two new error-handler
shutdown tests fail. With only the dispatch change reverted, the MCP
shutdown test fails. The negative-control tests pass in both cases.
- I could not run the full `pnpm -r typecheck`, `pnpm test:run`, and
`pnpm build` locally, or `public-mcp.test.ts`, which needs built
workspace packages. The sandbox has a 4 GB memory limit. CI runs the
full gates.

## Risks

- Low risk. The skip applies only to `CONNECTION_ENDED`, and only after
the shutdown starts to end the pools. postgres.js raises that code only
for a pool that this process ended. Network and database failures
(`CONNECTION_CLOSED`, `CONNECTION_DESTROYED`, and others) are still
reported.
- `CONNECTION_ENDED` outside shutdown (for example, a dedicated client
used after it ended) is still reported as a crash.
- The MCP dispatch app gets only the shutdown flag from the main app's
locals. It does not get any other locals.
- No schema, API contract, or UI change.

## Model Used

Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool
use (repository inspection, shell, test execution) and the Sentry and
GitHub connectors.

## 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-13 18:26:03 -07:00

325 lines
11 KiB
TypeScript

import { AgentLifecycleConflict } from "../modules/agent-lifecycle/index.js";
import type { Request, Response, NextFunction } from "express";
import type { Db } from "@paperclipai/db";
import { ZodError } from "zod";
import { HttpError } from "../errors.js";
import { trackErrorHandlerCrash } from "@paperclipai/shared/telemetry";
import { getTelemetryClient } from "../telemetry.js";
import { captureException } from "../sentry.js";
import { COMPANY_IMPORT_API_PATH } from "../routes/company-import-paths.js";
import { logger } from "./logger.js";
import { isSecretSensitiveHttpRequest } from "./http-log-policy.js";
import {
collectSensitiveStringValues,
redactSensitiveValueOccurrences,
} from "./redact-sensitive.js";
import { recordResponsibleUserDenialOnActiveRun } from "../services/responsible-user-denial-run-outcomes.js";
import { isExpectedMcpConnectionFailure } from "../services/mcp-connection-failure.js";
import { isEndedDbPoolError } from "../database-retry.js";
export interface ErrorContext {
error: {
message: string;
stack?: string;
name?: string;
details?: unknown;
raw?: unknown;
};
method: string;
url: string;
reqBody?: unknown;
reqParams?: unknown;
reqQuery?: unknown;
}
function isRedactedSkillPolicyDenial(details: Record<string, unknown> | null) {
return details?.code === "skill_policy_denied";
}
function readZodIssues(err: unknown): unknown[] | null {
if (err instanceof ZodError) return err.issues;
if (
!err ||
typeof err !== "object" ||
(err as { name?: unknown }).name !== "ZodError"
)
return null;
const issues = (err as { issues?: unknown }).issues;
return Array.isArray(issues) ? issues : null;
}
function attachErrorContext(
req: Request,
res: Response,
payload: ErrorContext["error"],
rawError?: Error,
) {
(res as any).__errorContext = {
error: payload,
method: req.method,
url: req.originalUrl,
reqBody: req.body,
reqParams: req.params,
reqQuery: req.query,
} satisfies ErrorContext;
if (rawError) {
(res as any).err = rawError;
}
}
function sanitizeSecretSensitiveError(req: Request, error: Error): Error {
if (!isSecretSensitiveHttpRequest(req.method, req.originalUrl)) return error;
const sanitized = new Error("Secret-sensitive request failed");
// Both `name` and `message` are attacker/provider-controlled properties on
// JavaScript errors. Do not preserve either on a credential-bearing route.
sanitized.name = "Error";
return sanitized;
}
function sanitizeSecretSensitiveResponse(
req: Request,
value: unknown,
): unknown {
if (!isSecretSensitiveHttpRequest(req.method, req.originalUrl)) return value;
return redactSensitiveValueOccurrences(
value,
collectSensitiveStringValues(req.body),
);
}
/** Report a server-side crash to every error sink. */
function reportCrash(error: Error, reportToSentry = true): void {
const tc = getTelemetryClient();
if (tc) trackErrorHandlerCrash(tc, { errorCode: error.name });
if (reportToSentry) captureException(error);
}
/** Set by the orderly shutdown just before it ends the database pools. */
function isDatabaseClosing(req: Request): boolean {
const locals = req.app?.locals as { paperclipDatabaseClosing?: boolean } | undefined;
return locals?.paperclipDatabaseClosing === true;
}
function getPaperclipDb(req: Request): Db | null {
const locals = req.app?.locals as { paperclipDb?: Db; db?: Db } | undefined;
return locals?.paperclipDb ?? locals?.db ?? null;
}
function recordResponsibleUserDenialFromHttpError(
req: Request,
details: Record<string, unknown> | null,
) {
if (req.actor?.type !== "agent") return;
const db = getPaperclipDb(req);
if (!db) return;
void recordResponsibleUserDenialOnActiveRun(db, {
runId: req.actor.runId ?? null,
agentId: req.actor.agentId ?? null,
companyId: req.actor.companyId ?? null,
code: details?.code,
}).catch((recordErr) => {
logger.warn(
{
err: recordErr,
runId: req.actor?.runId ?? null,
agentId:
req.actor?.type === "agent" ? (req.actor.agentId ?? null) : null,
},
"failed to record responsible-user denial on heartbeat run",
);
});
}
export function errorHandler(
err: unknown,
req: Request,
res: Response,
_next: NextFunction,
) {
if (err instanceof AgentLifecycleConflict) { res.status(409).json({ error: err.message }); return; }
if (err instanceof HttpError) {
const details =
err.details &&
typeof err.details === "object" &&
!Array.isArray(err.details)
? (err.details as Record<string, unknown>)
: null;
const redactedSkillPolicyDenial = isRedactedSkillPolicyDenial(details);
const workspaceRepairPreconditionFailure =
details?.code === "workspace_repair_precondition_failed";
const structuredConnectionError = new Set([
"user_authorization_required",
"organization_authorization_required",
"grant_audience_denied",
"grant_revoked",
"needs_reauthorization",
"installation_required",
"connection_not_installed",
"subject_not_permitted",
"standing_delegation_required",
"grant_owner_membership_inactive",
]).has(typeof details?.code === "string" ? details.code : "");
const responseDetailsValue = sanitizeSecretSensitiveResponse(
req,
err.details,
);
const responseDetails =
responseDetailsValue &&
typeof responseDetailsValue === "object" &&
!Array.isArray(responseDetailsValue)
? (responseDetailsValue as Record<string, unknown>)
: null;
recordResponsibleUserDenialFromHttpError(req, details);
if (err.status >= 500) {
const reportableError = sanitizeSecretSensitiveError(req, err);
attachErrorContext(
req,
res,
isSecretSensitiveHttpRequest(req.method, req.originalUrl)
? { message: reportableError.message, name: reportableError.name }
: {
message: err.message,
stack: err.stack,
name: err.name,
details: err.details,
},
reportableError,
);
reportCrash(reportableError, !isExpectedMcpConnectionFailure(err));
}
const secretSensitiveServerError =
err.status >= 500 &&
isSecretSensitiveHttpRequest(req.method, req.originalUrl);
res.status(err.status).json(
secretSensitiveServerError
? { error: "Internal server error" }
: {
error: sanitizeSecretSensitiveResponse(req, err.message),
...(typeof responseDetails?.code === "string"
? { code: responseDetails.code }
: {}),
...(redactedSkillPolicyDenial &&
typeof responseDetails?.reason === "string"
? { reason: responseDetails.reason }
: {}),
...(workspaceRepairPreconditionFailure &&
typeof responseDetails?.reason === "string"
? { reason: responseDetails.reason }
: {}),
...(workspaceRepairPreconditionFailure &&
typeof responseDetails?.repairPhase === "string"
? { repairPhase: responseDetails.repairPhase }
: {}),
...(typeof responseDetails?.remediation === "string" ||
(structuredConnectionError &&
responseDetails?.remediation &&
typeof responseDetails.remediation === "object")
? { remediation: responseDetails.remediation }
: {}),
...(structuredConnectionError && responseDetails?.connection
? { connection: responseDetails.connection }
: {}),
...(structuredConnectionError && responseDetails?.subject
? { subject: responseDetails.subject }
: {}),
...(structuredConnectionError &&
typeof responseDetails?.grantId === "string"
? { grantId: responseDetails.grantId }
: {}),
...(!redactedSkillPolicyDenial &&
!workspaceRepairPreconditionFailure &&
responseDetailsValue
? { details: responseDetailsValue }
: {}),
},
);
return;
}
const zodIssues = readZodIssues(err);
if (zodIssues) {
res.status(400).json({
error: "Validation error",
details: sanitizeSecretSensitiveResponse(req, zodIssues),
});
return;
}
// Only body-parser's malformed-JSON errors are client input failures.
// Parser messages can quote request bytes; return a constant response and
// keep the raw error out of crash reporting and HTTP error context.
if (
err instanceof SyntaxError &&
"status" in err && err.status === 400 &&
"type" in err && err.type === "entity.parse.failed"
) {
res.status(400).json({ error: "Invalid JSON body" });
return;
}
const rootError = err instanceof Error ? err : new Error(String(err));
// The client tore down the connection mid-request (closed tab, dropped
// mobile network, cancelled upload): Node surfaces it as `Error: aborted`
// with ECONNRESET. There is no server fault to report and nobody left to
// answer, so skip the error sinks and just close out the response.
if (
rootError.message === "aborted" &&
(rootError as NodeJS.ErrnoException).code === "ECONNRESET"
) {
if (!res.headersSent) res.status(499);
res.end();
return;
}
// Shutdown drains the HTTP listener before it ends the pool, but a handler
// whose client already disconnected (or whose socket the drain timeout
// closed) keeps running. Its next query then fails with CONNECTION_ENDED
// against the pool this process deliberately ended. That is shutdown, not a
// server fault, so skip the error sinks. Any other error during shutdown is
// still reported.
if (isDatabaseClosing(req) && isEndedDbPoolError(rootError)) {
if (!res.headersSent) {
res.status(503).json({ error: "Server is shutting down" });
} else {
res.end();
}
return;
}
const reportableError = sanitizeSecretSensitiveError(req, rootError);
attachErrorContext(
req,
res,
isSecretSensitiveHttpRequest(req.method, req.originalUrl)
? { message: reportableError.message, name: reportableError.name }
: err instanceof Error
? { message: err.message, stack: err.stack, name: err.name }
: {
message: String(err),
raw: err,
stack: rootError.stack,
name: rootError.name,
},
reportableError,
);
reportCrash(reportableError);
res.status(500).json({
error: "Internal server error",
...(shouldExposeTrustedCloudTenantImportError(req)
? { message: rootError.message }
: {}),
});
}
function shouldExposeTrustedCloudTenantImportError(req: Request) {
return (
req.actor?.source === "cloud_tenant" &&
req.method === "POST" &&
req.originalUrl.split("?")[0] === COMPANY_IMPORT_API_PATH
);
}