Skip to content

chore: add retries for all requests #5354

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tomiir wants to merge 4 commits into
base: main
Choose a base branch
from
Open

chore: add retries for all requests #5354

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fa91045
chore: add retries for all requests
tomiir Nov 14, 2025
2d94d45
chore amend tests
tomiir Nov 14, 2025
b463d2c
chore: remove timeout clear
tomiir Nov 14, 2025
3b59780
Merge branch 'main' into feat/retry-core-requests
tomiir Nov 14, 2025
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
64 changes: 52 additions & 12 deletions packages/controllers/src/controllers/ApiController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { ConstantsUtil } from '@reown/appkit-common'
import type { ChainNamespace } from '@reown/appkit-common'

import { AssetUtil } from '../utils/AssetUtil.js'
import { ConstantsUtil as ControllersConstantsUtil } from '../utils/ConstantsUtil.js'
import { CoreHelperUtil } from '../utils/CoreHelperUtil.js'
import { FetchUtil } from '../utils/FetchUtil.js'
import { CUSTOM_DEEPLINK_WALLETS } from '../utils/MobileWallet.js'
Expand Down Expand Up @@ -36,6 +37,29 @@ const entries = 40
const recommendedEntries = 4
const imageCountToFetch = 20

function withRetries<T>(fn: () => Promise<T>, intervalMs: number, maxRetries = 0): Promise<T> {
return new Promise((resolve, reject) => {
let attempts = 0
async function check(): Promise<void> {
try {
const result = await fn()

resolve(result)
} catch (error) {
if (attempts >= maxRetries) {
// Stop retrying after exhausting attempts
reject(error)

return
}
attempts += 1
setTimeout(check, intervalMs)
}
}
check()
})
}

