From 899a008642ab8bebe10a0e9bac7013e14fc6e410 Mon Sep 17 00:00:00 2001 From: Ben Limmer Date: Fri, 26 Jun 2026 14:02:04 -0600 Subject: [PATCH] feat: retry transient S3 upload failures with richer telemetry The analysis-bundle PUT goes straight to a presigned S3 URL via raw fetch, so nothing retries it on a transient blip. Retry the transient outcomes (429/5xx/RequestTimeout, network errors) with exponential backoff, capped at 4 attempts, while failing fast on 4xx signature/permission errors. Parse the S3 XML error / and surface them through a privacy-safe uploadErrorTelemetry() (never the raw body or presigned URL, which embed the owner/email object key). sharePrompt now reports that detail to analytics and logs the failure for o11y. --- src/context/Uploader.test.ts | 73 ++++++++++++++++++++++++++++++++++ src/context/Uploader.ts | 71 +++++++++++++++++++++++++++++++-- src/interactive/sharePrompt.ts | 7 ++-- 3 files changed, 144 insertions(+), 7 deletions(-) diff --git a/src/context/Uploader.test.ts b/src/context/Uploader.test.ts index cf15363..c8ea165 100644 --- a/src/context/Uploader.test.ts +++ b/src/context/Uploader.test.ts @@ -96,6 +96,79 @@ describe('UploaderImpl', () => { }); }); + test('S3 PUT XML error → parses s3Code and requestId', async () => { + const xml = + 'AccessDenied' + + 'Access DeniedABC123XYZ' + + 'hostid=='; + const { fetch } = recordFetch([presignResponse(), new Response(xml, { status: 403 })]); + const result = await new UploaderImpl({ endpoint: ENDPOINT, fetch }).upload(uploadInput.build()); + + expect(result.isErr()).toBe(true); + const err = result._unsafeUnwrapErr(); + expect(err).toMatchObject({ kind: 's3-bad-status', status: 403, s3Code: 'AccessDenied', requestId: 'ABC123XYZ' }); + }); + + test('retries a transient S3 5xx, then succeeds', async () => { + const { fetch, calls } = recordFetch([ + presignResponse(), + new Response('SlowDown', { status: 503 }), + new Response('', { status: 200 }), + ]); + const result = await new UploaderImpl({ endpoint: ENDPOINT, fetch, retryMinTimeoutMs: 0 }).upload( + uploadInput.build(), + ); + + expect(result.isOk()).toBe(true); + expect(calls.length).toBe(3); // presign + 2 PUT attempts + }); + + test('retries a network failure during PUT, then succeeds', async () => { + const responses = [presignResponse(), 'throw' as const, new Response('', { status: 200 })]; + let calls = 0; + const fetchFn: FetchFn = () => { + const next = responses[calls++]; + if (next === 'throw') return Promise.reject(new Error('econnreset')); + return Promise.resolve(next as Response); + }; + const result = await new UploaderImpl({ endpoint: ENDPOINT, fetch: fetchFn, retryMinTimeoutMs: 0 }).upload( + uploadInput.build(), + ); + + expect(result.isOk()).toBe(true); + expect(calls).toBe(3); // presign + failed PUT + retried PUT + }); + + test('does not retry a non-transient 403 from S3', async () => { + const { fetch, calls } = recordFetch([presignResponse(), new Response('access denied', { status: 403 })]); + const result = await new UploaderImpl({ endpoint: ENDPOINT, fetch, retryMinTimeoutMs: 0 }).upload( + uploadInput.build(), + ); + + expect(result.isErr()).toBe(true); + expect(result._unsafeUnwrapErr().kind).toBe('s3-bad-status'); + expect(calls.length).toBe(2); // presign + single PUT, no retry + }); + + test('exhausts attempts on a persistent transient failure', async () => { + const { fetch, calls } = recordFetch([ + presignResponse(), + new Response('InternalError', { status: 500 }), + new Response('InternalError', { status: 500 }), + ]); + const result = await new UploaderImpl({ + endpoint: ENDPOINT, + fetch, + maxAttempts: 2, + retryMinTimeoutMs: 0, + }).upload(uploadInput.build()); + + expect(result.isErr()).toBe(true); + const err = result._unsafeUnwrapErr(); + expect(err).toMatchObject({ kind: 's3-bad-status', status: 500, s3Code: 'InternalError' }); + expect(calls.length).toBe(3); // presign + 2 PUT attempts + }); + test('network failure during presign → presign-request-failed', async () => { const fetchFn: FetchFn = () => Promise.reject(new Error('econnreset')); const result = await new UploaderImpl({ endpoint: ENDPOINT, fetch: fetchFn }).upload(uploadInput.build()); diff --git a/src/context/Uploader.ts b/src/context/Uploader.ts index f4bb486..eec2113 100644 --- a/src/context/Uploader.ts +++ b/src/context/Uploader.ts @@ -8,7 +8,7 @@ export type UploadError = | { kind: 'presign-bad-status'; status: number; body: string } | { kind: 'presign-bad-response'; message: string } | { kind: 's3-put-failed'; message: string } - | { kind: 's3-bad-status'; status: number; body: string }; + | { kind: 's3-bad-status'; status: number; body: string; s3Code?: string; requestId?: string }; export interface UploadInput { readonly bytes: Uint8Array; @@ -31,6 +31,8 @@ export type FetchFn = (url: string, init?: RequestInit) => Promise; interface UploaderImplOptions { readonly endpoint?: string; readonly fetch?: FetchFn; + readonly maxAttempts?: number; + readonly retryMinTimeoutMs?: number; } interface PresignResponse { @@ -42,18 +44,37 @@ interface PresignResponse { export class UploaderImpl implements Uploader { readonly #endpoint: string; readonly #fetch: FetchFn; + readonly #maxAttempts: number; + readonly #retryMinTimeoutMs: number; constructor(options: UploaderImplOptions = {}) { this.#endpoint = options.endpoint ?? DEFAULT_UPLOAD_ENDPOINT; this.#fetch = options.fetch ?? fetch; + this.#maxAttempts = options.maxAttempts ?? 4; + this.#retryMinTimeoutMs = options.retryMinTimeoutMs ?? 500; } upload(input: UploadInput): ResultAsync { return this.#requestPresign(input).andThen((presign) => - this.#putToS3(presign.presignedUrl, input.bytes).map(() => ({ uploadId: presign.uploadId })), + this.#putToS3WithRetry(presign.presignedUrl, input.bytes).map(() => ({ uploadId: presign.uploadId })), ); } + // The bytes are buffered in memory, so the PUT is safely replayable. Retry only + // the transient S3 outcomes (RequestTimeout/SlowDown/5xx, network errors) — + // see isRetryableUploadError; 4xx signature/permission failures fail fast. + // Backoff is exponential from retryMinTimeoutMs; attempts cap at maxAttempts. + #putToS3WithRetry(url: string, bytes: Uint8Array, attempt = 1): ResultAsync { + return this.#putToS3(url, bytes).orElse((err) => { + if (attempt >= this.#maxAttempts || !isRetryableUploadError(err)) { + return errAsync(err); + } + return delay(this.#retryMinTimeoutMs * 2 ** (attempt - 1)).andThen(() => + this.#putToS3WithRetry(url, bytes, attempt + 1), + ); + }); + } + #requestPresign(input: UploadInput): ResultAsync { const body = JSON.stringify({ owner: input.owner, @@ -95,7 +116,7 @@ export class UploaderImpl implements Uploader { ).andThen((res) => { if (!res.ok) { return ResultAsync.fromSafePromise(res.text().catch(() => '')).andThen((text) => - errAsync({ kind: 's3-bad-status', status: res.status, body: text }), + errAsync({ kind: 's3-bad-status', status: res.status, body: text, ...parseS3Error(text) }), ); } return okAsync(undefined); @@ -126,6 +147,48 @@ function isObject(value: unknown): value is Record { return typeof value === 'object' && value !== null && !Array.isArray(value); } +const delay = (ms: number): ResultAsync => + ResultAsync.fromSafePromise(new Promise((resolve) => setTimeout(resolve, ms))); + +function isRetryableUploadError(err: UploadError): boolean { + switch (err.kind) { + case 's3-put-failed': + return true; + case 's3-bad-status': + return err.status === 429 || err.status >= 500 || err.s3Code === 'RequestTimeout'; + default: + return false; + } +} + +// S3 errors are XML; we pull the machine-readable and (cause + AWS +// support handle) but drop the body, which echoes the owner/email object key. +function parseS3Error(body: string): { s3Code?: string; requestId?: string } { + const s3Code = body.match(/([^<]+)<\/Code>/)?.[1]; + const requestId = body.match(/([^<]+)<\/RequestId>/)?.[1]; + return { ...(s3Code ? { s3Code } : {}), ...(requestId ? { requestId } : {}) }; +} + +// Privacy-safe telemetry: omits the raw S3 body and presigned URL (both embed the +// owner/email object key). Network error messages are safe to include. +export function uploadErrorTelemetry(err: UploadError): Record { + switch (err.kind) { + case 'presign-request-failed': + case 'presign-bad-response': + case 's3-put-failed': + return { error_kind: err.kind, error_message: err.message.slice(0, 300) }; + case 'presign-bad-status': + return { error_kind: err.kind, status: err.status }; + case 's3-bad-status': + return { + error_kind: err.kind, + status: err.status, + ...(err.s3Code ? { s3_code: err.s3Code } : {}), + ...(err.requestId ? { request_id: err.requestId } : {}), + }; + } +} + export function formatUploadError(err: UploadError): string { switch (err.kind) { case 'presign-request-failed': @@ -137,6 +200,6 @@ export function formatUploadError(err: UploadError): string { case 's3-put-failed': return `failed to upload to S3: ${err.message}`; case 's3-bad-status': - return `S3 returned ${err.status}: ${err.body || '(empty body)'}`; + return `S3 returned ${err.status}${err.s3Code ? ` (${err.s3Code})` : ''}: ${err.body || '(empty body)'}`; } } diff --git a/src/interactive/sharePrompt.ts b/src/interactive/sharePrompt.ts index a350eef..3a522d9 100644 --- a/src/interactive/sharePrompt.ts +++ b/src/interactive/sharePrompt.ts @@ -1,6 +1,6 @@ import type { Context } from '../context/index.ts'; import { type Prompter, formatPromptError } from '../context/Prompter.ts'; -import { formatUploadError } from '../context/Uploader.ts'; +import { formatUploadError, uploadErrorTelemetry } from '../context/Uploader.ts'; const SUPPORT_LINE = 'Reach us at founders@contextbridge.ai — or learn more at https://patchwave.ai'; @@ -41,7 +41,7 @@ export function showLocalReportReadyNotice(inputs: ReportReadyNoticeInputs): voi } export async function runSharePrompt(inputs: SharePromptInputs): Promise { - const { prompter, analytics, uploader } = inputs.context; + const { prompter, analytics, uploader, logger } = inputs.context; prompter.note([`Scanned: ${inputs.target}`, `HTML report: ${inputs.htmlPath}`].join('\n'), 'Report ready'); @@ -92,7 +92,8 @@ export async function runSharePrompt(inputs: SharePromptInputs): Promise