From a4de2da01d61d0dfaf7974006c5b0ddcbab22a1e Mon Sep 17 00:00:00 2001 From: Loris Leiva Date: Fri, 4 Sep 2026 12:34:04 +0100 Subject: [PATCH 1/3] Retry rate-limited RPC requests in the CLI This makes the CLI resilient to RPC rate limiting (HTTP 429), which currently aborts metadata uploads part way through. Uploading an IDL fans a large payload out into many write transactions, each performing several RPC calls; against a rate-limited endpoint (public devnet in particular) those bursts trip the limiter and the whole upload fails with "HTTP error (429): Too Many Requests". The kit RPC executor has no built-in retry, and neither solanaRpc nor createSolanaRpc exposes a custom-transport hook, so the retry is added at the transport layer. A new createRetryingSolanaRpc wraps the default transport to retry on HTTP 429, honouring the server's Retry-After header when present and otherwise falling back to exponential backoff with full jitter (capped at 10s). Only 429s are retried; every other error propagates immediately. Because the retry lives in the transport, it covers every RPC call (blockhash lookups, simulations, sends and status polls), not just sends. To inject the retrying transport, getClient now builds the RPC and subscriptions itself and applies solanaRpc's constituent plugins (rpcGetMinimumBalance, rpcTransactionPlanner, rpcTransactionPlanSendingExecutor) rather than the all-in-one solanaRpc, keeping executor and planner defaults identical. getReadonlyClient uses the retrying RPC too. --- clients/js/src/cli/rpc.ts | 143 ++++++++++++++++++++++++++++++++++++ clients/js/src/cli/utils.ts | 31 +++++--- clients/js/test/rpc.test.ts | 120 ++++++++++++++++++++++++++++++ 3 files changed, 284 insertions(+), 10 deletions(-) create mode 100644 clients/js/src/cli/rpc.ts create mode 100644 clients/js/test/rpc.test.ts diff --git a/clients/js/src/cli/rpc.ts b/clients/js/src/cli/rpc.ts new file mode 100644 index 0000000..1444391 --- /dev/null +++ b/clients/js/src/cli/rpc.ts @@ -0,0 +1,143 @@ +import { + createDefaultRpcTransport, + createSolanaRpcFromTransport, + isSolanaError, + Rpc, + RpcTransport, + SOLANA_ERROR__RPC__TRANSPORT_HTTP_ERROR, + SolanaRpcApi, +} from '@solana/kit'; + +/** The maximum number of retries attempted for a rate-limited request. */ +const DEFAULT_MAX_RETRIES = 5; +/** The base delay, in milliseconds, used for exponential backoff. */ +const BASE_BACKOFF_MS = 500; +/** The ceiling, in milliseconds, applied to any computed backoff delay. */ +const MAX_BACKOFF_MS = 10_000; + +/** + * Options controlling how {@link createRetryingSolanaRpc} retries rate-limited + * requests. + */ +export type RetryingRpcConfig = { + /** + * Optional configuration forwarded to the underlying default RPC transport, + * such as custom headers. + */ + transportConfig?: Omit[0], 'url'>; + /** + * The maximum number of retries attempted after an initial rate-limited + * (HTTP 429) response before giving up. Defaults to {@link DEFAULT_MAX_RETRIES}. + */ + maxRetries?: number; + /** + * Sleep function used between retries. Injectable for testing; defaults to a + * `setTimeout`-based delay. + */ + sleep?: (ms: number) => Promise; +}; + +/** + * Creates a Solana RPC client whose transport automatically retries requests + * that fail with an HTTP 429 (Too Many Requests) response. + * + * Public RPC endpoints — devnet in particular — aggressively rate-limit bursts + * of requests. Since uploading metadata fans a large payload out into many + * write transactions (each performing several RPC calls), those bursts commonly + * trip the rate limiter and abort the whole upload. Retrying at the transport + * layer covers every RPC call (blockhash lookups, simulations, sends and status + * polls), not just the sends. + * + * The retry honours the server's `Retry-After` header when present; otherwise it + * falls back to exponential backoff with jitter. Only HTTP 429 responses are + * retried — every other error propagates immediately so genuine failures are + * surfaced without delay. + * + * @param url - The Solana RPC endpoint URL. + * @param config - Optional retry and transport configuration. + * @returns An {@link Rpc} backed by the retrying transport. + */ +export function createRetryingSolanaRpc(url: string, config: RetryingRpcConfig = {}): Rpc { + const baseTransport = createDefaultRpcTransport({ url, ...config.transportConfig }); + return createSolanaRpcFromTransport(withRateLimitRetries(baseTransport, config)); +} + +/** + * Wraps a transport so that requests failing with an HTTP 429 (Too Many + * Requests) response are retried. Exposed separately from + * {@link createRetryingSolanaRpc} so the retry behaviour can be tested against a + * mock transport without performing real network I/O. + * + * @param transport - The underlying transport to wrap. + * @param config - Optional retry configuration. + * @returns A transport that retries rate-limited requests. + */ +export function withRateLimitRetries(transport: RpcTransport, config: RetryingRpcConfig = {}): RpcTransport { + const maxRetries = config.maxRetries ?? DEFAULT_MAX_RETRIES; + const sleep = config.sleep ?? defaultSleep; + + return async (request: Parameters[0]): Promise => { + for (let attempt = 0; ; attempt++) { + try { + return await transport(request); + } catch (error) { + if (attempt >= maxRetries || !isRateLimitError(error)) { + throw error; + } + await sleep(getRetryDelayMs(error, attempt)); + } + } + }; +} + +/** Returns whether the given error is an HTTP 429 (rate limit) transport error. */ +function isRateLimitError(error: unknown): boolean { + return isSolanaError(error, SOLANA_ERROR__RPC__TRANSPORT_HTTP_ERROR) && error.context.statusCode === 429; +} + +/** + * Computes how long to wait before retrying a rate-limited request. + * + * Prefers the server-provided `Retry-After` header (supporting both the + * delay-seconds and HTTP-date forms) and falls back to exponential backoff with + * full jitter, capped at {@link MAX_BACKOFF_MS}. + */ +export function getRetryDelayMs(error: unknown, attempt: number): number { + const retryAfter = getRetryAfterMs(error); + if (retryAfter !== null) { + return Math.min(retryAfter, MAX_BACKOFF_MS); + } + const exponential = Math.min(BASE_BACKOFF_MS * 2 ** attempt, MAX_BACKOFF_MS); + // Full jitter: a random delay in [0, exponential] to spread out retries and + // avoid a thundering herd against the rate limiter. + return Math.round(Math.random() * exponential); +} + +/** + * Parses the `Retry-After` header from a rate-limit error into milliseconds, or + * returns `null` when the header is absent or unparseable. + */ +function getRetryAfterMs(error: unknown): number | null { + if (!isSolanaError(error, SOLANA_ERROR__RPC__TRANSPORT_HTTP_ERROR)) { + return null; + } + const headerValue = error.context.headers?.get('retry-after'); + if (!headerValue) { + return null; + } + + // `Retry-After` may be a number of seconds or an HTTP-date. + const seconds = Number(headerValue); + if (Number.isFinite(seconds)) { + return Math.max(0, seconds * 1000); + } + const dateMs = Date.parse(headerValue); + if (Number.isFinite(dateMs)) { + return Math.max(0, dateMs - Date.now()); + } + return null; +} + +function defaultSleep(ms: number): Promise { + return new Promise(resolve => setTimeout(resolve, ms)); +} diff --git a/clients/js/src/cli/utils.ts b/clients/js/src/cli/utils.ts index f540501..63734ef 100644 --- a/clients/js/src/cli/utils.ts +++ b/clients/js/src/cli/utils.ts @@ -15,7 +15,6 @@ import { createClient, createKeyPairSignerFromBytes, createNoopSigner, - createSolanaRpc, createSolanaRpcSubscriptions, extendClient, flattenTransactionPlan, @@ -35,7 +34,12 @@ import { TransactionPlan, TransactionSigner, } from '@solana/kit'; -import { solanaRpc, TransactionPlannerConfig } from '@solana/kit-plugin-rpc'; +import { + rpcGetMinimumBalance, + rpcTransactionPlanner, + rpcTransactionPlanSendingExecutor, + TransactionPlannerConfig, +} from '@solana/kit-plugin-rpc'; import { identity, payer } from '@solana/kit-plugin-signer'; import { Command } from 'commander'; import picocolors from 'picocolors'; @@ -56,6 +60,7 @@ import { RpcOption, WriteOptions, } from './options'; +import { createRetryingSolanaRpc } from './rpc'; const LOCALHOST_URL = 'http://127.0.0.1:8899'; const DATA_SOURCE_OPTIONS = @@ -85,16 +90,22 @@ export async function getClient(options: GlobalOptions) { const rpcSubscriptionsUrl = getRpcSubscriptionsUrl(rpcUrl, configs); const [identitySigner, payerSigner] = await getKeyPairSigners(options, configs); + // We build the RPC connection ourselves rather than using the all-in-one + // `solanaRpc` plugin because that plugin does not expose a hook for a custom + // transport, and we need a transport that retries on HTTP 429 responses to + // survive rate-limited endpoints. We therefore attach our retrying RPC (and + // its subscriptions) directly and apply the RPC plugin's constituents. + const rpc = createRetryingSolanaRpc(rpcUrl); + const rpcSubscriptions = createSolanaRpcSubscriptions(rpcSubscriptionsUrl); + const transactionConfig = getTransactionConfig(options); + return createClient() .use(payer(payerSigner)) .use(identity(identitySigner)) - .use( - solanaRpc({ - rpcUrl, - rpcSubscriptionsUrl, - transactionConfig: getTransactionConfig(options), - }), - ) + .use(client => extendClient(client, { rpc, rpcSubscriptions })) + .use(rpcGetMinimumBalance()) + .use(rpcTransactionPlanner(transactionConfig)) + .use(rpcTransactionPlanSendingExecutor({ estimateResourceLimits: transactionConfig.estimateResourceLimits })) .use(programMetadataProgram()) .use(cliConfigs(configs)) .use(cliRunOrExport(options)); @@ -213,7 +224,7 @@ export function getReadonlyClient(options: RpcOption): ReadonlyClient { const rpcSubscriptionsUrl = getRpcSubscriptionsUrl(rpcUrl, configs); return { configs, - rpc: createSolanaRpc(rpcUrl), + rpc: createRetryingSolanaRpc(rpcUrl), rpcSubscriptions: createSolanaRpcSubscriptions(rpcSubscriptionsUrl), }; } diff --git a/clients/js/test/rpc.test.ts b/clients/js/test/rpc.test.ts new file mode 100644 index 0000000..fbbe1dd --- /dev/null +++ b/clients/js/test/rpc.test.ts @@ -0,0 +1,120 @@ +import { RpcTransport, SOLANA_ERROR__RPC__TRANSPORT_HTTP_ERROR, SolanaError } from '@solana/kit'; +import { describe, expect, it, vi } from 'vitest'; + +import { getRetryDelayMs, withRateLimitRetries } from '../src/cli/rpc'; + +/** Builds an HTTP transport error matching what the default transport throws. */ +function httpError(statusCode: number, headers: Record = {}): SolanaError { + return new SolanaError(SOLANA_ERROR__RPC__TRANSPORT_HTTP_ERROR, { + headers: new Headers(headers), + message: statusCode === 429 ? 'Too Many Requests' : 'Server Error', + statusCode, + }); +} + +const noSleep = () => Promise.resolve(); + +/** + * Builds a mock {@link RpcTransport} that replays the given behaviours in order, + * throwing entries that are `Error`s and resolving anything else. The trailing + * behaviour is reused once the queue is exhausted, so a single entry acts as a + * transport that always behaves that way. Tracks its call count via `calls`. + */ +function mockTransport(...behaviours: unknown[]): RpcTransport & { calls: number } { + const fn = (): Promise => { + const behaviour = behaviours[Math.min(transport.calls, behaviours.length - 1)]; + transport.calls++; + return behaviour instanceof Error ? Promise.reject(behaviour) : Promise.resolve(behaviour); + }; + const transport = fn as unknown as RpcTransport & { calls: number }; + transport.calls = 0; + return transport; +} + +describe('withRateLimitRetries', () => { + it('retries a rate-limited request until it succeeds', async () => { + // Given a transport that fails with 429 twice, then succeeds. + const inner = mockTransport(httpError(429), httpError(429), 'ok'); + const transport = withRateLimitRetries(inner, { sleep: noSleep }); + + // When we send a request through the retrying transport. + const result = await transport({ payload: {} }); + + // Then it retries and eventually resolves with the successful response. + expect(result).toBe('ok'); + expect(inner.calls).toBe(3); + }); + + it('propagates non-429 errors immediately without retrying', async () => { + // Given a transport that fails with a 500 server error. + const inner = mockTransport(httpError(500)); + const transport = withRateLimitRetries(inner, { sleep: noSleep }); + + // When we send a request, then the error propagates on the first attempt. + await expect(transport({ payload: {} })).rejects.toThrow('HTTP error (500)'); + expect(inner.calls).toBe(1); + }); + + it('propagates non-Solana errors immediately without retrying', async () => { + // Given a transport that fails with a generic error. + const inner = mockTransport(new Error('boom')); + const transport = withRateLimitRetries(inner, { sleep: noSleep }); + + // When we send a request, then the error propagates on the first attempt. + await expect(transport({ payload: {} })).rejects.toThrow('boom'); + expect(inner.calls).toBe(1); + }); + + it('gives up after the configured number of retries', async () => { + // Given a transport that always returns 429. + const inner = mockTransport(httpError(429)); + const transport = withRateLimitRetries(inner, { maxRetries: 3, sleep: noSleep }); + + // When we send a request, then it throws after exhausting the retries. + await expect(transport({ payload: {} })).rejects.toThrow('HTTP error (429)'); + // 1 initial attempt + 3 retries. + expect(inner.calls).toBe(4); + }); + + it('waits between retries using the injected sleep function', async () => { + // Given a transport that fails once with 429, then succeeds. + const inner = mockTransport(httpError(429), 'ok'); + const sleep = vi.fn(() => Promise.resolve()); + const transport = withRateLimitRetries(inner, { sleep }); + + // When we send a request, then sleep is invoked once before the retry. + await transport({ payload: {} }); + expect(sleep).toHaveBeenCalledTimes(1); + }); +}); + +describe('getRetryDelayMs', () => { + it('honours a numeric Retry-After header in seconds', () => { + const delay = getRetryDelayMs(httpError(429, { 'retry-after': '2' }), 0); + expect(delay).toBe(2000); + }); + + it('honours an HTTP-date Retry-After header', () => { + const twoSecondsFromNow = new Date(Date.now() + 2000).toUTCString(); + const delay = getRetryDelayMs(httpError(429, { 'retry-after': twoSecondsFromNow }), 0); + // Allow a small margin for clock drift during the test. + expect(delay).toBeGreaterThan(0); + expect(delay).toBeLessThanOrEqual(2000); + }); + + it('caps a large Retry-After at the maximum backoff', () => { + const delay = getRetryDelayMs(httpError(429, { 'retry-after': '3600' }), 0); + expect(delay).toBe(10_000); + }); + + it('falls back to jittered exponential backoff when no header is present', () => { + // With full jitter, the delay is bounded by the exponential ceiling for + // the attempt: base (500ms) * 2 ** attempt, capped at the maximum. + for (let attempt = 0; attempt < 6; attempt++) { + const delay = getRetryDelayMs(httpError(429), attempt); + const ceiling = Math.min(500 * 2 ** attempt, 10_000); + expect(delay).toBeGreaterThanOrEqual(0); + expect(delay).toBeLessThanOrEqual(ceiling); + } + }); +}); From 5e77e4b07a4c393f47a3f839a5c56fbb112c8a70 Mon Sep 17 00:00:00 2001 From: Loris Leiva Date: Fri, 4 Sep 2026 12:50:48 +0100 Subject: [PATCH 2/3] Address review feedback on RPC retries Refines the rate-limit retry wrapper following review. A server-provided Retry-After is now honoured up to a separate, more generous ceiling (60s); if the server asks for longer, the request gives up immediately rather than spending a retry on a wait that is unlikely to succeed. The exponential backoff path keeps its 10s cap. The backoff sleep is now abort-aware, so a request cancelled mid-wait (e.g. when the executor cancels siblings after a failure) surfaces promptly instead of hanging for up to the full delay. A new onRetry hook lets callers observe retries; the CLI uses it to log a "rate limited, retrying in Xs" warning so uploads no longer appear to hang. The retry decision is factored into a pure getRetryDecision helper, and tests cover the new ceiling, unparseable Retry-After, abort-mid-backoff and onRetry behaviours. --- clients/js/src/cli/rpc.ts | 110 +++++++++++++++++++++++++++++------- clients/js/src/cli/utils.ts | 5 +- clients/js/test/rpc.test.ts | 98 ++++++++++++++++++++++++++++---- 3 files changed, 180 insertions(+), 33 deletions(-) diff --git a/clients/js/src/cli/rpc.ts b/clients/js/src/cli/rpc.ts index 1444391..6f0adb5 100644 --- a/clients/js/src/cli/rpc.ts +++ b/clients/js/src/cli/rpc.ts @@ -12,8 +12,29 @@ import { const DEFAULT_MAX_RETRIES = 5; /** The base delay, in milliseconds, used for exponential backoff. */ const BASE_BACKOFF_MS = 500; -/** The ceiling, in milliseconds, applied to any computed backoff delay. */ +/** The ceiling, in milliseconds, applied to computed exponential backoff delays. */ const MAX_BACKOFF_MS = 10_000; +/** + * The ceiling, in milliseconds, applied to a server-provided `Retry-After` + * value. When the server asks us to wait longer than this, we give up + * immediately rather than spending a retry on a wait that is unlikely to be + * worthwhile. This is deliberately more generous than {@link MAX_BACKOFF_MS} + * because the server is telling us exactly when it will accept the request. + */ +const MAX_RETRY_AFTER_MS = 60_000; + +/** + * Information passed to the {@link RetryingRpcConfig.onRetry} callback before a + * rate-limited request is retried. + */ +export type RetryInfo = { + /** The zero-based index of the attempt that just failed. */ + attempt: number; + /** The delay, in milliseconds, before the next attempt. */ + delayMs: number; + /** The rate-limit error that triggered the retry. */ + error: unknown; +}; /** * Options controlling how {@link createRetryingSolanaRpc} retries rate-limited @@ -30,11 +51,17 @@ export type RetryingRpcConfig = { * (HTTP 429) response before giving up. Defaults to {@link DEFAULT_MAX_RETRIES}. */ maxRetries?: number; + /** + * Called before each retry, after the delay has been computed but before it + * elapses. Useful for surfacing progress (e.g. logging a "rate limited, + * retrying in Xs" warning) so a paused request does not appear to hang. + */ + onRetry?: (info: RetryInfo) => void; /** * Sleep function used between retries. Injectable for testing; defaults to a - * `setTimeout`-based delay. + * `setTimeout`-based delay that resolves early if the request is aborted. */ - sleep?: (ms: number) => Promise; + sleep?: (ms: number, signal?: AbortSignal) => Promise; }; /** @@ -48,10 +75,10 @@ export type RetryingRpcConfig = { * layer covers every RPC call (blockhash lookups, simulations, sends and status * polls), not just the sends. * - * The retry honours the server's `Retry-After` header when present; otherwise it - * falls back to exponential backoff with jitter. Only HTTP 429 responses are - * retried — every other error propagates immediately so genuine failures are - * surfaced without delay. + * The retry honours the server's `Retry-After` header when present (giving up if + * it asks for an unreasonably long wait); otherwise it falls back to exponential + * backoff with jitter. Only HTTP 429 responses are retried — every other error + * propagates immediately so genuine failures are surfaced without delay. * * @param url - The Solana RPC endpoint URL. * @param config - Optional retry and transport configuration. @@ -81,32 +108,55 @@ export function withRateLimitRetries(transport: RpcTransport, config: RetryingRp try { return await transport(request); } catch (error) { - if (attempt >= maxRetries || !isRateLimitError(error)) { + // Don't retry non-429 errors, once the budget is exhausted, or + // once the request has been aborted (e.g. the executor + // cancelled sibling requests after another transaction failed). + const decision = getRetryDecision(error, attempt, maxRetries); + if (decision.kind === 'give-up' || request.signal?.aborted) { throw error; } - await sleep(getRetryDelayMs(error, attempt)); + config.onRetry?.({ attempt, delayMs: decision.delayMs, error }); + await sleep(decision.delayMs, request.signal); } } }; } -/** Returns whether the given error is an HTTP 429 (rate limit) transport error. */ -function isRateLimitError(error: unknown): boolean { - return isSolanaError(error, SOLANA_ERROR__RPC__TRANSPORT_HTTP_ERROR) && error.context.statusCode === 429; -} +/** The outcome of deciding whether and how long to wait before a retry. */ +type RetryDecision = { kind: 'retry'; delayMs: number } | { kind: 'give-up' }; /** - * Computes how long to wait before retrying a rate-limited request. + * Decides whether a failed request should be retried and, if so, after how long. * - * Prefers the server-provided `Retry-After` header (supporting both the - * delay-seconds and HTTP-date forms) and falls back to exponential backoff with - * full jitter, capped at {@link MAX_BACKOFF_MS}. + * Retries only HTTP 429 (rate limit) errors, and only while retries remain. When + * the server provides a `Retry-After` value we honour it up to + * {@link MAX_RETRY_AFTER_MS}; a longer requested wait is treated as not worth + * retrying and yields `give-up`. Without a usable header, we fall back to + * exponential backoff with full jitter, capped at {@link MAX_BACKOFF_MS}. */ -export function getRetryDelayMs(error: unknown, attempt: number): number { +export function getRetryDecision(error: unknown, attempt: number, maxRetries: number): RetryDecision { + if (attempt >= maxRetries || !isRateLimitError(error)) { + return { kind: 'give-up' }; + } const retryAfter = getRetryAfterMs(error); if (retryAfter !== null) { - return Math.min(retryAfter, MAX_BACKOFF_MS); + // The server told us exactly when to retry. Honour it within reason; + // beyond the ceiling the wait is not worth a retry slot. + return retryAfter > MAX_RETRY_AFTER_MS ? { kind: 'give-up' } : { kind: 'retry', delayMs: retryAfter }; } + return { kind: 'retry', delayMs: getBackoffDelayMs(attempt) }; +} + +/** Returns whether the given error is an HTTP 429 (rate limit) transport error. */ +function isRateLimitError(error: unknown): boolean { + return isSolanaError(error, SOLANA_ERROR__RPC__TRANSPORT_HTTP_ERROR) && error.context.statusCode === 429; +} + +/** + * Computes a jittered exponential backoff delay for the given attempt, capped at + * {@link MAX_BACKOFF_MS}. + */ +export function getBackoffDelayMs(attempt: number): number { const exponential = Math.min(BASE_BACKOFF_MS * 2 ** attempt, MAX_BACKOFF_MS); // Full jitter: a random delay in [0, exponential] to spread out retries and // avoid a thundering herd against the rate limiter. @@ -138,6 +188,24 @@ function getRetryAfterMs(error: unknown): number | null { return null; } -function defaultSleep(ms: number): Promise { - return new Promise(resolve => setTimeout(resolve, ms)); +/** + * Sleeps for the given duration, resolving early (without rejecting) if the + * optional abort signal fires. Resolving rather than throwing lets the retry + * loop re-check `signal.aborted` and surface the original error. + */ +function defaultSleep(ms: number, signal?: AbortSignal): Promise { + if (signal?.aborted) { + return Promise.resolve(); + } + return new Promise(resolve => { + const onAbort = () => { + clearTimeout(timeout); + resolve(); + }; + const timeout = setTimeout(() => { + signal?.removeEventListener('abort', onAbort); + resolve(); + }, ms); + signal?.addEventListener('abort', onAbort, { once: true }); + }); } diff --git a/clients/js/src/cli/utils.ts b/clients/js/src/cli/utils.ts index 63734ef..1581daa 100644 --- a/clients/js/src/cli/utils.ts +++ b/clients/js/src/cli/utils.ts @@ -95,7 +95,10 @@ export async function getClient(options: GlobalOptions) { // transport, and we need a transport that retries on HTTP 429 responses to // survive rate-limited endpoints. We therefore attach our retrying RPC (and // its subscriptions) directly and apply the RPC plugin's constituents. - const rpc = createRetryingSolanaRpc(rpcUrl); + const rpc = createRetryingSolanaRpc(rpcUrl, { + onRetry: ({ delayMs }) => + logWarning(`RPC rate limited (HTTP 429), retrying in ${(delayMs / 1000).toFixed(1)}s...`), + }); const rpcSubscriptions = createSolanaRpcSubscriptions(rpcSubscriptionsUrl); const transactionConfig = getTransactionConfig(options); diff --git a/clients/js/test/rpc.test.ts b/clients/js/test/rpc.test.ts index fbbe1dd..973bff0 100644 --- a/clients/js/test/rpc.test.ts +++ b/clients/js/test/rpc.test.ts @@ -1,7 +1,7 @@ import { RpcTransport, SOLANA_ERROR__RPC__TRANSPORT_HTTP_ERROR, SolanaError } from '@solana/kit'; import { describe, expect, it, vi } from 'vitest'; -import { getRetryDelayMs, withRateLimitRetries } from '../src/cli/rpc'; +import { getBackoffDelayMs, getRetryDecision, withRateLimitRetries } from '../src/cli/rpc'; /** Builds an HTTP transport error matching what the default transport throws. */ function httpError(statusCode: number, headers: Record = {}): SolanaError { @@ -86,32 +86,108 @@ describe('withRateLimitRetries', () => { await transport({ payload: {} }); expect(sleep).toHaveBeenCalledTimes(1); }); + + it('gives up when the server asks to wait longer than the ceiling', async () => { + // Given a transport that returns 429 with a two-minute Retry-After. + const inner = mockTransport(httpError(429, { 'retry-after': '120' })); + const sleep = vi.fn(() => Promise.resolve()); + const transport = withRateLimitRetries(inner, { sleep }); + + // When we send a request, then it throws immediately without sleeping. + await expect(transport({ payload: {} })).rejects.toThrow('HTTP error (429)'); + expect(inner.calls).toBe(1); + expect(sleep).not.toHaveBeenCalled(); + }); + + it('invokes onRetry before each retry with the retry details', async () => { + // Given a transport that fails twice with 429, then succeeds. + const inner = mockTransport(httpError(429), httpError(429), 'ok'); + const onRetry = vi.fn(); + const transport = withRateLimitRetries(inner, { onRetry, sleep: noSleep }); + + // When we send a request, then onRetry is called once per retry. + await transport({ payload: {} }); + expect(onRetry).toHaveBeenCalledTimes(2); + expect(onRetry).toHaveBeenNthCalledWith(1, expect.objectContaining({ attempt: 0 })); + expect(onRetry).toHaveBeenNthCalledWith(2, expect.objectContaining({ attempt: 1 })); + expect(onRetry.mock.calls[0][0]).toMatchObject({ + delayMs: expect.any(Number), + error: expect.any(SolanaError), + }); + }); + + it('stops retrying once the request is aborted mid-backoff', async () => { + // Given a transport that always returns 429 and a signal that aborts + // while the wrapper is waiting to retry after the first failure. + const inner = mockTransport(httpError(429)); + const controller = new AbortController(); + const sleep = vi.fn((_ms: number, _signal?: AbortSignal) => { + controller.abort(); + return Promise.resolve(); + }); + const transport = withRateLimitRetries(inner, { maxRetries: 5, sleep }); + + // When we send a request with the abortable signal, then it aborts after + // the single backoff rather than exhausting the full retry budget: one + // retry runs the transport again, sees the signal aborted, and throws. + await expect(transport({ payload: {}, signal: controller.signal })).rejects.toThrow('HTTP error (429)'); + expect(sleep).toHaveBeenCalledTimes(1); + expect(inner.calls).toBe(2); + }); }); -describe('getRetryDelayMs', () => { +describe('getRetryDecision', () => { + it('gives up on non-429 errors', () => { + expect(getRetryDecision(httpError(500), 0, 5)).toEqual({ kind: 'give-up' }); + }); + + it('gives up once the retry budget is exhausted', () => { + expect(getRetryDecision(httpError(429), 5, 5)).toEqual({ kind: 'give-up' }); + }); + it('honours a numeric Retry-After header in seconds', () => { - const delay = getRetryDelayMs(httpError(429, { 'retry-after': '2' }), 0); - expect(delay).toBe(2000); + expect(getRetryDecision(httpError(429, { 'retry-after': '2' }), 0, 5)).toEqual({ + kind: 'retry', + delayMs: 2000, + }); }); it('honours an HTTP-date Retry-After header', () => { const twoSecondsFromNow = new Date(Date.now() + 2000).toUTCString(); - const delay = getRetryDelayMs(httpError(429, { 'retry-after': twoSecondsFromNow }), 0); + const decision = getRetryDecision(httpError(429, { 'retry-after': twoSecondsFromNow }), 0, 5); + expect(decision.kind).toBe('retry'); // Allow a small margin for clock drift during the test. - expect(delay).toBeGreaterThan(0); - expect(delay).toBeLessThanOrEqual(2000); + expect(decision.kind === 'retry' && decision.delayMs).toBeGreaterThan(0); + expect(decision.kind === 'retry' && decision.delayMs).toBeLessThanOrEqual(2000); + }); + + it('gives up when Retry-After exceeds the ceiling', () => { + expect(getRetryDecision(httpError(429, { 'retry-after': '120' }), 0, 5)).toEqual({ kind: 'give-up' }); }); - it('caps a large Retry-After at the maximum backoff', () => { - const delay = getRetryDelayMs(httpError(429, { 'retry-after': '3600' }), 0); - expect(delay).toBe(10_000); + it('falls back to jittered exponential backoff for an unparseable Retry-After', () => { + const decision = getRetryDecision(httpError(429, { 'retry-after': 'soon' }), 0, 5); + expect(decision.kind).toBe('retry'); + // Attempt 0 backoff is bounded by the base delay (500ms). + expect(decision.kind === 'retry' && decision.delayMs).toBeGreaterThanOrEqual(0); + expect(decision.kind === 'retry' && decision.delayMs).toBeLessThanOrEqual(500); }); it('falls back to jittered exponential backoff when no header is present', () => { + const decision = getRetryDecision(httpError(429), 2, 5); + expect(decision.kind).toBe('retry'); + // Attempt 2 backoff is bounded by base (500ms) * 2 ** 2 = 2000ms. + expect(decision.kind === 'retry' && decision.delayMs).toBeGreaterThanOrEqual(0); + expect(decision.kind === 'retry' && decision.delayMs).toBeLessThanOrEqual(2000); + }); +}); + +describe('getBackoffDelayMs', () => { + it('stays within the jittered exponential ceiling for each attempt', () => { // With full jitter, the delay is bounded by the exponential ceiling for // the attempt: base (500ms) * 2 ** attempt, capped at the maximum. for (let attempt = 0; attempt < 6; attempt++) { - const delay = getRetryDelayMs(httpError(429), attempt); + const delay = getBackoffDelayMs(attempt); const ceiling = Math.min(500 * 2 ** attempt, 10_000); expect(delay).toBeGreaterThanOrEqual(0); expect(delay).toBeLessThanOrEqual(ceiling); From c3c849d3829968dd6b50208494deb693dd7ada43 Mon Sep 17 00:00:00 2001 From: Loris Leiva Date: Fri, 4 Sep 2026 12:59:31 +0100 Subject: [PATCH 3/3] Apply review nits to RPC retries Addresses two non-blocking review nits. The retry loop now re-checks the abort signal after the backoff sleep, so a request cancelled mid-wait surfaces the original 429 rather than looping into a transport call that would reject with an AbortError (matching the defaultSleep docstring). The onRetry warning is now shared between getClient and getReadonlyClient via a getRetryingRpcConfig helper, so read commands warn on rate limiting too instead of pausing silently. --- clients/js/src/cli/rpc.ts | 6 ++++++ clients/js/src/cli/utils.ts | 21 +++++++++++++++------ clients/js/test/rpc.test.ts | 8 ++++---- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/clients/js/src/cli/rpc.ts b/clients/js/src/cli/rpc.ts index 6f0adb5..c48b400 100644 --- a/clients/js/src/cli/rpc.ts +++ b/clients/js/src/cli/rpc.ts @@ -117,6 +117,12 @@ export function withRateLimitRetries(transport: RpcTransport, config: RetryingRp } config.onRetry?.({ attempt, delayMs: decision.delayMs, error }); await sleep(decision.delayMs, request.signal); + // The sleep resolves early on abort; surface the original 429 + // rather than looping back into a transport call that would + // reject with an `AbortError` instead. + if (request.signal?.aborted) { + throw error; + } } } }; diff --git a/clients/js/src/cli/utils.ts b/clients/js/src/cli/utils.ts index 1581daa..ea3f064 100644 --- a/clients/js/src/cli/utils.ts +++ b/clients/js/src/cli/utils.ts @@ -60,7 +60,7 @@ import { RpcOption, WriteOptions, } from './options'; -import { createRetryingSolanaRpc } from './rpc'; +import { createRetryingSolanaRpc, RetryingRpcConfig } from './rpc'; const LOCALHOST_URL = 'http://127.0.0.1:8899'; const DATA_SOURCE_OPTIONS = @@ -95,10 +95,7 @@ export async function getClient(options: GlobalOptions) { // transport, and we need a transport that retries on HTTP 429 responses to // survive rate-limited endpoints. We therefore attach our retrying RPC (and // its subscriptions) directly and apply the RPC plugin's constituents. - const rpc = createRetryingSolanaRpc(rpcUrl, { - onRetry: ({ delayMs }) => - logWarning(`RPC rate limited (HTTP 429), retrying in ${(delayMs / 1000).toFixed(1)}s...`), - }); + const rpc = createRetryingSolanaRpc(rpcUrl, getRetryingRpcConfig()); const rpcSubscriptions = createSolanaRpcSubscriptions(rpcSubscriptionsUrl); const transactionConfig = getTransactionConfig(options); @@ -114,6 +111,18 @@ export async function getClient(options: GlobalOptions) { .use(cliRunOrExport(options)); } +/** + * Shared configuration for the CLI's retrying RPC. Surfaces a warning whenever a + * request is rate limited and retried, so a paused command does not appear to + * hang. Used by both {@link getClient} and {@link getReadonlyClient}. + */ +function getRetryingRpcConfig(): RetryingRpcConfig { + return { + onRetry: ({ delayMs }) => + logWarning(`RPC rate limited (HTTP 429), retrying in ${(delayMs / 1000).toFixed(1)}s...`), + }; +} + /** * Builds the transaction planner config for the requested transaction version. * The config shape is discriminated by `version`: legacy and version 0 @@ -227,7 +236,7 @@ export function getReadonlyClient(options: RpcOption): ReadonlyClient { const rpcSubscriptionsUrl = getRpcSubscriptionsUrl(rpcUrl, configs); return { configs, - rpc: createRetryingSolanaRpc(rpcUrl), + rpc: createRetryingSolanaRpc(rpcUrl, getRetryingRpcConfig()), rpcSubscriptions: createSolanaRpcSubscriptions(rpcSubscriptionsUrl), }; } diff --git a/clients/js/test/rpc.test.ts b/clients/js/test/rpc.test.ts index 973bff0..d6f4c93 100644 --- a/clients/js/test/rpc.test.ts +++ b/clients/js/test/rpc.test.ts @@ -127,12 +127,12 @@ describe('withRateLimitRetries', () => { }); const transport = withRateLimitRetries(inner, { maxRetries: 5, sleep }); - // When we send a request with the abortable signal, then it aborts after - // the single backoff rather than exhausting the full retry budget: one - // retry runs the transport again, sees the signal aborted, and throws. + // When we send a request with the abortable signal, then after the first + // failure and single backoff the loop notices the abort and throws the + // original 429 without making a second transport call. await expect(transport({ payload: {}, signal: controller.signal })).rejects.toThrow('HTTP error (429)'); expect(sleep).toHaveBeenCalledTimes(1); - expect(inner.calls).toBe(2); + expect(inner.calls).toBe(1); }); });