Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/cli/src/lib/api/chunk-upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,8 @@ export async function uploadMissingChunks(params: {
}

const limit = pLimit(serverOptions.concurrency);
const { fetch: authFetch } = getSdkConfig(regionUrl);
// Chunks are stored by SHA-1, so a replayed upload is a no-op.
const { fetch: authFetch } = getSdkConfig(regionUrl, { idempotent: true });

await Promise.all(
missingChunks.map((chunk) =>
Expand Down Expand Up @@ -487,7 +488,8 @@ export async function uploadMissingBufferChunks(params: {
}

const limit = pLimit(serverOptions.concurrency);
const { fetch: authFetch } = getSdkConfig(regionUrl);
// Chunks are stored by SHA-1, so a replayed upload is a no-op.
const { fetch: authFetch } = getSdkConfig(regionUrl, { idempotent: true });

await Promise.all(
missingChunks.map((chunk) =>
Expand Down Expand Up @@ -550,6 +552,8 @@ export async function pollAssembly(params: {
method: "POST",
body,
schema,
// Assembly state is keyed by checksum; re-sending only polls it.
idempotent: true,
},
);

Expand Down
3 changes: 3 additions & 0 deletions packages/cli/src/lib/api/dart-symbols.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ export async function uploadDartSymbolMap(
method: "POST",
body: assembleBody,
schema: DifAssembleResponseSchema,
// Assembly state is keyed by checksum; re-sending only polls it.
idempotent: true,
},
);

Expand Down Expand Up @@ -194,6 +196,7 @@ export async function uploadDartSymbolMap(
method: "POST",
body: assembleBody,
schema: DifAssembleResponseSchema,
idempotent: true,
},
);

Expand Down
8 changes: 7 additions & 1 deletion packages/cli/src/lib/api/debug-files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,16 @@ async function postAssemble(
endpoint: string,
body: unknown,
): Promise<DifAssembleResponse> {
// Assembly state is keyed by checksum; re-sending only polls it.
const { data } = await apiRequestToRegion<DifAssembleResponse>(
regionUrl,
endpoint,
{ method: "POST", body, schema: DifAssembleResponseSchema },
{
method: "POST",
body,
schema: DifAssembleResponseSchema,
idempotent: true,
},
);
return data;
}
Expand Down
7 changes: 7 additions & 0 deletions packages/cli/src/lib/api/infrastructure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,11 @@ export type ApiRequestOptions<T = unknown> = {
credential?: CredentialContext;
/** Manually validate each redirect before forwarding credentials. */
validatedRedirects?: boolean;
/**
* Retry transient failures even for writes. Only for requests the server
* dedupes (see `idempotent` in sentry-client.ts).
*/
idempotent?: boolean;
};

/**
Expand Down Expand Up @@ -774,11 +779,13 @@ export async function apiRequestToRegion<T>(
schema,
credential,
validatedRedirects,
idempotent,
} = options;
const config = getSdkConfig(regionUrl, {
credential,
validatedRedirects,
bufferResponseBody: true,
idempotent,
});

const endpointWithParams = appendSearchParams(endpoint, params);
Expand Down
3 changes: 3 additions & 0 deletions packages/cli/src/lib/api/proguard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ export async function uploadProguardMappings(
method: "POST",
body: assembleBody,
schema: DifAssembleResponseSchema,
// Assembly state is keyed by checksum; re-sending only polls it.
idempotent: true,
Comment thread
BYK marked this conversation as resolved.
Resolved
},
);

Expand Down Expand Up @@ -235,6 +237,7 @@ async function pollDifAssembly(params: {
method: "POST",
body,
schema: DifAssembleResponseSchema,
idempotent: true,
},
);

Expand Down
2 changes: 2 additions & 0 deletions packages/cli/src/lib/api/sourcemaps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,8 @@ async function uploadArtifactBundle(opts: {
method: "POST",
body: assembleBody,
schema: AssembleResponseSchema,
// Assembly state is keyed by checksum; re-sending only polls it.
idempotent: true,
},
);

Expand Down
29 changes: 29 additions & 0 deletions packages/cli/src/lib/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,35 @@ export class ApiError extends CliError {
}
}

/**
* The Sentry API rejected a request with 429 and the CLI could not wait it
* out (the wait exceeds the cap, or retries ran out).
*
* @param endpoint - API path that was rate limited
* @param retryAfterMs - Server-requested wait, when known
* @param serverDetail - The API's `detail` message, when present
*/
export class RateLimitError extends ApiError {
readonly retryAfterMs?: number;

constructor(endpoint: string, retryAfterMs?: number, serverDetail?: string) {
const seconds = Math.max(1, Math.ceil((retryAfterMs ?? 0) / 1000));
const wait =
retryAfterMs === undefined
? "a few seconds"
: `${seconds} second${seconds === 1 ? "" : "s"}`;
const advice = `Retry in ${wait}; do not repeat the request before then. Make fewer requests in parallel.`;
Comment thread
sentry-junior[bot] marked this conversation as resolved.
Resolved
super(
"Rate limited by the Sentry API",
429,
serverDetail ? `${serverDetail}\n ${advice}` : advice,
endpoint,
);
this.name = "RateLimitError";
this.retryAfterMs = retryAfterMs;
}
}

export type AuthErrorReason = "not_authenticated" | "expired" | "invalid";

/** Options for AuthError */
Expand Down
160 changes: 160 additions & 0 deletions packages/cli/src/lib/retry-policy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
/**
* Retry policy for Sentry API requests.
*
* Decides whether a failed request may be replayed automatically and how
* long to wait first. Kept pure (no CLI imports, no I/O) so it can be swapped
* for the shared toolkit-core policy without touching callers.
*
* Only idempotent methods are replayed by default. A transient status, a
* network error, or a timeout after a POST does not tell us whether the
* server already applied it, so replaying could duplicate the write.
* PUT and DELETE are excluded too: Sentry uses PUT for partial updates with
* side effects (issue merges, activity, notifications), and a replayed
* DELETE turns a successful delete into a 404. Callers that know a write is
* safe to replay (content-addressed uploads) opt in per request.
*
* A 429 is the exception and retries for any method: Sentry's rate-limit
* middleware rejects the request before the view runs, so nothing was
* applied.
*/

/**
* HTTP statuses that indicate a transient failure worth retrying. Not a
* blanket 5xx: 501, 505 and similar are deterministic.
*/
const RETRYABLE_STATUS_CODES: ReadonlySet<number> = new Set([
408, 429, 500, 502, 503, 504,
]);

/** Methods that are safe to replay without an explicit opt-in. */
const IDEMPOTENT_METHODS: ReadonlySet<string> = new Set([
Comment thread
sentry-junior[bot] marked this conversation as resolved.
Resolved
"GET",
"HEAD",
"OPTIONS",
]);

/** First fallback backoff step; doubles per attempt up to the cap. */
const BASE_BACKOFF_MS = 1000;

/**
* Whether a request may be replayed after a transient status, network
* error, or timeout.
*
* @param method - HTTP method (any case)
* @param idempotent - Caller asserts the request is safe to replay
*/
export function isRetryableMethod(method: string, idempotent = false): boolean {
return idempotent || IDEMPOTENT_METHODS.has(method.toUpperCase());
}

/**
* Jittered exponential backoff: a random delay in the upper half of
* `min(1s * 2^attempt, maxDelayMs)`, so parallel requests spread out
* without retrying sooner than half the nominal step.
*/
export function backoffDelayMs(
attempt: number,
maxDelayMs: number,
random: () => number = Math.random,
): number {
const ceiling = Math.min(BASE_BACKOFF_MS * 2 ** attempt, maxDelayMs);
return Math.round(ceiling / 2 + random() * (ceiling / 2));
}

function parseRetryAfterMs(value: string, referenceMs: number): number {
const trimmed = value.trim();
if (/^\d+$/.test(trimmed)) {
return Number(trimmed) * 1000;
}
return Date.parse(trimmed) - referenceMs;
}

/**
* Delay requested by the server before retrying, in milliseconds.
*
* Reads `Retry-After` (delta seconds or HTTP date) on any status, and on 429
* also `X-Sentry-Rate-Limit-Reset` (Unix seconds when the window resets).
* The reset header is ignored when `X-Sentry-Rate-Limit-Remaining` is
* non-zero: that 429 came from the concurrency limit, where the window reset
* is unrelated and jittered backoff is the right wait.
*
* Absolute times are measured against the response `Date` header when
* present, so a skewed local clock cannot turn a 1s window into a long wait
* or an immediate retry. Times already in the past are dropped.
*
* @returns The longest requested wait, or undefined when the server gave
* no usable hint
*/
export function serverRetryDelayMs(
status: number,
headers: Headers,
nowMs: number,
): number | undefined {
const serverDate = Date.parse(headers.get("date") ?? "");
const referenceMs = Number.isNaN(serverDate) ? nowMs : serverDate;
const candidates: number[] = [];

const retryAfter = headers.get("retry-after");
if (retryAfter) {
candidates.push(parseRetryAfterMs(retryAfter, referenceMs));
}

const remaining = headers.get("x-sentry-rate-limit-remaining");
const reset = headers.get("x-sentry-rate-limit-reset");
if (
status === 429 &&
reset &&
(remaining === null || Number(remaining) <= 0)
) {
candidates.push(Number(reset) * 1000 - referenceMs);
}

const valid = candidates.filter((ms) => Number.isFinite(ms) && ms >= 0);
return valid.length > 0 ? Math.max(...valid) : undefined;
}

/** Outcome of {@link decideStatusRetry}. */
export type RetryDecision =
| { retry: true; delayMs: number }
| {
retry: false;
/** Server-requested wait, when known; tells the user when to retry. */
retryAfterMs?: number;
};

/**
* Decide whether to retry a response with the given status.
*
* Retries retryable statuses while attempts remain and the server-requested
* wait fits within `maxDelayMs`. A 429 retries for any method; other
* statuses only when `methodRetryable`. Without a server hint, waits a
* jittered exponential backoff.
*/
export function decideStatusRetry(params: {
status: number;
headers: Headers;
methodRetryable: boolean;
attempt: number;
maxRetries: number;
maxDelayMs: number;
nowMs: number;
random?: () => number;
}): RetryDecision {
const { status, headers, methodRetryable, attempt, maxRetries, maxDelayMs } =
params;
if (!RETRYABLE_STATUS_CODES.has(status)) {
return { retry: false };
}
const serverDelay = serverRetryDelayMs(status, headers, params.nowMs);
if (
(!methodRetryable && status !== 429) ||
attempt >= maxRetries ||
(serverDelay !== undefined && serverDelay > maxDelayMs)
) {
return { retry: false, retryAfterMs: serverDelay };
}
return {
retry: true,
delayMs: serverDelay ?? backoffDelayMs(attempt, maxDelayMs, params.random),
};
}
Loading
Loading