Skip to content

Commit

Permalink
chore: use term 'retries' instead of 'retryAttempts' consistently (#6465
Browse files Browse the repository at this point in the history
)
  • Loading branch information
nflaig authored Feb 22, 2024
1 parent eedfaa1 commit c999e4a
Show file tree
Hide file tree
Showing 12 changed files with 46 additions and 45 deletions.
6 changes: 3 additions & 3 deletions packages/api/src/utils/client/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {HttpStatusCode} from "./httpStatusCode.js";

/* eslint-disable @typescript-eslint/no-explicit-any */

type ExtraOpts = {retryAttempts?: number};
type ExtraOpts = {retries?: number};
type ParametersWithOptionalExtraOpts<T extends (...args: any) => any> = [...Parameters<T>, ExtraOpts] | Parameters<T>;

export type ApiWithExtraOpts<T extends Record<string, APIClientHandler>> = {
Expand Down Expand Up @@ -78,8 +78,8 @@ export function generateGenericJsonClient<
//
const argLen = (args as any[])?.length ?? 0;
const lastArg = (args as any[])[argLen] as ExtraOpts | undefined;
const retryAttempts = lastArg?.retryAttempts;
const extraOpts = {retryAttempts};
const retries = lastArg?.retries;
const extraOpts = {retries};

if (returnType) {
// open extraOpts first if some serializer wants to add some overriding param
Expand Down
6 changes: 3 additions & 3 deletions packages/api/src/utils/client/httpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export type FetchOpts = {
/** Optional, for metrics */
routeId?: string;
timeoutMs?: number;
retryAttempts?: number;
retries?: number;
};

export interface IHttpClient {
Expand Down Expand Up @@ -183,15 +183,15 @@ export class HttpClient implements IHttpClient {
opts: FetchOpts,
getBody: (res: Response) => Promise<T>
): Promise<{status: HttpStatusCode; body: T}> {
if (opts.retryAttempts !== undefined) {
if (opts.retries !== undefined) {
const routeId = opts.routeId ?? DEFAULT_ROUTE_ID;

return retry(
async (_attempt) => {
return this.requestWithBodyWithFallbacks<T>(opts, getBody);
},
{
retries: opts.retryAttempts,
retries: opts.retries,
retryDelay: 200,
onRetry: (e, attempt) => {
this.logger?.debug("Retrying request", {routeId, attempt, lastError: e.message});
Expand Down
10 changes: 5 additions & 5 deletions packages/beacon-node/src/eth1/provider/jsonRpcHttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export type ReqOpts = {
// To label request metrics
routeId?: string;
// retry opts
retryAttempts?: number;
retries?: number;
retryDelay?: number;
shouldRetry?: (lastError: Error) => boolean;
};
Expand Down Expand Up @@ -108,9 +108,9 @@ export class JsonRpcHttpClient implements IJsonRpcHttpClient {
jwtId?: string;
/** If jwtSecret and jwtVersion are provided, jwtVersion will be included in JwtClaim.clv. */
jwtVersion?: string;
/** Retry attempts */
retryAttempts?: number;
/** Retry delay, only relevant with retry attempts */
/** Number of retries per request */
retries?: number;
/** Retry delay, only relevant if retries > 0 */
retryDelay?: number;
/** Metrics for retry, could be expanded later */
metrics?: JsonRpcHttpClientMetrics | null;
Expand Down Expand Up @@ -162,7 +162,7 @@ export class JsonRpcHttpClient implements IJsonRpcHttpClient {
return this.fetchJson({jsonrpc: "2.0", id: this.id++, ...payload}, opts);
},
{
retries: opts?.retryAttempts ?? this.opts?.retryAttempts ?? 0,
retries: opts?.retries ?? this.opts?.retries ?? 0,
retryDelay: opts?.retryDelay ?? this.opts?.retryDelay ?? 0,
shouldRetry: opts?.shouldRetry,
signal: this.opts?.signal,
Expand Down
2 changes: 1 addition & 1 deletion packages/beacon-node/src/execution/builder/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export class ExecutionBuilderHttp implements IExecutionBuilder {
async submitBlindedBlock(
signedBlindedBlock: allForks.SignedBlindedBeaconBlock
): Promise<allForks.SignedBeaconBlockOrContents> {
const res = await this.api.submitBlindedBlock(signedBlindedBlock, {retryAttempts: 2});
const res = await this.api.submitBlindedBlock(signedBlindedBlock, {retries: 2});
ApiError.assert(res, "execution.builder.submitBlindedBlock");
const {data} = res.response;

Expand Down
6 changes: 3 additions & 3 deletions packages/beacon-node/src/execution/engine/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export type ExecutionEngineModules = {

export type ExecutionEngineHttpOpts = {
urls: string[];
retryAttempts: number;
retries: number;
retryDelay: number;
timeout?: number;
/**
Expand All @@ -72,7 +72,7 @@ export const defaultExecutionEngineHttpOpts: ExecutionEngineHttpOpts = {
* port/url, one can override this and skip providing a jwt secret.
*/
urls: ["http://localhost:8551"],
retryAttempts: 2,
retries: 2,
retryDelay: 2000,
timeout: 12000,
};
Expand Down Expand Up @@ -305,7 +305,7 @@ export class ExecutionEngineHttp implements IExecutionEngine {
// If we are just fcUing and not asking execution for payload, retry is not required
// and we can move on, as the next fcU will be issued soon on the new slot
const fcUReqOpts =
payloadAttributes !== undefined ? forkchoiceUpdatedV1Opts : {...forkchoiceUpdatedV1Opts, retryAttempts: 0};
payloadAttributes !== undefined ? forkchoiceUpdatedV1Opts : {...forkchoiceUpdatedV1Opts, retries: 0};

const request = this.rpcFetchQueue.push({
method,
Expand Down
32 changes: 16 additions & 16 deletions packages/beacon-node/test/e2e/eth1/jsonRpcHttpClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,43 +185,43 @@ describe("eth1 / jsonRpcHttpClient - with retries", function () {

const url = "https://goerli.fake-website.io";
const payload = {method: "get", params: []};
const retryAttempts = 2;
const retries = 2;

const controller = new AbortController();
const eth1JsonRpcClient = new JsonRpcHttpClient([url], {signal: controller.signal});
await expect(
eth1JsonRpcClient.fetchWithRetries(payload, {
retryAttempts,
retries,
shouldRetry: () => {
// using the shouldRetry function to keep tab of the retried requests
retryCount++;
return true;
},
})
).rejects.toThrow("getaddrinfo ENOTFOUND");
expect(retryCount).toBeWithMessage(retryAttempts, "ENOTFOUND should be retried before failing");
expect(retryCount).toBeWithMessage(retries, "ENOTFOUND should be retried before failing");
});

it("should retry ECONNREFUSED", async function () {
let retryCount = 0;

const url = `http://localhost:${port + 1}`;
const payload = {method: "get", params: []};
const retryAttempts = 2;
const retries = 2;

const controller = new AbortController();
const eth1JsonRpcClient = new JsonRpcHttpClient([url], {signal: controller.signal});
await expect(
eth1JsonRpcClient.fetchWithRetries(payload, {
retryAttempts,
retries,
shouldRetry: () => {
// using the shouldRetry function to keep tab of the retried requests
retryCount++;
return true;
},
})
).rejects.toThrow(expect.objectContaining({code: "ECONNREFUSED"}));
expect(retryCount).toBeWithMessage(retryAttempts, "code ECONNREFUSED should be retried before failing");
expect(retryCount).toBeWithMessage(retries, "code ECONNREFUSED should be retried before failing");
});

it("should retry 404", async function () {
Expand All @@ -246,12 +246,12 @@ describe("eth1 / jsonRpcHttpClient - with retries", function () {

const url = `http://localhost:${port}`;
const payload = {method: "get", params: []};
const retryAttempts = 2;
const retries = 2;

const controller = new AbortController();
const eth1JsonRpcClient = new JsonRpcHttpClient([url], {signal: controller.signal});
await expect(eth1JsonRpcClient.fetchWithRetries(payload, {retryAttempts})).rejects.toThrow("Not Found");
expect(requestCount).toBeWithMessage(retryAttempts + 1, "404 responses should be retried before failing");
await expect(eth1JsonRpcClient.fetchWithRetries(payload, {retries})).rejects.toThrow("Not Found");
expect(requestCount).toBeWithMessage(retries + 1, "404 responses should be retried before failing");
});

it("should retry timeout", async function () {
Expand All @@ -276,15 +276,15 @@ describe("eth1 / jsonRpcHttpClient - with retries", function () {

const url = `http://localhost:${port}`;
const payload = {method: "get", params: []};
const retryAttempts = 2;
const retries = 2;
const timeout = 2000;

const controller = new AbortController();
const eth1JsonRpcClient = new JsonRpcHttpClient([url], {signal: controller.signal});
await expect(eth1JsonRpcClient.fetchWithRetries(payload, {retryAttempts, timeout})).rejects.toThrow(
await expect(eth1JsonRpcClient.fetchWithRetries(payload, {retries, timeout})).rejects.toThrow(
"Timeout request"
);
expect(requestCount).toBeWithMessage(retryAttempts + 1, "Timeout request should be retried before failing");
expect(requestCount).toBeWithMessage(retries + 1, "Timeout request should be retried before failing");
});

it("should retry aborted", async function () {
Expand All @@ -307,13 +307,13 @@ describe("eth1 / jsonRpcHttpClient - with retries", function () {

const url = `http://localhost:${port}`;
const payload = {method: "get", params: []};
const retryAttempts = 2;
const retries = 2;
const timeout = 2000;

const controller = new AbortController();
setTimeout(() => controller.abort(), 50);
const eth1JsonRpcClient = new JsonRpcHttpClient([url], {signal: controller.signal});
await expect(eth1JsonRpcClient.fetchWithRetries(payload, {retryAttempts, timeout})).rejects.toThrow("Aborted");
await expect(eth1JsonRpcClient.fetchWithRetries(payload, {retries, timeout})).rejects.toThrow("Aborted");
expect(requestCount).toBeWithMessage(1, "Aborted request should be retried before failing");
});

Expand All @@ -339,11 +339,11 @@ describe("eth1 / jsonRpcHttpClient - with retries", function () {

const url = `http://localhost:${port}`;
const payload = {method: "get", params: []};
const retryAttempts = 2;
const retries = 2;

const controller = new AbortController();
const eth1JsonRpcClient = new JsonRpcHttpClient([url], {signal: controller.signal});
await expect(eth1JsonRpcClient.fetchWithRetries(payload, {retryAttempts})).rejects.toThrow("Method not found");
await expect(eth1JsonRpcClient.fetchWithRetries(payload, {retries})).rejects.toThrow("Method not found");
expect(requestCount).toBeWithMessage(1, "Payload error (non-network error) should not be retried");
});
}, {timeout: 10_000});
4 changes: 2 additions & 2 deletions packages/beacon-node/test/sim/merge-interop.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ import {shell} from "./shell.js";
const terminalTotalDifficultyPreMerge = 10;
const TX_SCENARIOS = process.env.TX_SCENARIOS?.split(",") || [];
const jwtSecretHex = "0xdc6457099f127cf0bac78de8b297df04951281909db4f58b43def7c7151e765d";
const retryAttempts = defaultExecutionEngineHttpOpts.retryAttempts;
const retries = defaultExecutionEngineHttpOpts.retries;
const retryDelay = defaultExecutionEngineHttpOpts.retryDelay;

describe("executionEngine / ExecutionEngineHttp", function () {
Expand Down Expand Up @@ -110,7 +110,7 @@ describe("executionEngine / ExecutionEngineHttp", function () {

//const controller = new AbortController();
const executionEngine = initializeExecutionEngine(
{mode: "http", urls: [engineRpcUrl], jwtSecretHex, retryAttempts, retryDelay},
{mode: "http", urls: [engineRpcUrl], jwtSecretHex, retries, retryDelay},
{signal: controller.signal, logger: testLogger("Node-A-Engine")}
);

Expand Down
4 changes: 2 additions & 2 deletions packages/beacon-node/test/sim/withdrawal-interop.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {shell} from "./shell.js";
/* eslint-disable no-console, @typescript-eslint/naming-convention */

const jwtSecretHex = "0xdc6457099f127cf0bac78de8b297df04951281909db4f58b43def7c7151e765d";
const retryAttempts = defaultExecutionEngineHttpOpts.retryAttempts;
const retries = defaultExecutionEngineHttpOpts.retries;
const retryDelay = defaultExecutionEngineHttpOpts.retryDelay;

describe("executionEngine / ExecutionEngineHttp", function () {
Expand Down Expand Up @@ -82,7 +82,7 @@ describe("executionEngine / ExecutionEngineHttp", function () {

//const controller = new AbortController();
const executionEngine = initializeExecutionEngine(
{mode: "http", urls: [engineRpcUrl], jwtSecretHex, retryAttempts, retryDelay},
{mode: "http", urls: [engineRpcUrl], jwtSecretHex, retries, retryDelay},
{signal: controller.signal, logger: testLogger("executionEngine")}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe("ExecutionEngine / http", () => {
{
mode: "http",
urls: [baseUrl],
retryAttempts: defaultExecutionEngineHttpOpts.retryAttempts,
retries: defaultExecutionEngineHttpOpts.retries,
retryDelay: defaultExecutionEngineHttpOpts.retryDelay,
},
{signal: controller.signal, logger: console as unknown as Logger}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe("ExecutionEngine / http ", () => {
{
mode: "http",
urls: [baseUrl],
retryAttempts: defaultExecutionEngineHttpOpts.retryAttempts,
retries: defaultExecutionEngineHttpOpts.retries,
retryDelay: defaultExecutionEngineHttpOpts.retryDelay,
},
{signal: controller.signal, logger: console as unknown as Logger}
Expand Down Expand Up @@ -86,7 +86,7 @@ describe("ExecutionEngine / http ", () => {
});

it("notifyForkchoiceUpdate with retry when pay load attributes", async function () {
errorResponsesBeforeSuccess = defaultExecutionEngineHttpOpts.retryAttempts - 1;
errorResponsesBeforeSuccess = defaultExecutionEngineHttpOpts.retries - 1;
const forkChoiceHeadData = {
headBlockHash: "0xb084c10440f05f5a23a55d1d7ebcb1b3892935fb56f23cdc9a7f42c348eed174",
safeBlockHash: "0xb084c10440f05f5a23a55d1d7ebcb1b3892935fb56f23cdc9a7f42c348eed174",
Expand Down
11 changes: 6 additions & 5 deletions packages/cli/src/options/beaconNodeOptions/execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {CliCommandOptions, extractJwtHexSecret} from "../../util/index.js";
export type ExecutionEngineArgs = {
"execution.urls": string[];
"execution.timeout"?: number;
"execution.retryAttempts": number;
"execution.retries": number;
"execution.retryDelay": number;
"execution.engineMock"?: boolean;
jwtSecret?: string;
Expand All @@ -23,7 +23,7 @@ export function parseArgs(args: ExecutionEngineArgs): IBeaconNodeOptions["execut
return {
urls: args["execution.urls"],
timeout: args["execution.timeout"],
retryAttempts: args["execution.retryAttempts"],
retries: args["execution.retries"],
retryDelay: args["execution.retryDelay"],
/**
* jwtSecret is parsed as hex instead of bytes because the merge with defaults
Expand Down Expand Up @@ -55,10 +55,11 @@ export const options: CliCommandOptions<ExecutionEngineArgs> = {
group: "execution",
},

"execution.retryAttempts": {
description: "Number of retry attempts when calling execution engine API",
"execution.retries": {
alias: ["execution.retryAttempts"],
description: "Number of retries when calling execution engine API",
type: "number",
default: defaultExecutionEngineHttpOpts.retryAttempts,
default: defaultExecutionEngineHttpOpts.retries,
group: "execution",
},

Expand Down
4 changes: 2 additions & 2 deletions packages/cli/test/unit/options/beaconNodeOptions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe("options / beaconNodeOptions", () => {
"execution.urls": ["http://localhost:8551"],
"execution.timeout": 12000,
"execution.retryDelay": 2000,
"execution.retryAttempts": 1,
"execution.retries": 1,

builder: false,
"builder.url": "http://localhost:8661",
Expand Down Expand Up @@ -153,7 +153,7 @@ describe("options / beaconNodeOptions", () => {
},
executionEngine: {
urls: ["http://localhost:8551"],
retryAttempts: 1,
retries: 1,
retryDelay: 2000,
timeout: 12000,
},
Expand Down

0 comments on commit c999e4a

Please sign in to comment.