Skip to content

fix(tron): let balance Retry reach the network while the circuit breaker is open #526

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

Draft
rvelaz wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix(tron): let balance Retry reach the network while the circuit breaker is open #526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
70 changes: 70 additions & 0 deletions packages/tron-wallet-snap/src/clients/tron-http/TronHttpClient.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
BaseConfigProvider,
BrokenCircuitError,
FetchPolicyErrorName,
} from '@metamask/snap-networks-utils';

Expand Down Expand Up @@ -120,4 +121,73 @@ describe('TronHttpClient', () => {
expect(fetchMock).toHaveBeenCalledTimes(2);
});
});

describe('open circuit bypass (v5)', () => {
/**
* Opens the fetch policy circuit with non-retryable server failures.
*
* @param client - The client.
*/
async function openCircuit(client: TronHttpClient): Promise<void> {
jest
.spyOn(global, 'fetch')
.mockImplementation(
async () => new globalThis.Response('', { status: 501 }),
);
for (let attempt = 0; attempt < 30; attempt++) {
try {
await client.getReward(Network.Mainnet, `address-${attempt}`);
} catch (error) {
if (error instanceof BrokenCircuitError) {
return;
}
}
}
throw new Error('The circuit did not open');
}

it('lets an explicit request reach the network once while the circuit is open', async () => {
const client = new TronHttpClient({ configProvider });
await openCircuit(client);
const fetchMock = jest.spyOn(global, 'fetch').mockImplementation(
async () =>
new globalThis.Response(JSON.stringify({ reward: 7 }), {
status: 200,
}),
);

// The spy is reused: forget the calls that opened the circuit.
fetchMock.mockClear();

await expect(
client.getReward(Network.Mainnet, 'address'),
).rejects.toBeInstanceOf(BrokenCircuitError);
expect(fetchMock).not.toHaveBeenCalled();

expect(
await client.getReward(Network.Mainnet, 'address', {
bypassOpenCircuit: true,
}),
).toBe(7);
expect(fetchMock).toHaveBeenCalledTimes(1);

// The next explicit call for the same request (the balances call that
// follows `listAccountAssets`) reuses the direct result.
expect(
await client.getReward(Network.Mainnet, 'address', {
bypassOpenCircuit: true,
}),
).toBe(7);
expect(fetchMock).toHaveBeenCalledTimes(1);

// The service answered: the circuit is closed again for every read.

expect(
await client.getAccountResources(Network.Mainnet, 'address', {
bypassOpenCircuit: true,
}),
).toStrictEqual({ reward: 7 });
expect(fetchMock).toHaveBeenCalledTimes(2);
});
});
});
125 changes: 86 additions & 39 deletions packages/tron-wallet-snap/src/clients/tron-http/TronHttpClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ import { Network } from '../../constants';
import type { ConfigProvider } from '../../services/config';
import { hexToString } from '../../utils/hex';
import logger, { noOpLogger } from '../../utils/logger';
import { OpenCircuitBypass } from '../../utils/openCircuitBypass';
import type {
BypassablePolicy,
ExplicitRequestOptions,
} from '../../utils/openCircuitBypass';
import {
AccountResourcesStruct,
ChainParametersResponseStruct,
Expand Down Expand Up @@ -55,7 +60,9 @@ export class TronHttpClient {
}
> = new Map();

readonly #policies: Map<Network, FetchPolicy>;
readonly #policies: Map<Network, FetchPolicy & BypassablePolicy>;

readonly #openCircuitBypass = new OpenCircuitBypass();

readonly #requestCache: (
network: Network,
Expand Down Expand Up @@ -172,12 +179,14 @@ export class TronHttpClient {
* @see https://developers.tron.network/reference/getaccountresource
* @param network - Network to query.
* @param accountAddress - Account address in base58 format.
* @param options - Options of an explicit client request.
* @returns Promise<AccountResources> - Account resources (energy, bandwidth, etc.).
* @throws Error - HTTP errors or configuration errors.
*/
async getAccountResources(
network: Network,
accountAddress: string,
options?: ExplicitRequestOptions,
): Promise<AccountResources> {
const client = this.#clients.get(network);
if (!client) {
Expand All @@ -195,11 +204,17 @@ export class TronHttpClient {
visible: true,
});

const response = await this.#fetch(network, url, {
method: 'POST',
headers,
body,
});
const response = await this.#fetch(
network,
url,
{
method: 'POST',
headers,
body,
},
[],
options,
);

