fix: request retries are abortable (#6466)

* Request retries are abortable * Reduce timeouts in tests * Abort retry if not first attempt
ChainSafe · Feb 22, 2024 · 202c1d7 · 202c1d7
1 parent 2f71c32
commit 202c1d7
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 8 deletions.
diff --git a/packages/api/src/utils/client/httpClient.ts b/packages/api/src/utils/client/httpClient.ts
@@ -193,6 +193,7 @@ export class HttpClient implements IHttpClient {
         {
           retries: opts.retries,
           retryDelay: 200,
+          signal: this.getAbortSignal?.(),
           onRetry: (e, attempt) => {
             this.logger?.debug("Retrying request", {routeId, attempt, lastError: e.message});
           },

diff --git a/packages/beacon-node/src/eth1/provider/jsonRpcHttpClient.ts b/packages/beacon-node/src/eth1/provider/jsonRpcHttpClient.ts
@@ -163,7 +163,7 @@ export class JsonRpcHttpClient implements IJsonRpcHttpClient {
         },
         {
           retries: opts?.retries ?? this.opts?.retries ?? 0,
-          retryDelay: opts?.retryDelay ?? this.opts?.retryDelay ?? 0,
+          retryDelay: opts?.retryDelay ?? this.opts?.retryDelay,
           shouldRetry: opts?.shouldRetry,
           signal: this.opts?.signal,
           onRetry: () => {

diff --git a/packages/beacon-node/test/e2e/eth1/jsonRpcHttpClient.test.ts b/packages/beacon-node/test/e2e/eth1/jsonRpcHttpClient.test.ts
@@ -277,15 +277,15 @@ describe("eth1 / jsonRpcHttpClient - with retries", function () {
     const url = `http://localhost:${port}`;
     const payload = {method: "get", params: []};
     const retries = 2;
-    const timeout = 2000;
+    const timeout = 200;
 
     const controller = new AbortController();
     const eth1JsonRpcClient = new JsonRpcHttpClient([url], {signal: controller.signal});
     await expect(eth1JsonRpcClient.fetchWithRetries(payload, {retries, timeout})).rejects.toThrow("Timeout request");
     expect(requestCount).toBeWithMessage(retries + 1, "Timeout request should be retried before failing");
   });
 
-  it("should retry aborted", async function () {
+  it("should not retry aborted", async function () {
     let requestCount = 0;
     const server = http.createServer(() => {
       requestCount++;
@@ -306,13 +306,13 @@ describe("eth1 / jsonRpcHttpClient - with retries", function () {
     const url = `http://localhost:${port}`;
     const payload = {method: "get", params: []};
     const retries = 2;
-    const timeout = 2000;
+    const timeout = 200;
 
     const controller = new AbortController();
     setTimeout(() => controller.abort(), 50);
     const eth1JsonRpcClient = new JsonRpcHttpClient([url], {signal: controller.signal});
     await expect(eth1JsonRpcClient.fetchWithRetries(payload, {retries, timeout})).rejects.toThrow("Aborted");
-    expect(requestCount).toBeWithMessage(1, "Aborted request should be retried before failing");
+    expect(requestCount).toBeWithMessage(1, "Aborted request should not be retried");
   });
 
   it("should not retry payload error", async function () {

diff --git a/packages/utils/src/retry.ts b/packages/utils/src/retry.ts
@@ -1,3 +1,4 @@
+import {ErrorAborted} from "./errors.js";
 import {sleep} from "./sleep.js";
 
 export type RetryOptions = {
@@ -21,6 +22,9 @@ export type RetryOptions = {
    * Milliseconds to wait before retrying again
    */
   retryDelay?: number;
+  /**
+   * Abort signal to stop retrying
+   */
   signal?: AbortSignal;
 };
 
@@ -39,10 +43,16 @@ export async function retry<A>(fn: (attempt: number) => A | Promise<A>, opts?: R
 
   let lastError: Error = Error("RetryError");
   for (let i = 1; i <= maxAttempts; i++) {
-    try {
-      // If not the first attempt, invoke right before retrying
-      if (i > 1) onRetry?.(lastError, i);
+    // If not the first attempt
+    if (i > 1) {
+      if (opts?.signal?.aborted) {
+        throw new ErrorAborted("retry");
+      }
+      // Invoke right before retrying
+      onRetry?.(lastError, i);
+    }
 
+    try {
       return await fn(i);
     } catch (e) {
       lastError = e as Error;