Skip to content

Commit

Permalink
feat: add logs to track number of http retries (#6458)
Browse files Browse the repository at this point in the history
  • Loading branch information
nflaig committed Feb 20, 2024
1 parent 0a12b1b commit d1dc217
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 35 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 @@ -8,10 +8,10 @@ import {HttpStatusCode} from "./httpStatusCode.js";
/* eslint-disable @typescript-eslint/no-explicit-any */

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

export type ApiWithExtraOpts<T extends Record<string, APIClientHandler>> = {
[K in keyof T]: (...args: ParamatersWithOptionalExtaOpts<T[K]>) => ReturnType<T[K]>;
[K in keyof T]: (...args: ParametersWithOptionalExtraOpts<T[K]>) => ReturnType<T[K]>;
};

// See /packages/api/src/routes/index.ts for reasoning
Expand Down Expand Up @@ -71,7 +71,7 @@ export function generateGenericJsonClient<
const returnType = returnTypes[routeId as keyof ReturnTypes<Api>] as TypeJson<any> | null;

return async function request(
...args: ParamatersWithOptionalExtaOpts<Api[keyof Api]>
...args: ParametersWithOptionalExtraOpts<Api[keyof Api]>
): Promise<ReturnType<Api[keyof Api]>> {
try {
// extract the extraOpts if provided
Expand Down
10 changes: 9 additions & 1 deletion packages/api/src/utils/client/httpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,11 +184,19 @@ export class HttpClient implements IHttpClient {
getBody: (res: Response) => Promise<T>
): Promise<{status: HttpStatusCode; body: T}> {
if (opts.retryAttempts !== undefined) {
const routeId = opts.routeId ?? DEFAULT_ROUTE_ID;

return retry(
async (_attempt) => {
return this.requestWithBodyWithFallbacks<T>(opts, getBody);
},
{retries: opts.retryAttempts, retryDelay: 200}
{
retries: opts.retryAttempts,
retryDelay: 200,
onRetry: (e, attempt) => {
this.logger?.debug("Retrying request", {routeId, attempt, lastError: e.message});
},
}
);
} else {
return this.requestWithBodyWithFallbacks<T>(opts, getBody);
Expand Down
9 changes: 4 additions & 5 deletions packages/beacon-node/src/eth1/provider/jsonRpcHttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,18 +158,17 @@ export class JsonRpcHttpClient implements IJsonRpcHttpClient {
const routeId = opts?.routeId ?? "unknown";

const res = await retry<RpcResponse<R>>(
async (attempt) => {
/** If this is a retry, increment the retry counter for this method */
if (attempt > 1) {
this.opts?.metrics?.retryCount.inc({routeId});
}
async (_attempt) => {
return this.fetchJson({jsonrpc: "2.0", id: this.id++, ...payload}, opts);
},
{
retries: opts?.retryAttempts ?? this.opts?.retryAttempts ?? 1,
retryDelay: opts?.retryDelay ?? this.opts?.retryDelay ?? 0,
shouldRetry: opts?.shouldRetry,
signal: this.opts?.signal,
onRetry: () => {
this.opts?.metrics?.retryCount.inc({routeId});
},
}
);
return parseRpcResponse(res, payload);
Expand Down
2 changes: 0 additions & 2 deletions packages/spec-test-util/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,13 @@
],
"dependencies": {
"@lodestar/utils": "^1.15.1",
"async-retry": "^1.3.3",
"axios": "^1.3.4",
"rimraf": "^4.4.1",
"snappyjs": "^0.7.0",
"tar": "^6.1.13",
"vitest": "^1.2.1"
},
"devDependencies": {
"@types/async-retry": "1.4.3",
"@types/tar": "^6.1.4"
},
"peerDependencies": {
Expand Down
3 changes: 1 addition & 2 deletions packages/spec-test-util/src/downloadTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {promisify} from "node:util";
import {rimraf} from "rimraf";
import axios from "axios";
import {x as extractTar} from "tar";
import retry from "async-retry";
import {retry} from "@lodestar/utils";

export const defaultSpecTestsRepoUrl = "https://github.com/ethereum/consensus-spec-tests";

Expand Down Expand Up @@ -67,7 +67,6 @@ export async function downloadGenericSpecTests<TestNames extends string>(
const url = `${specTestsRepoUrl ?? defaultSpecTestsRepoUrl}/releases/download/${specVersion}/${test}.tar.gz`;

await retry(
// async (bail) => {
async () => {
const {data, headers} = await axios({
method: "get",
Expand Down
16 changes: 13 additions & 3 deletions packages/utils/src/retry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,17 @@ export type RetryOptions = {
*/
retries?: number;
/**
* An optional Function that is invoked after the provided callback throws
* It expects a boolean to know if it should retry or not
* Useful to make retrying conditional on the type of error thrown
* An optional Function that is invoked after the provided callback throws.
* It expects a boolean to know if it should retry or not.
* Useful to make retrying conditional on the type of error thrown.
*/
shouldRetry?: (lastError: Error) => boolean;
/**
* An optional Function that is invoked right before a retry is performed.
* It's passed the Error that triggered it and a number identifying the attempt.
* Useful to track number of retries and errors in logs or metrics.
*/
onRetry?: (lastError: Error, attempt: number) => unknown;
/**
* Milliseconds to wait before retrying again
*/
Expand All @@ -27,10 +33,14 @@ export type RetryOptions = {
export async function retry<A>(fn: (attempt: number) => A | Promise<A>, opts?: RetryOptions): Promise<A> {
const maxRetries = opts?.retries ?? 5;
const shouldRetry = opts?.shouldRetry;
const onRetry = opts?.onRetry;

let lastError: Error = Error("RetryError");
for (let i = 1; i <= maxRetries; i++) {
try {
// If not the first attempt, invoke right before retrying
if (i > 1) onRetry?.(lastError, i);

return await fn(i);
} catch (e) {
lastError = e as Error;
Expand Down
19 changes: 0 additions & 19 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2810,13 +2810,6 @@
resolved "https://registry.npmjs.org/@types/abstract-leveldown/-/abstract-leveldown-5.0.1.tgz"
integrity sha512-wYxU3kp5zItbxKmeRYCEplS2MW7DzyBnxPGj+GJVHZEUZiK/nn5Ei1sUFgURDh+X051+zsGe28iud3oHjrYWQQ==

"@types/async-retry@1.4.3":
version "1.4.3"
resolved "https://registry.yarnpkg.com/@types/async-retry/-/async-retry-1.4.3.tgz#8b78f6ce88d97e568961732cdd9e5325cdc8c246"
integrity sha512-B3C9QmmNULVPL2uSJQ088eGWTNPIeUk35hca6CV8rRDJ8GXuQJP5CCVWA1ZUCrb9xYP7Js/RkLqnNNwKhe+Zsw==
dependencies:
"@types/retry" "*"

"@types/buffer-xor@^2.0.0":
version "2.0.0"
resolved "https://registry.npmjs.org/@types/buffer-xor/-/buffer-xor-2.0.0.tgz"
Expand Down Expand Up @@ -3965,13 +3958,6 @@ async-lock@^1.4.0:
resolved "https://registry.yarnpkg.com/async-lock/-/async-lock-1.4.0.tgz#c8b6630eff68fbbdd8a5b6eb763dac3bfbb8bf02"
integrity sha512-coglx5yIWuetakm3/1dsX9hxCNox22h7+V80RQOu2XUUMidtArxKoZoOtHUPuR84SycKTXzgGzAUR5hJxujyJQ==

async-retry@^1.3.3:
version "1.3.3"
resolved "https://registry.npmjs.org/async-retry/-/async-retry-1.3.3.tgz"
integrity sha512-wfr/jstw9xNi/0teMHrRW7dsz3Lt5ARhYNZ2ewpadnhaIp5mbALhOAP+EAdsC7t4Z6wqsDVv9+W6gm1Dk9mEyw==
dependencies:
retry "0.13.1"

async@^3.2.3:
version "3.2.4"
resolved "https://registry.yarnpkg.com/async/-/async-3.2.4.tgz#2d22e00f8cddeb5fde5dd33522b56d1cf569a81c"
Expand Down Expand Up @@ -10939,11 +10925,6 @@ ret@~0.2.0:
resolved "https://registry.npmjs.org/ret/-/ret-0.2.2.tgz"
integrity sha512-M0b3YWQs7R3Z917WRQy1HHA7Ba7D8hvZg6UE5mLykJxQVE2ju0IXbGlaHPPlkY+WN7wFP+wUMXmBFA0aV6vYGQ==

retry@0.13.1:
version "0.13.1"
resolved "https://registry.npmjs.org/retry/-/retry-0.13.1.tgz"
integrity sha512-XQBQ3I8W1Cge0Seh+6gjj03LbmRFWuoszgK9ooCpwYIrhhoO80pfq4cUkU5DkknwfOfFteRwlZ56PYOGYyFWdg==

retry@^0.12.0:
version "0.12.0"
resolved "https://registry.yarnpkg.com/retry/-/retry-0.12.0.tgz#1b42a6266a21f07421d1b0b54b7dc167b01c013b"
Expand Down

0 comments on commit d1dc217

Please sign in to comment.