Refactor subgraph options to enforce complete retry configuration and update related tests

Author: flopez7

packages/sdk/python/human-protocol-sdk/human_protocol_sdk/escrow/escrow_utils.py

@@ -24,7 +24,6 @@
 ------
 """
 
-from datetime import datetime
 import logging
 from typing import List, Optional
 

packages/sdk/python/human-protocol-sdk/human_protocol_sdk/utils.py

@@ -3,7 +3,7 @@
 import os
 import time
 import re
-from typing import Tuple, Optional, Dict, Any
+from typing import Tuple, Optional
 from dataclasses import dataclass
 
 import requests
@@ -37,8 +37,8 @@
 class SubgraphOptions:
     """Configuration for subgraph logic."""
 
-    max_retries: int = 3
-    base_delay: int = 1000  # milliseconds
+    max_retries: Optional[int] = None
+    base_delay: Optional[int] = None  # milliseconds
 
 
 def is_indexer_error(error: Exception) -> bool:
@@ -94,7 +94,17 @@ def custom_gql_fetch(
     if not options:
         return _fetch_subgraph_data(network, query, params)
 
-    max_retries = options.max_retries
+    if (
+        options.max_retries is not None
+        and options.base_delay is None
+        or options.max_retries is None
+        and options.base_delay is not None
+    ):
+        raise ValueError(
+            "Retry configuration must include both max_retries and base_delay"
+        )
+
+    max_retries = int(options.max_retries)
     base_delay = options.base_delay / 1000
 
     last_error = None

packages/sdk/python/human-protocol-sdk/test/human_protocol_sdk/test_utils.py

@@ -81,6 +81,18 @@ def test_retries_on_indexer_error_and_succeeds(self):
         self.assertEqual(mock_fetch.call_count, 2)
         mock_sleep.assert_called_once()
 
+    def test_raises_when_retry_options_incomplete(self):
+        options = SubgraphOptions(max_retries=2)
+
+        with patch("human_protocol_sdk.utils._fetch_subgraph_data") as mock_fetch:
+            with self.assertRaises(ValueError) as ctx:
+                custom_gql_fetch(
+                    self.network, self.query, self.variables, options=options
+                )
+
+        self.assertIn("max_retries", str(ctx.exception))
+        mock_fetch.assert_not_called()
+
     def test_raises_immediately_on_non_indexer_error(self):
         options = SubgraphOptions(max_retries=3, base_delay=50)
         with patch(

packages/sdk/typescript/human-protocol-sdk/src/error.ts

@@ -306,6 +306,13 @@ export const ErrorBulkPayOutVersion = new Error(
   'Invalid bulkPayOut parameters for the contract version of the specified escrow address'
 );
 
+/**
+ * @constant {Error} - Retry configuration is missing required parameters.
+ */
+export const ErrorRetryParametersMissing = new Error(
+  'Retry configuration must include both maxRetries and baseDelay'
+);
+
 /**
  * @constant {Warning} - Possible version mismatch.
  */

packages/sdk/typescript/human-protocol-sdk/src/interfaces.ts

@@ -317,8 +317,8 @@ export interface IEscrowWithdraw {
  * Configuration options for subgraph requests with retry logic.
  */
 export interface SubgraphOptions {
-  /** Maximum number of retry attempts (default: 3) */
+  /** Maximum number of retry attempts */
   maxRetries?: number;
-  /** Base delay between retries in milliseconds (default: 1000) */
+  /** Base delay between retries in milliseconds */
   baseDelay?: number;
 }

packages/sdk/typescript/human-protocol-sdk/src/utils.ts

@@ -7,6 +7,7 @@ import { SUBGRAPH_API_KEY_PLACEHOLDER } from './constants';
 import { ChainId } from './enums';
 import {
   ContractExecutionError,
+  ErrorRetryParametersMissing,
   EthereumError,
   InvalidArgumentError,
   NonceExpired,
@@ -131,27 +132,31 @@ export const customGqlFetch = async <T = any>(
     return await gqlFetch<T>(url, query, variables);
   }
 
-  const maxRetries = options.maxRetries ?? 3;
-  const baseDelay = options.baseDelay ?? 1000;
+  if (
+    (options.maxRetries && options.baseDelay === undefined) ||
+    (options.baseDelay && options.maxRetries === undefined)
+  ) {
+    throw ErrorRetryParametersMissing;
+  }
 
   let lastError: any;
 
-  for (let attempt = 0; attempt <= maxRetries; attempt++) {
+  for (let attempt = 0; attempt <= (options.maxRetries as number); attempt++) {
     try {
       const result = await gqlFetch<T>(url, query, variables);
       return result;
     } catch (error) {
       lastError = error;
 
-      if (attempt === maxRetries) {
+      if (attempt === options.maxRetries) {
         throw error;
       }
 
       if (!isIndexerError(error)) {
         throw error;
       }
 
-      const delay = baseDelay * attempt;
+      const delay = (options.baseDelay as number) * attempt;
       await sleep(delay);
     }
   }

packages/sdk/typescript/human-protocol-sdk/test/utils.test.ts

@@ -19,6 +19,7 @@ import {
   ReplacementUnderpriced,
   TransactionReplaced,
   WarnSubgraphApiKeyNotProvided,
+  ErrorRetryParametersMissing,
 } from '../src/error';
 import {
   getSubgraphUrl,
@@ -298,21 +299,24 @@ describe('customGqlFetch', () => {
     expect(gqlFetchSpy).toHaveBeenCalledTimes(3);
   });
 
-  test('uses default values for missing maxRetries', async () => {
-    const badIndexerError = {
-      response: {
-        errors: [{ message: 'bad indexers: {0x123: Timeout}' }],
-      },
-    };
-    const gqlFetchSpy = vi
-      .spyOn(gqlFetch, 'default')
-      .mockRejectedValue(badIndexerError);
+  test('throws when retry options are incomplete', async () => {
+    const gqlFetchSpy = vi.spyOn(gqlFetch, 'default');
 
     await expect(
       customGqlFetch(mockUrl, mockQuery, mockVariables, { baseDelay: 10 })
-    ).rejects.toEqual(badIndexerError);
+    ).rejects.toBe(ErrorRetryParametersMissing);
+
+    expect(gqlFetchSpy).not.toHaveBeenCalled();
+  });
+
+  test('throws when only maxRetries is provided', async () => {
+    const gqlFetchSpy = vi.spyOn(gqlFetch, 'default');
+
+    await expect(
+      customGqlFetch(mockUrl, mockQuery, mockVariables, { maxRetries: 1 })
+    ).rejects.toBe(ErrorRetryParametersMissing);
 
-    expect(gqlFetchSpy).toHaveBeenCalledTimes(4);
+    expect(gqlFetchSpy).not.toHaveBeenCalled();
   });
 
   test('uses custom maxRetries when provided', async () => {