const accountResources: AccountResources = JSON.parse(response.body);

Expand Down Expand Up @@ -434,9 +449,14 @@ export class TronHttpClient {
* @see https://developers.tron.network/reference/getreward
* @param network - Network to query
* @param accountAddress - Account address in base58 format
* @param options - Options of an explicit client request.
* @returns Promise<number> - Unclaimed rewards in sun (0 if no rewards)
*/
async getReward(network: Network, accountAddress: string): Promise<number> {
async getReward(
network: Network,
accountAddress: string,
options?: ExplicitRequestOptions,
): Promise<number> {
const client = this.#clients.get(network);
if (!client) {
throw new Error(`No client configured for network: ${network}`);
Expand All @@ -453,11 +473,17 @@ export class TronHttpClient {
visible: true,
});

const response = await this.#fetch(network, url, {
method: 'POST',
headers,
body,
});
const response = await this.#fetch(
network,
url,
{
method: 'POST',
headers,
body,
},
[],
options,
);

const rewardResponse: GetRewardResponse = JSON.parse(response.body);

Expand All @@ -472,14 +498,25 @@ export class TronHttpClient {
url: string,
init: RequestInit,
allowedStatuses: number[] = [],
options?: ExplicitRequestOptions,
): Promise<{ body: string; status: number }> {
return this.#requestCache(
network,
url,
init.method ?? 'GET',
init.headers as Record<string, string> | undefined,
typeof init.body === 'string' ? init.body : '',
allowedStatuses,
const method = init.method ?? 'GET';
const headers = init.headers as Record<string, string> | undefined;
const body = typeof init.body === 'string' ? init.body : '';
return this.#openCircuitBypass.run(
`${network} ${method} ${url} ${body}`,
options,
async () =>
this.#requestCache(
network,
url,
method,
headers,
body,
allowedStatuses,
),
async () => this.#request(url, method, headers, body, allowedStatuses),
this.#policies.get(network),
);
}

Expand All @@ -496,26 +533,36 @@ export class TronHttpClient {
throw new Error(`No policy configured for network: ${network}`);
}

return executeWithFetchPolicy(policy, async () => {
const response = await fetch(url, {
method,
headers,
...(body ? { body } : {}),
signal: AbortSignal.timeout(10_000),
});
if (!response.ok && !allowedStatuses.includes(response.status)) {
throw createFetchPolicyError(`HTTP error! status: ${response.status}`, {
httpStatus: response.status,
retryAfterMs: parseRetryAfter(response.headers.get('Retry-After')),
});
}
if (allowedStatuses.includes(response.status)) {
return { body: '', status: response.status };
}
return {
body: await response.text(),
status: response.status,
};
return executeWithFetchPolicy(policy, async () =>
this.#request(url, method, headers, body, allowedStatuses),
);
}

async #request(
url: string,
method: string,
headers: Record<string, string> | undefined,
body: string,
allowedStatuses: number[],
): Promise<{ body: string; status: number }> {
const response = await fetch(url, {
method,
headers,
...(body ? { body } : {}),
signal: AbortSignal.timeout(10_000),
});
if (!response.ok && !allowedStatuses.includes(response.status)) {
throw createFetchPolicyError(`HTTP error! status: ${response.status}`, {
httpStatus: response.status,
retryAfterMs: parseRetryAfter(response.headers.get('Retry-After')),
});
}
if (allowedStatuses.includes(response.status)) {
return { body: '', status: response.status };
}
return {
body: await response.text(),
status: response.status,
};
}
}
103 changes: 103 additions & 0 deletions packages/tron-wallet-snap/src/clients/trongrid/TrongridApiClient.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
BaseConfigProvider,
BrokenCircuitError,
InMemoryCache,
} from '@metamask/snap-networks-utils';
import type { ICache, Serializable } from '@metamask/snap-networks-utils';
Expand Down Expand Up @@ -737,4 +738,106 @@ describe('TrongridApiClient', () => {
});
});
});

