fix!: Improve error handling when environment fetch fails. Minor type safety breaking changes

sdk/analytics.ts

@@ -1,6 +1,6 @@
 import { pino, Logger } from 'pino';
 import { Fetch } from "./types.js";
-import { Flags } from "./models.js";
+import { FlagsmithConfig } from "./types.js";
 
 export const ANALYTICS_ENDPOINT = './analytics/flags/';
 
@@ -21,17 +21,17 @@ export interface AnalyticsProcessorOptions {
     logger?: Logger;
     /** Custom {@link fetch} implementation to use for API requests. **/
     fetch?: Fetch
-    
+
     /** @deprecated Use {@link analyticsUrl} instead. **/
     baseApiUrl?: string;
 }
 
 /**
  * Tracks how often individual features are evaluated whenever {@link trackFeature} is called.
- * 
+ *
  * Analytics data is posted after {@link trackFeature} is called and at least {@link ANALYTICS_TIMER} seconds have
  * passed since the previous analytics API request was made (if any), or by calling {@link flush}.
- * 
+ *
  * Data will stay in memory indefinitely until it can be successfully posted to the API.
  * @see https://docs.flagsmith.com/advanced-use/flag-analytics.
  */
@@ -89,7 +89,7 @@ export class AnalyticsProcessor {
     /**
      * Track a single evaluation event for a feature.
      *
-     * This method is called whenever {@link Flags.isFeatureEnabled}, {@link Flags.getFeatureValue} or {@link Flags.getFlag} are called.
+     * @see FlagsmithConfig.enableAnalytics
      */
     trackFeature(featureName: string) {
         this.analyticsData[featureName] = (this.analyticsData[featureName] || 0) + 1;

sdk/polling_manager.ts

@@ -1,20 +1,27 @@
 import Flagsmith from './index.js';
+import { Logger } from 'pino';
 
 export class EnvironmentDataPollingManager {
     private interval?: NodeJS.Timeout;
     private main: Flagsmith;
     private refreshIntervalSeconds: number;
+    private logger: Logger;
 
-    constructor(main: Flagsmith, refreshIntervalSeconds: number) {
+    constructor(main: Flagsmith, refreshIntervalSeconds: number, logger: Logger) {
         this.main = main;
         this.refreshIntervalSeconds = refreshIntervalSeconds;
+        this.logger = logger;
     }
 
     start() {
         const updateEnvironment = () => {
             if (this.interval) clearInterval(this.interval);
             this.interval = setInterval(async () => {
-                await this.main.updateEnvironment();
+                try {
+                    await this.main.updateEnvironment();
+                } catch (error) {
+                    this.logger.error(error, 'failed to poll environment');
+                }
             }, this.refreshIntervalSeconds * 1000);
         };
         updateEnvironment();

sdk/types.ts

@@ -3,6 +3,7 @@ import { EnvironmentModel } from '../flagsmith-engine/index.js';
 import { Dispatcher } from 'undici-types';
 import { Logger } from 'pino';
 import { BaseOfflineHandler } from './offline_handlers.js';
+import { Flagsmith } from './index.js'
 
 export type IFlagsmithValue<T = string | number | boolean | null> = T;
 
@@ -27,22 +28,92 @@ export interface FlagsmithCache {
 
 export type Fetch = typeof fetch
 
+/**
+ * The configuration options for a {@link Flagsmith} client.
+ */
 export interface FlagsmithConfig {
+    /**
+     * The environment's client-side or server-side key.
+     */
     environmentKey?: string;
+    /**
+     * The Flagsmith API URL. Set this if you are not using Flagsmith's public SaaS service, i.e. https://app.flagsmith.com.
+     *
+     * @default https://edge.api.flagsmith.com/api/v1/
+     */
     apiUrl?: string;
+    /**
+     * A custom {@link Dispatcher} to use when making HTTP requests.
+     */
     agent?: Dispatcher;
+    /**
+     * A custom {@link fetch} implementation to use when making HTTP requests.
+     */
     fetch?: Fetch;
-    customHeaders?: { [key: string]: any };
+    /**
+     * Custom headers to use in all HTTP requests.
+     */
+    customHeaders?: HeadersInit
+    /**
+     * The network request timeout duration, in seconds.
+     *
+     * @default 10
+     */
     requestTimeoutSeconds?: number;
+    /**
+     * The amount of time, in milliseconds, to wait before retrying failed network requests.
+     */
+    requestRetryDelayMilliseconds?: number;
+    /**
+     * If enabled, flags are evaluated locally using the environment state cached in memory.
+     *
+     * The client will lazily fetch the environment from the Flagsmith API, and poll it every {@link environmentRefreshIntervalSeconds}.
+     */
     enableLocalEvaluation?: boolean;
+    /**
+     * The time, in seconds, to wait before refreshing the cached environment state.
+     * @default 60
+     */
     environmentRefreshIntervalSeconds?: number;
+    /**
+     * How many times to retry any failed network request before giving up.
+     * @default 3
+     */
     retries?: number;
+    /**
+     * If enabled, the client will keep track of any flags evaluated using {@link Flags.isFeatureEnabled},
+     * {@link Flags.getFeatureValue} or {@link Flags.getFlag}, and periodically flush this data to the Flagsmith API.
+     */
     enableAnalytics?: boolean;
-    defaultFlagHandler?: (featureName: string) => DefaultFlag;
+    /**
+     * Used to return fallback values for flags when evaluation fails for any reason. If not provided and flag
+     * evaluation fails, an error will be thrown intsead.
+     *
+     * @param flagKey The key of the flag that failed to evaluate.
+     *
+     * @example
+     * // All flags disabled and with no value by default
+     * const defaultHandler = () => new DefaultFlag(undefined, false)
+     *
+     * // Enable only VIP flags by default
+     * const vipDefaultHandler = (key: string) => new Default(undefined, key.startsWith('vip_'))
+     */
+    defaultFlagHandler?: (flagKey: string) => DefaultFlag;
     cache?: FlagsmithCache;
-    onEnvironmentChange?: (error: Error | null, result: EnvironmentModel) => void;
+    /**
+     * A callback function to invoke whenever the cached environment is updated.
+     * @param error The error that occurred when the environment state failed to update, if any.
+     * @param result The updated environment state, if no error was thrown.
+     */
+    onEnvironmentChange?: (error: Error | null, result?: EnvironmentModel) => void;
     logger?: Logger;
+    /**
+     * If enabled, the client will work offline and not make any network requests. Requires {@link offlineHandler}.
+     */
     offlineMode?: boolean;
+    /**
+     * If {@link offlineMode} is enabled, this handler is used to calculate the values of all flags.
+     */
     offlineHandler?: BaseOfflineHandler;
 }
 

sdk/utils.ts

@@ -46,25 +46,59 @@ export const retryFetch = (
     fetchOptions: RequestInit & { dispatcher?: Dispatcher },
     retries: number = 3,
     timeoutMs: number = 10, // set an overall timeout for this function
+    retryDelayMs: number = 1000,
     customFetch: Fetch,
 ): Promise<Response> => {
-    return new Promise((resolve, reject) => {
-        const retryWrapper = (n: number) => {
-            customFetch(url, {
+    const retryWrapper = async (n: number): Promise<Response> => {
+        try {
+            return await customFetch(url, {
                 ...fetchOptions,
                 signal: AbortSignal.timeout(timeoutMs)
-            })
-            .then(res => resolve(res))
-            .catch(async err => {
-                if (n > 0) {
-                    await delay(1000);
-                    retryWrapper(--n);
-                } else {
-                    reject(err);
-                }
             });
-        };
-
-        retryWrapper(retries);
-    });
+        } catch (e) {
+            if (n > 0) {
+                await delay(retryDelayMs);
+                return await retryWrapper(n - 1);
+            } else {
+                throw e;
+            }
+        }
+    };
+    return retryWrapper(retries);
 };
+
+/**
+ * A deferred promise can be resolved or rejected outside its creation scope.
+ *
+ * @template T The type of the value that the deferred promise will resolve to.
+ *
+ * @example
+ * const deferred = new Deferred<string>()
+ *
+ * // Pass the promise somewhere
+ * performAsyncOperation(deferred.promise)
+ *
+ * // Resolve it when ready from anywhere
+ * deferred.resolve("Operation completed")
+ * deferred.failed("Error")
+ */
+export class Deferred<T> {
+    public readonly promise: Promise<T>;
+    private resolvePromise!: (value: T | PromiseLike<T>) => void;
+    private rejectPromise!: (reason?: unknown) => void;
+
+    constructor(initial?: T) {
+        this.promise = new Promise<T>((resolve, reject) => {
+            this.resolvePromise = resolve;
+            this.rejectPromise = reject;
+        });
+    }
+
+    public resolve(value: T | PromiseLike<T>): void {
+        this.resolvePromise(value);
+    }
+
+    public reject(reason?: unknown): void {
+        this.rejectPromise(reason);
+    }
+}

tests/sdk/flagsmith-cache.test.ts

@@ -61,14 +61,16 @@ test('test_get_environment_flags_uses_local_environment_when_available', async (
   const cache = new TestCache();
   const set = vi.spyOn(cache, 'set');
 
-  const flg = flagsmith({ cache });
+  const flg = flagsmith({ cache, environmentKey: 'ser.key', enableLocalEvaluation: true });
   const model = environmentModel(JSON.parse(environmentJSON));
-  flg.environment = model;
+  const getEnvironment = vi.spyOn(flg, 'getEnvironment')
+  getEnvironment.mockResolvedValue(model)
 
-  const allFlags = await (await flg.getEnvironmentFlags()).allFlags();
+  const allFlags = (await flg.getEnvironmentFlags()).allFlags();
 
   expect(set).toBeCalled();
   expect(fetch).toBeCalledTimes(0);
+  expect(getEnvironment).toBeCalledTimes(1);
   expect(allFlags[0].enabled).toBe(model.featureStates[0].enabled);
   expect(allFlags[0].value).toBe(model.featureStates[0].getValue());
   expect(allFlags[0].featureName).toBe(model.featureStates[0].feature.name);

tests/sdk/flagsmith-environment-flags.test.ts

@@ -20,20 +20,6 @@ test('test_get_environment_flags_calls_api_when_no_local_environment', async ()
   expect(allFlags[0].featureName).toBe('some_feature');
 });
 
-test('test_get_environment_flags_uses_local_environment_when_available', async () => {
-  fetch.mockResolvedValue(new Response(flagsJSON));
-
-  const flg = flagsmith();
-  const model = environmentModel(JSON.parse(environmentJSON));
-  flg.environment = model;
-
-  const allFlags = await (await flg.getEnvironmentFlags()).allFlags();
-  expect(fetch).toBeCalledTimes(0);
-  expect(allFlags[0].enabled).toBe(model.featureStates[0].enabled);
-  expect(allFlags[0].value).toBe(model.featureStates[0].getValue());
-  expect(allFlags[0].featureName).toBe(model.featureStates[0].feature.name);
-});
-
 test('test_default_flag_is_used_when_no_environment_flags_returned', async () => {
   fetch.mockResolvedValue(new Response(JSON.stringify([])));
 

tests/sdk/flagsmith-identity-flags.test.ts

@@ -1,5 +1,13 @@
 import Flagsmith from '../../sdk/index.js';
-import { fetch, environmentJSON, flagsmith, identitiesJSON, identityWithTransientTraitsJSON, transientIdentityJSON } from './utils.js';
+import {
+  fetch,
+  environmentJSON,
+  flagsmith,
+  identitiesJSON,
+  identityWithTransientTraitsJSON,
+  transientIdentityJSON,
+  badFetch
+} from './utils.js';
 import { DefaultFlag } from '../../sdk/models.js';
 
 vi.mock('../../sdk/polling_manager');
@@ -125,7 +133,6 @@ test('test_default_flag_is_used_when_no_identity_flags_returned_and_no_custom_de
   expect(flag.enabled).toBe(false);
 });
 
-
 test('test_get_identity_flags_multivariate_value_with_local_evaluation_enabled', async () => {
   fetch.mockResolvedValue(new Response(environmentJSON));
   const identifier = 'identifier';
@@ -170,10 +177,10 @@ test('test_transient_identity', async () => {
 test('test_identity_with_transient_traits', async () => {
   fetch.mockResolvedValue(new Response(identityWithTransientTraitsJSON));
   const identifier = 'transient_trait_identifier';
-  const traits = { 
+  const traits = {
     some_trait: 'some_value',
     another_trait: {value: 'another_value', transient: true},
-    explicitly_non_transient_trait: {value: 'non_transient_value', transient: false} 
+    explicitly_non_transient_trait: {value: 'non_transient_value', transient: false}
   }
   const traitsInRequest = [
     {
@@ -206,3 +213,12 @@ test('test_identity_with_transient_traits', async () => {
   expect(identityFlags[0].value).toBe('some-identity-with-transient-trait-value');
   expect(identityFlags[0].featureName).toBe('some_feature');
 });
+
+test('getIdentityFlags fails if API call failed and no default flag handler was provided', async () => {
+  const flg = flagsmith({
+    fetch: badFetch,
+  })
+  await expect(flg.getIdentityFlags('user'))
+      .rejects
+      .toThrow('getIdentityFlags failed and no default flag handler was provided')
+})