// -- Types --------------------------------------------- //
export interface ApiControllerState {
promises: Record<string, Promise<unknown>>
Expand Down Expand Up @@ -176,20 +200,30 @@ export const ApiController = {
},

async fetchProjectConfig() {
const response = await api.get<ApiGetProjectConfigResponse>({
path: '/appkit/v1/config',
params: ApiController._getSdkProperties()
})
const response = await withRetries(
() =>
api.get<ApiGetProjectConfigResponse>({
path: '/appkit/v1/config',
params: ApiController._getSdkProperties()
}),
ControllersConstantsUtil.FIVE_SEC_MS,
3
)

return response.features
},

async fetchUsage() {
try {
const response = await api.get<ApiGetUsageResponse>({
path: '/appkit/v1/project-limits',
params: ApiController._getSdkProperties()
})
const response = await withRetries(
() =>
api.get<ApiGetUsageResponse>({
path: '/appkit/v1/project-limits',
params: ApiController._getSdkProperties()
}),
ControllersConstantsUtil.FIVE_SEC_MS,
3
)

const { tier, isAboveMauLimit, isAboveRpcLimit } = response.planLimits

Expand All @@ -211,10 +245,16 @@ export const ApiController = {

async fetchAllowedOrigins() {
try {
const { allowedOrigins } = await api.get<ApiGetAllowedOriginsResponse>({
path: '/projects/v1/origins',
params: ApiController._getSdkProperties()
})
const { allowedOrigins } = await withRetries(
() =>
api.get<ApiGetAllowedOriginsResponse>({
path: '/projects/v1/origins',
params: ApiController._getSdkProperties()
}),
// 3 retries with 5 seconds interval
ControllersConstantsUtil.FIVE_SEC_MS,
3
)
Copy link

@cursor cursor bot Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Retry wrapper incorrectly retries client errors.

The withRetries wrapper retries all errors including HTTP 429 (rate limit) responses. When rate limited, the function makes 4 total attempts (initial + 3 retries) with 5-second intervals before throwing RATE_LIMITED. Retrying on 429 errors without respecting rate limit signals can exacerbate rate limiting and violate HTTP semantics. The retry logic should skip retries for 4xx client errors, particularly 429.

Fix in Cursor Fix in Web


return allowedOrigins
} catch (error) {
Expand Down
182 changes: 169 additions & 13 deletions packages/controllers/tests/controllers/ApiController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from '../../exports/index.js'
import { mockChainControllerState } from '../../exports/testing.js'
import { api } from '../../src/controllers/ApiController.js'
import { ConstantsUtil as ControllersConstantsUtil } from '../../src/utils/ConstantsUtil.js'
import { CUSTOM_DEEPLINK_WALLETS } from '../../src/utils/MobileWallet.js'

// -- Constants ----------------------------------------------------------------
Expand Down Expand Up @@ -914,65 +915,120 @@ describe('ApiController', () => {
})

it('should throw RATE_LIMITED error for HTTP 429 status', async () => {
vi.useFakeTimers()
const mockError = new Error('Rate limited')
mockError.cause = new Response('Too Many Requests', { status: 429 })
const fetchSpy = vi.spyOn(api, 'get').mockRejectedValueOnce(mockError)
const fetchSpy = vi.spyOn(api, 'get').mockRejectedValue(mockError)

await expect(ApiController.fetchAllowedOrigins()).rejects.toThrow('RATE_LIMITED')
const promise = ApiController.fetchAllowedOrigins().catch(e => e as Error)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)

const err = (await promise) as Error
expect(err.message).toBe('RATE_LIMITED')
expect(fetchSpy).toHaveBeenCalledWith({
path: '/projects/v1/origins',
params: ApiController._getSdkProperties()
})
await vi.runOnlyPendingTimersAsync()
vi.useRealTimers()
})

it('should throw SERVER_ERROR for HTTP 5xx status codes', async () => {
vi.useFakeTimers()
const mockError = new Error('Internal Server Error')
mockError.cause = new Response('Internal Server Error', { status: 500 })
const fetchSpy = vi.spyOn(api, 'get').mockRejectedValueOnce(mockError)
const fetchSpy = vi.spyOn(api, 'get').mockRejectedValue(mockError)

const promise = ApiController.fetchAllowedOrigins().catch(e => e as Error)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)

await expect(ApiController.fetchAllowedOrigins()).rejects.toThrow('SERVER_ERROR')
const err = (await promise) as Error
expect(err.message).toBe('SERVER_ERROR')
expect(fetchSpy).toHaveBeenCalledWith({
path: '/projects/v1/origins',
params: ApiController._getSdkProperties()
})
await vi.runOnlyPendingTimersAsync()
vi.useRealTimers()
})

it('should throw SERVER_ERROR for HTTP 502 status code', async () => {
vi.useFakeTimers()
const mockError = new Error('Bad Gateway')
mockError.cause = new Response('Bad Gateway', { status: 502 })
vi.spyOn(api, 'get').mockRejectedValueOnce(mockError)

await expect(ApiController.fetchAllowedOrigins()).rejects.toThrow('SERVER_ERROR')
vi.spyOn(api, 'get').mockRejectedValue(mockError)

const promise = ApiController.fetchAllowedOrigins().catch(e => e as Error)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
const err = (await promise) as Error
expect(err.message).toBe('SERVER_ERROR')
await vi.runOnlyPendingTimersAsync()
vi.useRealTimers()
})

it('should return empty array for HTTP 403 status (existing behavior)', async () => {
vi.useFakeTimers()
const mockError = new Error('Forbidden')
mockError.cause = new Response('Forbidden', { status: 403 })
const fetchSpy = vi.spyOn(api, 'get').mockRejectedValueOnce(mockError)
const fetchSpy = vi.spyOn(api, 'get').mockRejectedValue(mockError)

const result = await ApiController.fetchAllowedOrigins()
const promise = ApiController.fetchAllowedOrigins().catch(e => e as Error)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)

const result = await promise
expect(result).toEqual([])
expect(fetchSpy).toHaveBeenCalledWith({
path: '/projects/v1/origins',
params: ApiController._getSdkProperties()
})
await vi.runOnlyPendingTimersAsync()
vi.useRealTimers()
})

it('should return empty array for non-HTTP errors (existing behavior)', async () => {
vi.useFakeTimers()
const mockError = new Error('Network error')
vi.spyOn(api, 'get').mockRejectedValueOnce(mockError)
vi.spyOn(api, 'get').mockRejectedValue(mockError)

const result = await ApiController.fetchAllowedOrigins()
const promise = ApiController.fetchAllowedOrigins()
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)

const result = await promise
expect(result).toEqual([])
await vi.runOnlyPendingTimersAsync()
vi.useRealTimers()
})

it('should return empty array for HTTP errors without Response cause', async () => {
vi.useFakeTimers()
const mockError = new Error('Some error')
mockError.cause = 'not a response object'
vi.spyOn(api, 'get').mockRejectedValueOnce(mockError)
vi.spyOn(api, 'get').mockRejectedValue(mockError)

const result = await ApiController.fetchAllowedOrigins()
const promise = ApiController.fetchAllowedOrigins()
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)

const result = await promise
expect(result).toEqual([])
await vi.runOnlyPendingTimersAsync()
vi.useRealTimers()
})

it('should filter out wallets without mobile_link in mobile environment', () => {
Expand Down Expand Up @@ -1404,3 +1460,103 @@ describe('ApiController', () => {
})
})
})

