refactor awaitRequiredStatus (#483)

* refactor awaitRequiredStatus * Update SmartContractsClient.ts --------- Co-authored-by: Ben <br@massa.net>
massalabs · Oct 17, 2023 · 286d326 · 286d326
1 parent c5379d4
commit 286d326
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 145 deletions.
diff --git a/packages/massa-web3/src/utils/retryExecuteFunction.ts b/packages/massa-web3/src/utils/retryExecuteFunction.ts
@@ -1,7 +1,9 @@
-import { wait } from './time';
 import { JSON_RPC_REQUEST_METHOD } from '../interfaces/JsonRpcMethods';
+import { wait } from './time';
 
 const MAX_NUMBER_RETRIALS = 5;
+const RETRY_INTERVAL_MS = 300;
+
 type CallbackFunction<R> = (
   resource: JSON_RPC_REQUEST_METHOD,
   params: object,
@@ -37,11 +39,10 @@ export const trySafeExecute = async <R>(
       res = await func(...args);
       break;
     } catch (ex) {
-      const msg = `Failed to execute function ${
-        func.name
-      }. Retrying for ${++failureCounter}th time in 1s.`;
+      ++failureCounter;
+      const msg = `Failed to execute function ${func.name}. Retrying for ${failureCounter}th time in ${RETRY_INTERVAL_MS}ms.`;
       console.error(msg);
-      await wait(200 * (failureCounter + 1));
+      await wait(RETRY_INTERVAL_MS);
 
       if (failureCounter === retryTimes) {
         throw ex;

diff --git a/packages/massa-web3/src/utils/time.ts b/packages/massa-web3/src/utils/time.ts
@@ -1,3 +1,7 @@
+export function wait(ms: number) {
+  return new Promise((resolve) => setTimeout(resolve, ms));
+}
+
 /**
  * A class representing a timeout that triggers a callback function after a specified time interval.
  */
@@ -75,22 +79,6 @@ export class Interval {
   }
 }
 
-/**
- * Returns a promise that resolves after the specified time interval.
- *
- * @param timeMilli - The time interval in milliseconds.
- *
- * @returns A promise that resolves after the specified time interval.
- */
-export const wait = async (timeMilli: number): Promise<void> => {
-  return new Promise<void>((resolve) => {
-    const timeout = new Timeout(timeMilli, () => {
-      timeout.clear();
-      return resolve();
-    });
-  });
-};
-
 /**
  * Returns a promise that resolves after the specified time interval and throws an error if the
  * specified promise does not resolve before the timeout interval.

diff --git a/packages/massa-web3/src/web3/SmartContractsClient.ts b/packages/massa-web3/src/web3/SmartContractsClient.ts
@@ -22,7 +22,6 @@ import { ISmartContractsClient } from '../interfaces/ISmartContractsClient';
 import { JSON_RPC_REQUEST_METHOD } from '../interfaces/JsonRpcMethods';
 import { fromMAS } from '../utils/converters';
 import { trySafeExecute } from '../utils/retryExecuteFunction';
-import { wait } from '../utils/time';
 import { BaseClient } from './BaseClient';
 import { PublicApiClient } from './PublicApiClient';
 import { IWalletClient } from '../interfaces/IWalletClient';
@@ -33,10 +32,12 @@ import {
   IEvent,
   Args,
 } from '@massalabs/web3-utils';
+import { wait } from '../utils/time';
 
-const MAX_READ_BLOCK_GAS = BigInt(4_294_967_295);
-const TX_POLL_INTERVAL_MS = 10000;
-const TX_STATUS_CHECK_RETRY_COUNT = 100;
+export const MAX_READ_BLOCK_GAS = BigInt(4_294_967_295);
+
+const WAIT_STATUS_TIMEOUT = 60000;
+const TX_POLL_INTERVAL_MS = 1000;
 
 /**
  * The key name (as a string) to look for when we are retrieving the proto file from a contract
@@ -376,35 +377,25 @@ export class SmartContractsClient
     opId: string,
     requiredStatus: EOperationStatus,
   ): Promise<EOperationStatus> {
-    let errCounter = 0;
-    let pendingCounter = 0;
-    while (true) {
-      let status = EOperationStatus.NOT_FOUND;
-      try {
-        status = await this.getOperationStatus(opId);
-      } catch (ex) {
-        if (++errCounter > 100) {
-          const msg = `Failed to retrieve the tx status after 100 failed attempts for operation id: ${opId}.`;
-          console.error(msg, ex);
-          throw ex;
-        }
+    const startTime = Date.now();
 
-        await wait(TX_POLL_INTERVAL_MS);
-      }
+    while (Date.now() - startTime < WAIT_STATUS_TIMEOUT) {
+      let currentStatus = EOperationStatus.NOT_FOUND;
 
-      if (status == requiredStatus) {
-        return status;
-      }
+      try {
+        currentStatus = await this.getOperationStatus(opId);
 
-      if (++pendingCounter > 1000) {
-        const msg = `Getting the tx status for operation Id ${opId} took too long to conclude. We gave up after ${
-          TX_POLL_INTERVAL_MS * TX_STATUS_CHECK_RETRY_COUNT
-        }ms.`;
-        console.warn(msg);
-        throw new Error(msg);
+        if (currentStatus === requiredStatus) {
+          return currentStatus;
+        }
+      } catch (ex) {
+        console.warn(ex);
       }
-
       await wait(TX_POLL_INTERVAL_MS);
     }
+
+    throw new Error(
+      `Failed to retrieve status of operation id: ${opId}: Timeout reached.`,
+    );
   }
 }
diff --git a/packages/massa-web3/test/utils/retryExecuteFunction.spec.ts b/packages/massa-web3/test/utils/retryExecuteFunction.spec.ts
@@ -1,6 +1,5 @@
 import { JSON_RPC_REQUEST_METHOD } from '../../src/interfaces/JsonRpcMethods';
 import { trySafeExecute } from '../../src/utils/retryExecuteFunction';
-import { wait } from '../../src/utils/time';
 
 jest.mock('../../src/utils/time');
 
@@ -12,9 +11,6 @@ describe('trySafeExecute function', () => {
 
   beforeEach(() => {
     jest.spyOn(global, 'setTimeout');
-    (wait as jest.Mock).mockImplementation((delay: number) => {
-      return new Promise((resolve) => setTimeout(resolve, delay));
-    });
   });
 
   afterEach(() => {

diff --git a/packages/massa-web3/test/utils/time.spec.ts b/packages/massa-web3/test/utils/time.spec.ts
@@ -1,9 +1,4 @@
-import {
-  Timeout,
-  Interval,
-  wait,
-  withTimeoutRejection,
-} from '../../src/utils/time';
+import { Timeout, Interval, withTimeoutRejection } from '../../src/utils/time';
 
 describe('Timer utilities', () => {
   jest.useFakeTimers();
@@ -56,15 +51,6 @@ describe('Timer utilities', () => {
     });
   });
 
-  describe('wait function', () => {
-    it('should resolve after the specified time', async () => {
-      const promise = wait(1000);
-      jest.advanceTimersByTime(1000);
-
-      await expect(promise).resolves.toBeUndefined();
-    });
-  });
-
   describe('withTimeoutRejection function', () => {
     it('should resolve with the value of the promise if it resolves before the timeout', async () => {
       const promise = Promise.resolve('success');

diff --git a/packages/massa-web3/test/web3/smartContractsClient.spec.ts b/packages/massa-web3/test/web3/smartContractsClient.spec.ts
@@ -4,7 +4,10 @@ import { JSON_RPC_REQUEST_METHOD } from '../../src/interfaces/JsonRpcMethods';
 import { EOperationStatus } from '../../src/interfaces/EOperationStatus';
 import { fromMAS } from '../../src/utils/converters';
 import { PublicApiClient } from '../../src/web3/PublicApiClient';
-import { SmartContractsClient } from '../../src/web3/SmartContractsClient';
+import {
+  MAX_READ_BLOCK_GAS,
+  SmartContractsClient,
+} from '../../src/web3/SmartContractsClient';
 import { WalletClient } from '../../src/web3/WalletClient';
 import {
   mockClientConfig,
@@ -29,15 +32,11 @@ import {
 import { IExecuteReadOnlyResponse } from '../../src/interfaces/IExecuteReadOnlyResponse';
 import { Web3Account } from '../../src/web3/accounts/Web3Account';
 
-const MAX_READ_BLOCK_GAS = BigInt(4_294_967_295);
-const TX_POLL_INTERVAL_MS = 10000;
-const TX_STATUS_CHECK_RETRY_COUNT = 100;
-
 // Mock to not wait for the timeout to finish
 jest.mock('../../src/utils/time', () => {
   return {
     Timeout: jest.fn(),
-    wait: jest.fn(() => Promise.resolve()),
+    wait: jest.fn().mockResolvedValue(true),
   };
 });
 
@@ -495,89 +494,32 @@ describe('SmartContractsClient', () => {
   describe('awaitRequiredOperationStatus', () => {
     const opId = mockOpIds[0];
     const requiredStatus = EOperationStatus.FINAL_SUCCESS;
+    let getOperationStatusMock;
 
     beforeEach(() => {
-      // Reset the getOperationStatus function
-      smartContractsClient.getOperationStatus = jest.fn();
+      getOperationStatusMock = jest.spyOn(
+        smartContractsClient,
+        'getOperationStatus',
+      );
+    });
+
+    afterEach(() => {
+      getOperationStatusMock.mockReset();
     });
 
     test('waiting for NOT_FOUND status to become the required status', async () => {
-      let callCount = 0;
-      smartContractsClient.getOperationStatus = jest
-        .fn()
-        .mockImplementation(() => {
-          callCount++;
-          if (callCount === 1) {
-            return Promise.resolve(EOperationStatus.NOT_FOUND);
-          } else {
-            return Promise.resolve(requiredStatus);
-          }
-        });
-
-      const promise = smartContractsClient.awaitRequiredOperationStatus(
+      getOperationStatusMock
+        .mockResolvedValueOnce(EOperationStatus.NOT_FOUND)
+        .mockResolvedValueOnce(requiredStatus);
+
+      const status = await smartContractsClient.awaitRequiredOperationStatus(
         opId,
         requiredStatus,
       );
 
-      const status = await promise;
-
       expect(status).toBe(requiredStatus);
       expect(smartContractsClient.getOperationStatus).toHaveBeenCalledTimes(2);
     });
-
-    test('fails after reaching the error limit', async () => {
-      console.error = jest.fn();
-
-      // Always throw an error
-      const expectedErrorMessage = 'Test error';
-      smartContractsClient.getOperationStatus = jest
-        .fn()
-        .mockRejectedValue(new Error(expectedErrorMessage));
-
-      const consoleErrorSpy = jest
-        .spyOn(console, 'error')
-        .mockImplementation(() => {});
-
-      const error = await smartContractsClient
-        .awaitRequiredOperationStatus(opId, requiredStatus)
-        .catch((e) => e);
-
-      expect(error).toBeInstanceOf(Error);
-      expect(error.message).toEqual(expectedErrorMessage);
-      expect(smartContractsClient.getOperationStatus).toHaveBeenCalledTimes(
-        101,
-      );
-
-      // Restore console.error
-      consoleErrorSpy.mockRestore();
-    });
-
-    test('fails after reaching the pending limit', async () => {
-      console.warn = jest.fn();
-      // Always return a status other than the requiredStatus
-      smartContractsClient.getOperationStatus = jest
-        .fn()
-        .mockResolvedValue(EOperationStatus.NOT_FOUND);
-
-      const consoleWarnSpy = jest
-        .spyOn(console, 'warn')
-        .mockImplementation(() => {});
-
-      await expect(
-        smartContractsClient.awaitRequiredOperationStatus(opId, requiredStatus),
-      ).rejects.toThrow(
-        `Getting the tx status for operation Id ${opId} took too long to conclude. We gave up after ${
-          TX_POLL_INTERVAL_MS * TX_STATUS_CHECK_RETRY_COUNT
-        }ms.`,
-      );
-
-      expect(smartContractsClient.getOperationStatus).toHaveBeenCalledTimes(
-        1001,
-      );
-
-      // Restore console.warn
-      consoleWarnSpy.mockRestore();
-    });
   });
 
   describe('getContractBalance', () => {