Files
PaperClipAI/server/src/storage/s3-failure.ts
T
515ff0f284 fix(server): report which S3 call failed in storage error events (#17424)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - The server keeps attachments and other files in a storage provider.
Hosted instances use the S3 provider.
> - Sentry received `SignatureDoesNotMatch` errors from the S3 provider
on two staging hosts.
> - The events have only AWS SDK stack frames. They do not show which S3
call failed, the HTTP status, or the S3 request ID.
> - Without these facts, we cannot find the cause or make a safe fix.
> - This pull request records a small, fixed-shape failure record on the
S3 error and sends it with the Sentry event.
> - The benefit is that the next event shows the failed operation and
the provider request ID, and no private data goes to Sentry.

## Linked Issues or Issue Description

**What happened?**
The S3 storage provider threw `SignatureDoesNotMatch` four times on
staging. The Sentry events do not identify the S3 operation or the
caller.

**Expected behavior**
A storage provider failure in Sentry shows which storage call failed and
the provider's correlation fields.

**Steps to reproduce**
1. Configure the S3 storage provider against an endpoint that rejects
the request signature.
2. Read, write, check, or delete a stored file.
3. Look at the Sentry event. It has no first-party stack frame and no
operation name.

No public GitHub issue exists. I searched open and closed pull requests
for `SignatureDoesNotMatch` and storage failure diagnostics. No pull
request covers this.

## What Changed

- Add `server/src/storage/failure-diagnostics.ts`. It keeps a frozen,
fixed-shape record for each failed storage error in a `WeakMap`.
- Add `server/src/storage/s3-failure.ts`. It wraps each S3 call and
records the operation, error code, HTTP status, request ID, and SDK
attempt count. It reads only data properties. A value that does not
match its bounded format becomes `unknown` or `null`.
- Wrap `PutObject`, `GetObject`, `HeadObject`, `DeleteObject`,
`CreateMultipartUpload`, `UploadPart`, and `CompleteMultipartUpload`.
- In `captureException`, add the `storage_operation` and
`storage_error_code` tags and a `storage_failure` context to the event
of that error only. The original error and its default grouping do not
change.
- Document the new context in `doc/observability.md`.

## Verification

- `pnpm exec vitest run
src/__tests__/storage-s3-failure-diagnostics.test.ts
src/__tests__/sentry.test.ts` (in `server/`): 37 passed, 5 skipped. The
skipped tests are the existing real-SDK tests, which need an environment
flag.
- `pnpm exec vitest run src/__tests__/storage-s3-multipart.test.ts
src/__tests__/storage-local-provider.test.ts`: 8 passed.
- The new tests use a local HTTP server that returns a real S3
`SignatureDoesNotMatch` XML body with a `StringToSign` field. The tests
make sure that the record has the operation, code, status and request
ID, and that it does not contain the private string.
- I typechecked the changed source files with a scoped `tsc` project.
They have no errors. I could not run the full `pnpm -r typecheck` and
`pnpm build` in my sandbox: it has a 4 GiB memory limit and no `cargo`
for the runner binary. CI runs the full checks.

## Risks

- Low risk. The change adds a `try`/`catch` that records data and throws
the same error again. Storage behavior, not-found handling, and HTTP
responses do not change.
- Secret-sensitive routes replace the error before reporting, so those
events do not get the record. This is intended.
- The record never contains the bucket, object key, endpoint,
credentials, error message, or provider response body.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic. Used through Claude
Code with tool use (shell, file edit) and extended reasoning.

## 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-24 05:08:56 -07:00

47 lines
1.8 KiB
TypeScript

import { recordStorageFailure, type StorageFailureOperation } from "./failure-diagnostics.js";
const ERROR_CODE = /^[A-Za-z][A-Za-z0-9._-]{0,63}$/;
const REQUEST_ID = /^[A-Za-z0-9+/=._-]{1,128}$/;
// Inspect data properties only. An SDK error can carry getters, request
// objects or provider XML; none of those are diagnostic payloads.
function ownValue(value: unknown, key: string): unknown {
if (!value || typeof value !== "object") return undefined;
try {
const descriptor = Object.getOwnPropertyDescriptor(value, key);
return descriptor && "value" in descriptor ? descriptor.value : undefined;
} catch {
return undefined;
}
}
function matching(value: unknown, pattern: RegExp): string | null {
return typeof value === "string" && pattern.test(value) ? value : null;
}
function integerIn(value: unknown, min: number, max: number): number | null {
return Number.isInteger(value) && (value as number) >= min && (value as number) <= max ? value as number : null;
}
/** Record which S3 call failed and the provider's own correlation fields. */
export function recordS3Failure(error: unknown, operation: StorageFailureOperation): void {
const metadata = ownValue(error, "$metadata");
recordStorageFailure(error, {
provider: "s3",
operation,
errorCode: matching(ownValue(error, "name"), ERROR_CODE) ?? matching(ownValue(error, "code"), ERROR_CODE) ?? "unknown",
httpStatus: integerIn(ownValue(metadata, "httpStatusCode"), 100, 599),
requestId: matching(ownValue(metadata, "requestId"), REQUEST_ID),
attempts: integerIn(ownValue(metadata, "attempts"), 1, 100),
});
}
export async function withS3FailureDiagnostics<T>(operation: StorageFailureOperation, send: () => Promise<T>): Promise<T> {
try {
return await send();
} catch (error) {
recordS3Failure(error, operation);
throw error;
}
}