describe('fetchProjectConfig', () => {
it('should fetch project config and return features', async () => {
const features = [{ id: 'reown_branding', isEnabled: true, config: null }]
const fetchSpy = vi.spyOn(api, 'get').mockResolvedValueOnce({ features })

const result = await ApiController.fetchProjectConfig()

expect(fetchSpy).toHaveBeenCalledWith({
path: '/appkit/v1/config',
params: ApiController._getSdkProperties()
})
expect(result).toEqual(features)
})

it('should retry on failures and succeed on a later attempt', async () => {
vi.useFakeTimers()
const features = [{ id: 'reown_branding', isEnabled: true, config: null }]
const fetchSpy = vi
.spyOn(api, 'get')
.mockRejectedValueOnce(new Error('temporary-1'))
.mockRejectedValueOnce(new Error('temporary-2'))
.mockResolvedValueOnce({ features })

const promise = ApiController.fetchProjectConfig()

// Fast-forward timers to allow retries to run
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS * 2)

await expect(promise).resolves.toEqual(features)
// At least 3 calls: initial + 2 retries before success
expect(fetchSpy.mock.calls.length).toBeGreaterThanOrEqual(3)
vi.useRealTimers()
})
})

describe('fetchUsage', () => {
it('should update plan state based on usage limits', async () => {
// Reset plan to default
ApiController.state.plan = {
tier: 'none',
hasExceededUsageLimit: false,
limits: { isAboveRpcLimit: false, isAboveMauLimit: false }
}

const response = {
planLimits: { tier: 'starter', isAboveMauLimit: true, isAboveRpcLimit: false }
}
vi.spyOn(api, 'get').mockResolvedValueOnce(response)

await ApiController.fetchUsage()

expect(ApiController.state.plan).toEqual({
tier: 'starter',
hasExceededUsageLimit: true,
limits: { isAboveRpcLimit: false, isAboveMauLimit: true }
})
})
})

describe('fetchAllowedOrigins - retry mechanism', () => {
it('should retry and eventually return allowedOrigins after transient failure', async () => {
vi.useFakeTimers()
const allowed = ['https://example.com']
const fetchSpy = vi
.spyOn(api, 'get')
.mockRejectedValueOnce(new Error('temporary'))
.mockResolvedValueOnce({ allowedOrigins: allowed })

const promise = ApiController.fetchAllowedOrigins()

// Let retries run (initial + 1 retry)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)

await expect(promise).resolves.toEqual(allowed)
expect(fetchSpy).toHaveBeenCalledTimes(2)
await vi.runOnlyPendingTimersAsync()
vi.useRealTimers()
})

it('should retry 3 times and then throw RATE_LIMITED for repeated 429 errors', async () => {
vi.useFakeTimers()
const thrown = new Error('Rate limited')
thrown.cause = new Response('Too Many Requests', { status: 429 })
const fetchSpy = vi.spyOn(api, 'get').mockRejectedValue(thrown)

const promise = ApiController.fetchAllowedOrigins().catch(e => e as Error)

// Let all retries run (initial + 3 retries)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)
await vi.advanceTimersByTimeAsync(ControllersConstantsUtil.FIVE_SEC_MS)

const caughtErr = (await promise) as Error
expect(caughtErr.message).toBe('RATE_LIMITED')
expect(fetchSpy).toHaveBeenCalledTimes(4)
await vi.runOnlyPendingTimersAsync()
vi.useRealTimers()
})
})
Loading