From d3b177bd06967716e08531d6ca035ec50352e443 Mon Sep 17 00:00:00 2001 From: harkamal Date: Fri, 2 Feb 2024 15:55:48 +0530 Subject: [PATCH 1/3] feat: add retry functionality in the api for builder publish --- packages/api/src/beacon/client/beacon.ts | 2 +- packages/api/src/builder/client.ts | 4 +- packages/api/src/builder/index.ts | 13 +++++-- packages/api/src/utils/client/client.ts | 37 +++++++++++++++---- packages/api/src/utils/client/httpClient.ts | 15 +++++++- .../beacon-node/src/execution/builder/http.ts | 2 +- 6 files changed, 57 insertions(+), 16 deletions(-) diff --git a/packages/api/src/beacon/client/beacon.ts b/packages/api/src/beacon/client/beacon.ts index 7a92afe15c6f..ef1e1983577d 100644 --- a/packages/api/src/beacon/client/beacon.ts +++ b/packages/api/src/beacon/client/beacon.ts @@ -11,7 +11,7 @@ export function getClient(config: ChainForkConfig, httpClient: IHttpClient): Api const reqSerializers = getReqSerializers(config); const returnTypes = getReturnTypes(); // Some routes return JSON, use a client auto-generator - const client = generateGenericJsonClient(routesData, reqSerializers, returnTypes, httpClient); + const client = generateGenericJsonClient(routesData, reqSerializers, returnTypes, httpClient) as Api; const fetchOptsSerializer = getFetchOptsSerializers(routesData, reqSerializers); return { diff --git a/packages/api/src/builder/client.ts b/packages/api/src/builder/client.ts index ed344b9c0e7f..381732b81433 100644 --- a/packages/api/src/builder/client.ts +++ b/packages/api/src/builder/client.ts @@ -1,11 +1,11 @@ import {ChainForkConfig} from "@lodestar/config"; -import {IHttpClient, generateGenericJsonClient} from "../utils/client/index.js"; +import {IHttpClient, generateGenericJsonClient, ApiWithExtraOpts} from "../utils/client/index.js"; import {Api, ReqTypes, routesData, getReqSerializers, getReturnTypes} from "./routes.js"; /** * REST HTTP client for builder routes */ -export function getClient(config: ChainForkConfig, httpClient: IHttpClient): Api { +export function getClient(config: ChainForkConfig, httpClient: IHttpClient): ApiWithExtraOpts { const reqSerializers = getReqSerializers(config); const returnTypes = getReturnTypes(); // All routes return JSON, use a client auto-generator diff --git a/packages/api/src/builder/index.ts b/packages/api/src/builder/index.ts index 76100fba2057..531ceac1b624 100644 --- a/packages/api/src/builder/index.ts +++ b/packages/api/src/builder/index.ts @@ -1,14 +1,19 @@ import {ChainForkConfig} from "@lodestar/config"; -import {HttpClient, HttpClientModules, HttpClientOptions, IHttpClient} from "../utils/client/httpClient.js"; -import {Api} from "./routes.js"; +import { + HttpClient, + HttpClientModules, + HttpClientOptions, + IHttpClient, + ApiWithExtraOpts, +} from "../utils/client/index.js"; +import {Api as BuilderApi} from "../builder/routes.js"; import * as builder from "./client.js"; // NOTE: Don't export server here so it's not bundled to all consumers -export type {Api}; - // Note: build API does not have namespaces as routes are declared at the "root" namespace +export type Api = ApiWithExtraOpts; type ClientModules = HttpClientModules & { config: ChainForkConfig; httpClient?: IHttpClient; diff --git a/packages/api/src/utils/client/client.ts b/packages/api/src/utils/client/client.ts index a88b68553898..1782ceea57fd 100644 --- a/packages/api/src/utils/client/client.ts +++ b/packages/api/src/utils/client/client.ts @@ -5,10 +5,17 @@ import {APIClientHandler} from "../../interfaces.js"; import {FetchOpts, HttpError, IHttpClient} from "./httpClient.js"; import {HttpStatusCode} from "./httpStatusCode.js"; -// See /packages/api/src/routes/index.ts for reasoning - /* eslint-disable @typescript-eslint/no-explicit-any */ +type ExtraOpts = {retryAttempts?: number}; +type ParamatersWithOptionalExtaOpts any> = [...Parameters, ExtraOpts] | Parameters; + +export type ApiWithExtraOpts> = { + [K in keyof T]: (...args: ParamatersWithOptionalExtaOpts) => ReturnType; +}; + +// See /packages/api/src/routes/index.ts for reasoning + /** * Format FetchFn opts from Fn arguments given a route definition and request serializer. * For routes that return only JSOn use @see getGenericJsonClient @@ -58,22 +65,38 @@ export function generateGenericJsonClient< reqSerializers: ReqSerializers, returnTypes: ReturnTypes, fetchFn: IHttpClient -): Api { +): ApiWithExtraOpts { return mapValues(routesData, (routeDef, routeId) => { const fetchOptsSerializer = getFetchOptsSerializer(routeDef, reqSerializers[routeId], routeId as string); const returnType = returnTypes[routeId as keyof ReturnTypes] as TypeJson | null; - return async function request(...args: Parameters): Promise> { + return async function request( + ...args: ParamatersWithOptionalExtaOpts + ): Promise> { try { + // extract the extraOpts if provided + // + const argLen = (args as any[])?.length ?? 0; + const lastArg = (args as any[])[argLen] as ExtraOpts | undefined; + const retryAttempts = lastArg?.retryAttempts; + const extraOpts = {retryAttempts}; + if (returnType) { - const res = await fetchFn.json(fetchOptsSerializer(...args)); + // open extraOpts first if some serializer wants to add some overriding param + const res = await fetchFn.json({ + ...extraOpts, + ...fetchOptsSerializer(...(args as Parameters)), + }); // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/return-await return {ok: true, response: returnType.fromJson(res.body), status: res.status} as ReturnType; } else { // We need to avoid parsing the response as the servers might just // response status 200 and close the request instead of writing an // empty json response. We return the status code. - const res = await fetchFn.request(fetchOptsSerializer(...args)); + const res = await fetchFn.request({ + ...extraOpts, + ...fetchOptsSerializer(...(args as Parameters)), + }); // eslint-disable-next-line @typescript-eslint/return-await return {ok: true, response: undefined, status: res.status} as ReturnType; @@ -98,5 +121,5 @@ export function generateGenericJsonClient< throw err; } }; - }) as unknown as Api; + }) as unknown as ApiWithExtraOpts; } diff --git a/packages/api/src/utils/client/httpClient.ts b/packages/api/src/utils/client/httpClient.ts index c0fd2b8ad1aa..e9679204650e 100644 --- a/packages/api/src/utils/client/httpClient.ts +++ b/packages/api/src/utils/client/httpClient.ts @@ -1,4 +1,4 @@ -import {ErrorAborted, Logger, TimeoutError, isValidHttpUrl, toBase64} from "@lodestar/utils"; +import {ErrorAborted, Logger, TimeoutError, isValidHttpUrl, toBase64, retry} from "@lodestar/utils"; import {ReqGeneric, RouteDef} from "../index.js"; import {ApiClientResponse, ApiClientSuccessResponse} from "../../interfaces.js"; import {fetch, isFetchError} from "./fetch.js"; @@ -70,6 +70,7 @@ export type FetchOpts = { /** Optional, for metrics */ routeId?: string; timeoutMs?: number; + retryAttempts?: number; }; export interface IHttpClient { @@ -181,6 +182,18 @@ export class HttpClient implements IHttpClient { private async requestWithBodyWithRetries( opts: FetchOpts, getBody: (res: Response) => Promise + ): Promise<{status: HttpStatusCode; body: T}> { + return retry( + async (_attempt) => { + return this.requestWithBodyWithFallbacks(opts, getBody); + }, + {retries: opts?.retryAttempts ?? 1, retryDelay: 10} + ); + } + + private async requestWithBodyWithFallbacks( + opts: FetchOpts, + getBody: (res: Response) => Promise ): Promise<{status: HttpStatusCode; body: T}> { // Early return when no fallback URLs are setup if (this.urlsOpts.length === 1) { diff --git a/packages/beacon-node/src/execution/builder/http.ts b/packages/beacon-node/src/execution/builder/http.ts index c47e8471f199..b4013c384f81 100644 --- a/packages/beacon-node/src/execution/builder/http.ts +++ b/packages/beacon-node/src/execution/builder/http.ts @@ -118,7 +118,7 @@ export class ExecutionBuilderHttp implements IExecutionBuilder { async submitBlindedBlock( signedBlindedBlock: allForks.SignedBlindedBeaconBlock ): Promise { - const res = await this.api.submitBlindedBlock(signedBlindedBlock); + const res = await this.api.submitBlindedBlock(signedBlindedBlock, {retryAttempts: 3}); ApiError.assert(res, "execution.builder.submitBlindedBlock"); const {data} = res.response; From ef8f1e8f095e35fd42625922515337e1faeae629 Mon Sep 17 00:00:00 2001 From: harkamal Date: Sat, 17 Feb 2024 22:07:21 +0530 Subject: [PATCH 2/3] update retry delay --- packages/api/src/utils/client/httpClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/src/utils/client/httpClient.ts b/packages/api/src/utils/client/httpClient.ts index e9679204650e..9d3944b12370 100644 --- a/packages/api/src/utils/client/httpClient.ts +++ b/packages/api/src/utils/client/httpClient.ts @@ -187,7 +187,7 @@ export class HttpClient implements IHttpClient { async (_attempt) => { return this.requestWithBodyWithFallbacks(opts, getBody); }, - {retries: opts?.retryAttempts ?? 1, retryDelay: 10} + {retries: opts?.retryAttempts ?? 1, retryDelay: 200} ); } From 4fdb8137c7270b40a1b86ed572231a5c8db5b938 Mon Sep 17 00:00:00 2001 From: harkamal Date: Sat, 17 Feb 2024 22:26:05 +0530 Subject: [PATCH 3/3] retry logic when retires mentioned --- packages/api/src/utils/client/httpClient.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/api/src/utils/client/httpClient.ts b/packages/api/src/utils/client/httpClient.ts index 9d3944b12370..57214c1ae04a 100644 --- a/packages/api/src/utils/client/httpClient.ts +++ b/packages/api/src/utils/client/httpClient.ts @@ -183,12 +183,16 @@ export class HttpClient implements IHttpClient { opts: FetchOpts, getBody: (res: Response) => Promise ): Promise<{status: HttpStatusCode; body: T}> { - return retry( - async (_attempt) => { - return this.requestWithBodyWithFallbacks(opts, getBody); - }, - {retries: opts?.retryAttempts ?? 1, retryDelay: 200} - ); + if (opts.retryAttempts !== undefined) { + return retry( + async (_attempt) => { + return this.requestWithBodyWithFallbacks(opts, getBody); + }, + {retries: opts.retryAttempts, retryDelay: 200} + ); + } else { + return this.requestWithBodyWithFallbacks(opts, getBody); + } } private async requestWithBodyWithFallbacks(