Skip to content

Commit 899a008

Browse files
committed
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 <Code>/<RequestId> 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.
1 parent 4f08b76 commit 899a008

3 files changed

Lines changed: 144 additions & 7 deletions

File tree

src/context/Uploader.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,79 @@ describe('UploaderImpl', () => {
9696
});
9797
});
9898

99+
test('S3 PUT XML error → parses s3Code and requestId', async () => {
100+
const xml =
101+
'<?xml version="1.0" encoding="UTF-8"?><Error><Code>AccessDenied</Code>' +
102+
'<Message>Access Denied</Message><RequestId>ABC123XYZ</RequestId>' +
103+
'<HostId>hostid==</HostId></Error>';
104+
const { fetch } = recordFetch([presignResponse(), new Response(xml, { status: 403 })]);
105+
const result = await new UploaderImpl({ endpoint: ENDPOINT, fetch }).upload(uploadInput.build());
106+
107+
expect(result.isErr()).toBe(true);
108+
const err = result._unsafeUnwrapErr();
109+
expect(err).toMatchObject({ kind: 's3-bad-status', status: 403, s3Code: 'AccessDenied', requestId: 'ABC123XYZ' });
110+
});
111+
112+
test('retries a transient S3 5xx, then succeeds', async () => {
113+
const { fetch, calls } = recordFetch([
114+
presignResponse(),
115+
new Response('<Error><Code>SlowDown</Code></Error>', { status: 503 }),
116+
new Response('', { status: 200 }),
117+
]);
118+
const result = await new UploaderImpl({ endpoint: ENDPOINT, fetch, retryMinTimeoutMs: 0 }).upload(
119+
uploadInput.build(),
120+
);
121+
122+
expect(result.isOk()).toBe(true);
123+
expect(calls.length).toBe(3); // presign + 2 PUT attempts
124+
});
125+
126+
test('retries a network failure during PUT, then succeeds', async () => {
127+
const responses = [presignResponse(), 'throw' as const, new Response('', { status: 200 })];
128+
let calls = 0;
129+
const fetchFn: FetchFn = () => {
130+
const next = responses[calls++];
131+
if (next === 'throw') return Promise.reject(new Error('econnreset'));
132+
return Promise.resolve(next as Response);
133+
};
134+
const result = await new UploaderImpl({ endpoint: ENDPOINT, fetch: fetchFn, retryMinTimeoutMs: 0 }).upload(
135+
uploadInput.build(),
136+
);
137+
138+
expect(result.isOk()).toBe(true);
139+
expect(calls).toBe(3); // presign + failed PUT + retried PUT
140+
});
141+
142+
test('does not retry a non-transient 403 from S3', async () => {
143+
const { fetch, calls } = recordFetch([presignResponse(), new Response('access denied', { status: 403 })]);
144+
const result = await new UploaderImpl({ endpoint: ENDPOINT, fetch, retryMinTimeoutMs: 0 }).upload(
145+
uploadInput.build(),
146+
);
147+
148+
expect(result.isErr()).toBe(true);
149+
expect(result._unsafeUnwrapErr().kind).toBe('s3-bad-status');
150+
expect(calls.length).toBe(2); // presign + single PUT, no retry
151+
});
152+
153+
test('exhausts attempts on a persistent transient failure', async () => {
154+
const { fetch, calls } = recordFetch([
155+
presignResponse(),
156+
new Response('<Error><Code>InternalError</Code></Error>', { status: 500 }),
157+
new Response('<Error><Code>InternalError</Code></Error>', { status: 500 }),
158+
]);
159+
const result = await new UploaderImpl({
160+
endpoint: ENDPOINT,
161+
fetch,
162+
maxAttempts: 2,
163+
retryMinTimeoutMs: 0,
164+
}).upload(uploadInput.build());
165+
166+
expect(result.isErr()).toBe(true);
167+
const err = result._unsafeUnwrapErr();
168+
expect(err).toMatchObject({ kind: 's3-bad-status', status: 500, s3Code: 'InternalError' });
169+
expect(calls.length).toBe(3); // presign + 2 PUT attempts
170+
});
171+
99172
test('network failure during presign → presign-request-failed', async () => {
100173
const fetchFn: FetchFn = () => Promise.reject(new Error('econnreset'));
101174
const result = await new UploaderImpl({ endpoint: ENDPOINT, fetch: fetchFn }).upload(uploadInput.build());

src/context/Uploader.ts

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ export type UploadError =
88
| { kind: 'presign-bad-status'; status: number; body: string }
99
| { kind: 'presign-bad-response'; message: string }
1010
| { kind: 's3-put-failed'; message: string }
11-
| { kind: 's3-bad-status'; status: number; body: string };
11+
| { kind: 's3-bad-status'; status: number; body: string; s3Code?: string; requestId?: string };
1212

1313
export interface UploadInput {
1414
readonly bytes: Uint8Array;
@@ -31,6 +31,8 @@ export type FetchFn = (url: string, init?: RequestInit) => Promise<Response>;
3131
interface UploaderImplOptions {
3232
readonly endpoint?: string;
3333
readonly fetch?: FetchFn;
34+
readonly maxAttempts?: number;
35+
readonly retryMinTimeoutMs?: number;
3436
}
3537

3638
interface PresignResponse {
@@ -42,18 +44,37 @@ interface PresignResponse {
4244
export class UploaderImpl implements Uploader {
4345
readonly #endpoint: string;
4446
readonly #fetch: FetchFn;
47+
readonly #maxAttempts: number;
48+
readonly #retryMinTimeoutMs: number;
4549

4650
constructor(options: UploaderImplOptions = {}) {
4751
this.#endpoint = options.endpoint ?? DEFAULT_UPLOAD_ENDPOINT;
4852
this.#fetch = options.fetch ?? fetch;
53+
this.#maxAttempts = options.maxAttempts ?? 4;
54+
this.#retryMinTimeoutMs = options.retryMinTimeoutMs ?? 500;
4955
}
5056

5157
upload(input: UploadInput): ResultAsync<UploadResult, UploadError> {
5258
return this.#requestPresign(input).andThen((presign) =>
53-
this.#putToS3(presign.presignedUrl, input.bytes).map(() => ({ uploadId: presign.uploadId })),
59+
this.#putToS3WithRetry(presign.presignedUrl, input.bytes).map(() => ({ uploadId: presign.uploadId })),
5460
);
5561
}
5662

63+
// The bytes are buffered in memory, so the PUT is safely replayable. Retry only
64+
// the transient S3 outcomes (RequestTimeout/SlowDown/5xx, network errors) —
65+
// see isRetryableUploadError; 4xx signature/permission failures fail fast.
66+
// Backoff is exponential from retryMinTimeoutMs; attempts cap at maxAttempts.
67+
#putToS3WithRetry(url: string, bytes: Uint8Array, attempt = 1): ResultAsync<void, UploadError> {
68+
return this.#putToS3(url, bytes).orElse((err) => {
69+
if (attempt >= this.#maxAttempts || !isRetryableUploadError(err)) {
70+
return errAsync<void, UploadError>(err);
71+
}
72+
return delay(this.#retryMinTimeoutMs * 2 ** (attempt - 1)).andThen(() =>
73+
this.#putToS3WithRetry(url, bytes, attempt + 1),
74+
);
75+
});
76+
}
77+
5778
#requestPresign(input: UploadInput): ResultAsync<PresignResponse, UploadError> {
5879
const body = JSON.stringify({
5980
owner: input.owner,
@@ -95,7 +116,7 @@ export class UploaderImpl implements Uploader {
95116
).andThen((res) => {
96117
if (!res.ok) {
97118
return ResultAsync.fromSafePromise(res.text().catch(() => '')).andThen((text) =>
98-
errAsync<void, UploadError>({ kind: 's3-bad-status', status: res.status, body: text }),
119+
errAsync<void, UploadError>({ kind: 's3-bad-status', status: res.status, body: text, ...parseS3Error(text) }),
99120
);
100121
}
101122
return okAsync<void, UploadError>(undefined);
@@ -126,6 +147,48 @@ function isObject(value: unknown): value is Record<string, unknown> {
126147
return typeof value === 'object' && value !== null && !Array.isArray(value);
127148
}
128149

150+
const delay = (ms: number): ResultAsync<void, never> =>
151+
ResultAsync.fromSafePromise(new Promise<void>((resolve) => setTimeout(resolve, ms)));
152+
153+
function isRetryableUploadError(err: UploadError): boolean {
154+
switch (err.kind) {
155+
case 's3-put-failed':
156+
return true;
157+
case 's3-bad-status':
158+
return err.status === 429 || err.status >= 500 || err.s3Code === 'RequestTimeout';
159+
default:
160+
return false;
161+
}
162+
}
163+
164+
// S3 errors are XML; we pull the machine-readable <Code> and <RequestId> (cause + AWS
165+
// support handle) but drop the body, which echoes the owner/email object key.
166+
function parseS3Error(body: string): { s3Code?: string; requestId?: string } {
167+
const s3Code = body.match(/<Code>([^<]+)<\/Code>/)?.[1];
168+
const requestId = body.match(/<RequestId>([^<]+)<\/RequestId>/)?.[1];
169+
return { ...(s3Code ? { s3Code } : {}), ...(requestId ? { requestId } : {}) };
170+
}
171+
172+
// Privacy-safe telemetry: omits the raw S3 body and presigned URL (both embed the
173+
// owner/email object key). Network error messages are safe to include.
174+
export function uploadErrorTelemetry(err: UploadError): Record<string, string | number> {
175+
switch (err.kind) {
176+
case 'presign-request-failed':
177+
case 'presign-bad-response':
178+
case 's3-put-failed':
179+
return { error_kind: err.kind, error_message: err.message.slice(0, 300) };
180+
case 'presign-bad-status':
181+
return { error_kind: err.kind, status: err.status };
182+
case 's3-bad-status':
183+
return {
184+
error_kind: err.kind,
185+
status: err.status,
186+
...(err.s3Code ? { s3_code: err.s3Code } : {}),
187+
...(err.requestId ? { request_id: err.requestId } : {}),
188+
};
189+
}
190+
}
191+
129192
export function formatUploadError(err: UploadError): string {
130193
switch (err.kind) {
131194
case 'presign-request-failed':
@@ -137,6 +200,6 @@ export function formatUploadError(err: UploadError): string {
137200
case 's3-put-failed':
138201
return `failed to upload to S3: ${err.message}`;
139202
case 's3-bad-status':
140-
return `S3 returned ${err.status}: ${err.body || '(empty body)'}`;
203+
return `S3 returned ${err.status}${err.s3Code ? ` (${err.s3Code})` : ''}: ${err.body || '(empty body)'}`;
141204
}
142205
}

src/interactive/sharePrompt.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { Context } from '../context/index.ts';
22
import { type Prompter, formatPromptError } from '../context/Prompter.ts';
3-
import { formatUploadError } from '../context/Uploader.ts';
3+
import { formatUploadError, uploadErrorTelemetry } from '../context/Uploader.ts';
44

55
const SUPPORT_LINE = 'Reach us at founders@contextbridge.ai — or learn more at https://patchwave.ai';
66

@@ -41,7 +41,7 @@ export function showLocalReportReadyNotice(inputs: ReportReadyNoticeInputs): voi
4141
}
4242

4343
export async function runSharePrompt(inputs: SharePromptInputs): Promise<ShareOutcome> {
44-
const { prompter, analytics, uploader } = inputs.context;
44+
const { prompter, analytics, uploader, logger } = inputs.context;
4545

4646
prompter.note([`Scanned: ${inputs.target}`, `HTML report: ${inputs.htmlPath}`].join('\n'), 'Report ready');
4747

@@ -92,7 +92,8 @@ export async function runSharePrompt(inputs: SharePromptInputs): Promise<ShareOu
9292
if (uploadResult.isErr()) {
9393
const message = formatUploadError(uploadResult.error);
9494
spinner.stop('Upload failed.');
95-
analytics.capture('upload_failed', { error_kind: uploadResult.error.kind });
95+
analytics.capture('upload_failed', uploadErrorTelemetry(uploadResult.error));
96+
logger.warn({ err: uploadResult.error }, 'Analysis upload failed');
9697
prompter.error(message);
9798
prompter.note(
9899
[`Your local report is unchanged:`, ` ${inputs.htmlPath}`, '', SUPPORT_LINE].join('\n'),

0 commit comments

Comments
 (0)