describe('open circuit bypass (v5)', () => {
const mockAddress = 'TGJn1wnUYHJbvN88cynZbsAz2EMeZq73yx';
const mockTrc20Balances: Trc20Balance[] = [
{ TR7NHqjeKQxGTCi8q8ZY4pL8otSzgjLj6t: '24249143' },
];

/**
* Opens the fetch policy circuit with non-retryable server failures.
*
* @param client - The client.
*/
async function openCircuit(client: TrongridApiClient): Promise<void> {
jest
.spyOn(global, 'fetch')
// eslint-disable-next-line no-restricted-globals
.mockImplementation(async () => new Response('', { status: 501 }));
for (let attempt = 0; attempt < 30; attempt++) {
try {
await client.getTrc20BalancesByAddress(
Network.Mainnet,
`address-${attempt}`,
);
} catch (error) {
if (error instanceof BrokenCircuitError) {
return;
}
}
}
throw new Error('The circuit did not open');
}

it('lets an explicit request reach the network once while the circuit is open', async () => {
await withTrongridApiClient(async ({ client }) => {
await openCircuit(client);
const fetchMock = jest.spyOn(global, 'fetch').mockImplementation(
async () =>
// eslint-disable-next-line no-restricted-globals
new Response(
JSON.stringify({
data: mockTrc20Balances,
success: true,
meta: { at: 1770121997373, page_size: 4 },
}),
{ status: 200, headers: { 'Content-Type': 'application/json' } },
),
);

// The spy is reused: forget the calls that opened the circuit.
fetchMock.mockClear();

// Background reads stay short-circuited.
await expect(
client.getTrc20BalancesByAddress(Network.Mainnet, mockAddress),
).rejects.toBeInstanceOf(BrokenCircuitError);
expect(fetchMock).not.toHaveBeenCalled();

// Concurrent identical explicit requests share one attempt.
const results = await Promise.all([
client.getTrc20BalancesByAddress(Network.Mainnet, mockAddress, {
bypassOpenCircuit: true,
}),
client.getTrc20BalancesByAddress(Network.Mainnet, mockAddress, {
bypassOpenCircuit: true,
}),
]);
expect(results).toStrictEqual([mockTrc20Balances, mockTrc20Balances]);
expect(fetchMock).toHaveBeenCalledTimes(1);

// The next explicit call reuses the direct result.
expect(
await client.getTrc20BalancesByAddress(Network.Mainnet, mockAddress, {
bypassOpenCircuit: true,
}),
).toStrictEqual(mockTrc20Balances);
expect(fetchMock).toHaveBeenCalledTimes(1);

// The service answered: background reads are no longer
// short-circuited.
expect(
await client.getTrc20BalancesByAddress(Network.Mainnet, mockAddress),
).toStrictEqual(mockTrc20Balances);
expect(fetchMock).toHaveBeenCalledTimes(2);
});
});

it('does not bypass the circuit for other failures', async () => {
await withTrongridApiClient(async ({ client }) => {
const fetchMock = jest
.spyOn(global, 'fetch')
// eslint-disable-next-line no-restricted-globals
.mockImplementation(async () => new Response('', { status: 404 }));

await expect(
client.getTrc20BalancesByAddress(Network.Mainnet, mockAddress, {
bypassOpenCircuit: true,
}),
).rejects.toThrow(Error);
expect(fetchMock).toHaveBeenCalledTimes(1);
});
});
});
});
Loading
Loading