feat: increase default get timeout to 10s, add 3 retries

roncohen · roncohen · commit d475aa36263d · 2025-03-25T21:16:30.000+01:00
diff --git a/packages/node-sdk/src/client.ts b/packages/node-sdk/src/client.ts
@@ -6,14 +6,15 @@ import BatchBuffer from "./batch-buffer";
 import cache from "./cache";
 import {
   API_BASE_URL,
+  API_TIMEOUT_MS,
   BUCKET_LOG_PREFIX,
   FEATURE_EVENT_RATE_LIMITER_WINDOW_SIZE_MS,
   FEATURES_REFETCH_MS,
   loadConfig,
   SDK_VERSION,
   SDK_VERSION_HEADER_NAME,
 } from "./config";
-import fetchClient from "./fetch-http-client";
+import fetchClient, { withRetry } from "./fetch-http-client";
 import { subscribe as triggerOnExit } from "./flusher";
 import { newRateLimiter } from "./rate-limiter";
 import type {
@@ -116,6 +117,8 @@ export class BucketClient {
     rateLimiter: ReturnType<typeof newRateLimiter>;
     offline: boolean;
     configFile?: string;
+    featuresFetchRetries: number;
+    fetchTimeoutMs: number;
   };
 
   private _initialize = once(async () => {
@@ -140,7 +143,8 @@ export class BucketClient {
    * @param options.batchOptions - The options for the batch buffer (optional).
    * @param options.featureOverrides - The feature overrides to use for the client (optional).
    * @param options.configFile - The path to the config file (optional).
-
+   * @param options.featuresFetchRetries - Number of retries for fetching features (optional, defaults to 3).
+   * @param options.fetchTimeoutMs - Timeout for fetching features (optional, defaults to 10000ms).
    *
    * @throws An error if the options are invalid.
    **/
@@ -182,6 +186,20 @@ export class BucketClient {
       "configFile must be a string",
     );
 
+    ok(
+      options.featuresFetchRetries === undefined ||
+        (Number.isInteger(options.featuresFetchRetries) &&
+          options.featuresFetchRetries >= 0),
+      "featuresFetchRetries must be a non-negative integer",
+    );
+
+    ok(
+      options.fetchTimeoutMs === undefined ||
+        (Number.isInteger(options.fetchTimeoutMs) &&
+          options.fetchTimeoutMs >= 0),
+      "fetchTimeoutMs must be a non-negative integer",
+    );
+
     if (!options.configFile) {
       options.configFile =
         (process.env.BUCKET_CONFIG_FILE ??
@@ -266,6 +284,8 @@ export class BucketClient {
         typeof config.featureOverrides === "function"
           ? config.featureOverrides
           : () => config.featureOverrides,
+      featuresFetchRetries: options.featuresFetchRetries ?? 3,
+      fetchTimeoutMs: options.fetchTimeoutMs ?? API_TIMEOUT_MS,
     };
 
     if ((config.batchOptions?.flushOnExit ?? true) && !this._config.offline) {
@@ -643,35 +663,45 @@ export class BucketClient {
    * Sends a GET request to the specified path.
    *
    * @param path - The path to send the request to.
+   * @param retries - Optional number of retries for the request.
    *
    * @returns The response from the server.
    * @throws An error if the path is invalid.
    **/
-  private async get<TResponse>(path: string) {
+  private async get<TResponse>(path: string, retries: number = 3) {
     ok(typeof path === "string" && path.length > 0, "path must be a string");
 
     try {
       const url = this.buildUrl(path);
-      const response = await this._config.httpClient.get<
-        TResponse & { success: boolean }
-      >(url, this._config.headers);
-
-      this._config.logger?.debug(`get request to "${url}"`, response);
-
-      if (!response.ok || !isObject(response.body) || !response.body.success) {
-        this._config.logger?.warn(
-          `invalid response received from server for "${url}"`,
-          response,
-        );
-
-        return undefined;
-      }
-
-      const { success: _, ...result } = response.body;
-      return result as TResponse;
+      return await withRetry(
+        async () => {
+          const response = await this._config.httpClient.get<
+            TResponse & { success: boolean }
+          >(url, this._config.headers, this._config.fetchTimeoutMs);
+
+          this._config.logger?.debug(`get request to "${url}"`, response);
+
+          if (
+            !response.ok ||
+            !isObject(response.body) ||
+            !response.body.success
+          ) {
+            this._config.logger?.warn(
+              `invalid response received from server for "${url}"`,
+              response,
+            );
+            return undefined;
+          }
+          const { success: _, ...result } = response.body;
+          return result as TResponse;
+        },
+        retries,
+        1000,
+        10000,
+      );
     } catch (error) {
       this._config.logger?.error(
-        `get request to "${path}" failed with error`,
+        `get request to "${path}" failed with error after ${retries} retries`,
         error,
       );
       return undefined;
@@ -849,7 +879,10 @@ export class BucketClient {
         this._config.staleWarningInterval,
         this._config.logger,
         async () => {
-          const res = await this.get<FeaturesAPIResponse>("features");
+          const res = await this.get<FeaturesAPIResponse>(
+            "features",
+            this._config.featuresFetchRetries,
+          );
 
           if (!isObject(res) || !Array.isArray(res?.features)) {
             return undefined;
diff --git a/packages/node-sdk/src/config.ts b/packages/node-sdk/src/config.ts
@@ -8,7 +8,7 @@ import { isObject, ok } from "./utils";
 export const API_BASE_URL = "https://front.bucket.co";
 export const SDK_VERSION_HEADER_NAME = "bucket-sdk-version";
 export const SDK_VERSION = `node-sdk/${version}`;
-export const API_TIMEOUT_MS = 5000;
+export const API_TIMEOUT_MS = 10000;
 export const END_FLUSH_TIMEOUT_MS = 5000;
 
 export const BUCKET_LOG_PREFIX = "[Bucket]";
diff --git a/packages/node-sdk/src/fetch-http-client.ts b/packages/node-sdk/src/fetch-http-client.ts
@@ -13,6 +13,7 @@ const fetchClient: HttpClient = {
     url: string,
     headers: Record<string, string>,
     body: TBody,
+    timeoutMs: number = API_TIMEOUT_MS,
   ) => {
     ok(typeof url === "string" && url.length > 0, "URL must be a string");
     ok(typeof headers === "object", "Headers must be an object");
@@ -21,7 +22,7 @@ const fetchClient: HttpClient = {
       method: "post",
       headers,
       body: JSON.stringify(body),
-      signal: AbortSignal.timeout(API_TIMEOUT_MS),
+      signal: AbortSignal.timeout(timeoutMs),
     });
 
     const json = await response.json();
@@ -32,14 +33,18 @@ const fetchClient: HttpClient = {
     };
   },
 
-  get: async <TResponse>(url: string, headers: Record<string, string>) => {
+  get: async <TResponse>(
+    url: string,
+    headers: Record<string, string>,
+    timeoutMs: number = API_TIMEOUT_MS,
+  ) => {
     ok(typeof url === "string" && url.length > 0, "URL must be a string");
     ok(typeof headers === "object", "Headers must be an object");
 
     const response = await fetch(url, {
       method: "get",
       headers,
-      signal: AbortSignal.timeout(API_TIMEOUT_MS),
+      signal: AbortSignal.timeout(timeoutMs),
     });
 
     const json = await response.json();
@@ -51,4 +56,44 @@ const fetchClient: HttpClient = {
   },
 };
 
+/**
+ * Implements exponential backoff retry logic for async functions.
+ *
+ * @param fn - The async function to retry.
+ * @param maxRetries - Maximum number of retry attempts.
+ * @param baseDelay - Base delay in milliseconds before retrying.
+ * @param maxDelay - Maximum delay in milliseconds.
+ * @returns The result of the function call or throws an error if all retries fail.
+ */
+export async function withRetry<T>(
+  fn: () => Promise<T>,
+  maxRetries: number,
+  baseDelay: number,
+  maxDelay: number,
+): Promise<T> {
+  let lastError: unknown;
+
+  for (let attempt = 0; attempt <= maxRetries; attempt++) {
+    try {
+      return await fn();
+    } catch (error) {
+      lastError = error;
+
+      if (attempt === maxRetries) {
+        break;
+      }
+
+      // Calculate exponential backoff with jitter
+      const delay = Math.min(
+        maxDelay,
+        baseDelay * Math.pow(2, attempt) * (0.8 + Math.random() * 0.4),
+      );
+
+      await new Promise((resolve) => setTimeout(resolve, delay));
+    }
+  }
+
+  throw lastError;
+}
+
 export default fetchClient;
diff --git a/packages/node-sdk/src/types.ts b/packages/node-sdk/src/types.ts
@@ -386,6 +386,7 @@ export interface HttpClient {
   get<TResponse>(
     url: string,
     headers: Record<string, string>,
+    timeoutMs: number,
   ): Promise<HttpClientResponse<TResponse>>;
 }
 
@@ -531,6 +532,18 @@ export type ClientOptions = {
    **/
   httpClient?: HttpClient;
 
+  /**
+   * The timeout in milliseconds for fetching feature targeting data (optional).
+   * Default is 10000 ms.
+   **/
+  fetchTimeoutMs?: number;
+
+  /**
+   * Number of times to retry fetching feature definitions (optional).
+   * Default is 3 times.
+   **/
+  featuresFetchRetries?: number;
+
   /**
    * The options for the batch buffer (optional).
    * If not provided, the default options are used.
diff --git a/packages/node-sdk/test/client.test.ts b/packages/node-sdk/test/client.test.ts
@@ -15,6 +15,7 @@ import { evaluateFeatureRules } from "@bucketco/flag-evaluation";
 import { BoundBucketClient, BucketClient } from "../src";
 import {
   API_BASE_URL,
+  API_TIMEOUT_MS,
   BATCH_INTERVAL_MS,
   BATCH_MAX_SIZE,
   FEATURE_EVENT_RATE_LIMITER_WINDOW_SIZE_MS,
@@ -84,6 +85,7 @@ const validOptions: ClientOptions = {
   logger,
   httpClient,
   fallbackFeatures,
+  featuresFetchRetries: 2,
   batchOptions: {
     maxSize: 99,
     intervalMs: 100,
@@ -275,6 +277,7 @@ describe("BucketClient", () => {
           isEnabled: true,
         },
       });
+      expect(client["_config"].featuresFetchRetries).toBe(2);
     });
 
     it("should route messages to the supplied logger", () => {
@@ -970,6 +973,7 @@ describe("BucketClient", () => {
       expect(httpClient.get).toHaveBeenCalledWith(
         `https://api.example.com/features`,
         expectedHeaders,
+        API_TIMEOUT_MS,
       );
     });
   });
@@ -2291,6 +2295,7 @@ describe("BucketClient", () => {
       expect(httpClient.get).toHaveBeenCalledWith(
         "https://api.example.com/features/evaluated?context.other.custom=context&context.other.key=value&context.user.id=c1&context.company.id=u1",
         expectedHeaders,
+        API_TIMEOUT_MS,
       );
     });
 
@@ -2302,6 +2307,7 @@ describe("BucketClient", () => {
       expect(httpClient.get).toHaveBeenCalledWith(
         "https://api.example.com/features/evaluated?",
         expectedHeaders,
+        API_TIMEOUT_MS,
       );
     });
 
@@ -2373,6 +2379,7 @@ describe("BucketClient", () => {
       expect(httpClient.get).toHaveBeenCalledWith(
         "https://api.example.com/features/evaluated?context.other.custom=context&context.other.key=value&context.user.id=c1&context.company.id=u1&key=feature1",
         expectedHeaders,
+        API_TIMEOUT_MS,
       );
     });
 
@@ -2382,6 +2389,7 @@ describe("BucketClient", () => {
       expect(httpClient.get).toHaveBeenCalledWith(
         "https://api.example.com/features/evaluated?key=feature1",
         expectedHeaders,
+        API_TIMEOUT_MS,
       );
     });
 
@@ -2617,6 +2625,7 @@ describe("BoundBucketClient", () => {
       expect(httpClient.get).toHaveBeenCalledWith(
         "https://api.example.com/features/evaluated?context.user.id=user123&context.user.age=1&context.user.name=John&context.company.id=company123&context.company.employees=100&context.company.name=Acme+Inc.&context.other.custom=context&context.other.key=value",
         expectedHeaders,
+        API_TIMEOUT_MS,
       );
     });
 
@@ -2641,6 +2650,7 @@ describe("BoundBucketClient", () => {
       expect(httpClient.get).toHaveBeenCalledWith(
         "https://api.example.com/features/evaluated?context.user.id=user123&context.user.age=1&context.user.name=John&context.company.id=company123&context.company.employees=100&context.company.name=Acme+Inc.&context.other.custom=context&context.other.key=value&key=feature1",
         expectedHeaders,
+        API_TIMEOUT_MS,
       );
     });
   });
