fix: improved retry logic, improve retry logging (#408)

- removed `unref()` to ensure retries block the process from exiting
- improved logging around retries

Author: roncohen

packages/node-sdk/src/cache.ts

@@ -34,6 +34,8 @@ export default function cache<T>(
       if (newValue === undefined) {
         return;
       }
+      logger?.info("refreshed features");
+
       cachedValue = newValue;
 
       lastUpdate = Date.now();
@@ -63,7 +65,6 @@ export default function cache<T>(
       refreshPromise = update();
     }
     await refreshPromise;
-    logger?.info("refreshed cached features");
     return get();
   };
 

packages/node-sdk/src/client.ts

@@ -675,7 +675,7 @@ export class BucketClient {
       if (!response.ok || !isObject(response.body) || !response.body.success) {
         this.logger.warn(
           `invalid response received from server for "${url}"`,
-          response,
+          JSON.stringify(response),
         );
         return false;
       }
@@ -713,15 +713,16 @@ export class BucketClient {
             !isObject(response.body) ||
             !response.body.success
           ) {
-            this.logger.warn(
-              `invalid response received from server for "${url}"`,
-              response,
+            throw new Error(
+              `invalid response received from server for "${url}": ${JSON.stringify(response.body)}`,
             );
-            return undefined;
           }
           const { success: _, ...result } = response.body;
           return result as TResponse;
         },
+        () => {
+          this.logger.warn("failed to fetch features, will retry");
+        },
         retries,
         1000,
         10000,

packages/node-sdk/src/fetch-http-client.ts

@@ -67,6 +67,7 @@ const fetchClient: HttpClient = {
  */
 export async function withRetry<T>(
   fn: () => Promise<T>,
+  onFailedTry: () => void,
   maxRetries: number,
   baseDelay: number,
   maxDelay: number,
@@ -83,13 +84,15 @@ export async function withRetry<T>(
         break;
       }
 
+      onFailedTry();
+
       // 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).unref());
+      await new Promise((resolve) => setTimeout(resolve, delay));
     }
   }
 

packages/node-sdk/test/client.test.ts

@@ -88,7 +88,7 @@ const validOptions: ClientOptions = {
   featuresFetchRetries: 2,
   batchOptions: {
     maxSize: 99,
-    intervalMs: 100,
+    intervalMs: 10001,
     flushOnExit: false,
   },
   offline: false,
@@ -270,7 +270,7 @@ describe("BucketClient", () => {
       expect(client["_config"].headers).toEqual(expectedHeaders);
       expect(client["batchBuffer"]).toMatchObject({
         maxSize: 99,
-        intervalMs: 100,
+        intervalMs: 10001,
       });
 
       expect(client["_config"].fallbackFeatures).toEqual({
@@ -630,7 +630,7 @@ describe("BucketClient", () => {
 
       expect(logger.warn).toHaveBeenCalledWith(
         expect.stringMatching("invalid response received from server for"),
-        response,
+        JSON.stringify(response),
       );
     });
 
@@ -722,7 +722,7 @@ describe("BucketClient", () => {
 
       expect(logger.warn).toHaveBeenCalledWith(
         expect.stringMatching("invalid response received from server for"),
-        response,
+        JSON.stringify(response),
       );
     });
 
@@ -859,7 +859,7 @@ describe("BucketClient", () => {
 
       expect(logger.warn).toHaveBeenCalledWith(
         expect.stringMatching("invalid response received from server for "),
-        response,
+        JSON.stringify(response),
       );
     });