Skip to content
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.

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

chore: use term 'retries' instead of 'retryAttempts' consistently #6465

Merged
merged 2 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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