From 3f3c004a9e468526cf05ff8bd0e02b9c27423569 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Tue, 13 Aug 2024 19:38:30 +0200 Subject: [PATCH 1/8] simplify fetching logic --- packages/browser-sdk/src/client.ts | 9 +- .../browser-sdk/src/feature/featureCache.ts | 20 +--- packages/browser-sdk/src/feature/features.ts | 98 ++++++++----------- 3 files changed, 48 insertions(+), 79 deletions(-) diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index 58bb8ed5..3eefdf66 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -135,11 +135,14 @@ export class BucketClient { * Must be called before calling other SDK methods. */ async initialize() { - const inits = [this.featuresClient.initialize()]; if (this.liveSatisfaction) { - inits.push(this.liveSatisfaction.initialize()); + // do not block on live satisfaction initialization + this.liveSatisfaction.initialize().catch((e) => { + this.logger.error("error initializing live satisfaction", e); + }); } - await Promise.all(inits); + + await this.featuresClient.initialize(); this.logger.debug( `initialized with key "${this.publishableKey}" and options`, diff --git a/packages/browser-sdk/src/feature/featureCache.ts b/packages/browser-sdk/src/feature/featureCache.ts index dd6b7d18..5219dbc6 100644 --- a/packages/browser-sdk/src/feature/featureCache.ts +++ b/packages/browser-sdk/src/feature/featureCache.ts @@ -8,9 +8,7 @@ interface StorageItem { interface cacheEntry { expireAt: number; staleAt: number; - success: boolean; // we also want to cache failures to avoid the UI waiting and spamming the API - features: APIFeaturesResponse | undefined; - attemptCount: number; + features: APIFeaturesResponse; } // Parse and validate an API feature response @@ -41,10 +39,8 @@ export function parseAPIFeaturesResponse( } export interface CacheResult { - features: APIFeaturesResponse | undefined; + features: APIFeaturesResponse; stale: boolean; - success: boolean; - attemptCount: number; } export class FeatureCache { @@ -69,13 +65,9 @@ export class FeatureCache { set( key: string, { - success, features, - attemptCount, }: { - success: boolean; - features?: APIFeaturesResponse; - attemptCount: number; + features: APIFeaturesResponse; }, ) { let cacheData: CacheData = {}; @@ -93,8 +85,6 @@ export class FeatureCache { expireAt: Date.now() + this.expireTimeMs, staleAt: Date.now() + this.staleTimeMs, features, - success, - attemptCount, } satisfies cacheEntry; cacheData = Object.fromEntries( @@ -118,9 +108,7 @@ export class FeatureCache { ) { return { features: cachedResponse[key].features, - success: cachedResponse[key].success, stale: cachedResponse[key].staleAt < Date.now(), - attemptCount: cachedResponse[key].attemptCount, }; } } @@ -155,9 +143,7 @@ function validateCacheData(cacheDataInput: any) { cacheData[key] = { expireAt: cacheEntry.expireAt, staleAt: cacheEntry.staleAt, - success: cacheEntry.success, features: cacheEntry.features, - attemptCount: cacheEntry.attemptCount, }; } return cacheData; diff --git a/packages/browser-sdk/src/feature/features.ts b/packages/browser-sdk/src/feature/features.ts index 6cf4116d..aa4db293 100644 --- a/packages/browser-sdk/src/feature/features.ts +++ b/packages/browser-sdk/src/feature/features.ts @@ -25,21 +25,18 @@ export type FeaturesOptions = { fallbackFeatures?: string[]; timeoutMs?: number; staleWhileRevalidate?: boolean; - failureRetryAttempts?: number | false; }; type Config = { fallbackFeatures: string[]; timeoutMs: number; staleWhileRevalidate: boolean; - failureRetryAttempts: number | false; }; export const DEFAULT_FEATURES_CONFIG: Config = { fallbackFeatures: [], timeoutMs: 5000, staleWhileRevalidate: false, - failureRetryAttempts: false, }; // Deep merge two objects. @@ -177,40 +174,54 @@ export class FeaturesClient { } private async maybeFetchFeatures(): Promise { - const cachedItem = this.cache.get(this.fetchParams().toString()); - - // if there's no cached item OR the cached item is a failure and we haven't retried - // too many times yet - fetch now - if ( - !cachedItem || - (!cachedItem.success && - (this.config.failureRetryAttempts === false || - cachedItem.attemptCount < this.config.failureRetryAttempts)) - ) { - return await this.fetchFeatures(); - } + const cacheKey = this.fetchParams().toString(); + const cachedItem = this.cache.get(cacheKey); + + if (cachedItem) { + if (!cachedItem.stale) return cachedItem.features; - // cachedItem is a success or a failed attempt that we've retried too many times - if (cachedItem.stale) { // serve successful stale cache if `staleWhileRevalidate` is enabled - if (this.config.staleWhileRevalidate && cachedItem.success) { - // re-fetch in the background, return last successful value - this.fetchFeatures().catch(() => { - // we don't care about the result, we just want to re-fetch - }); + if (this.config.staleWhileRevalidate) { + // re-fetch in the background, but immediately return last successful value + this.fetchFeatures() + .then((features) => { + if (!features) return; + + this.cache.set(cacheKey, { + features, + }); + }) + .catch(() => { + // we don't care about the result, we just want to re-fetch + }); return cachedItem.features; } + } + + // if there's no cached item or there is a stale one but `staleWhileRevalidate` is disabled + // try fetching a new one + const fetchedFlags = await this.fetchFeatures(); - return await this.fetchFeatures(); + if (fetchedFlags) { + this.cache.set(cacheKey, { + features: fetchedFlags, + }); + + return fetchedFlags; } - // serve cached items if not stale and not expired - return cachedItem.features; + // fetch failed, nothing cached => return fallbacks + return this.config.fallbackFeatures.reduce((acc, key) => { + acc[key] = { + key, + isEnabled: true, + }; + return acc; + }, {} as APIFeaturesResponse); } - public async fetchFeatures(): Promise { + public async fetchFeatures(): Promise { const params = this.fetchParams(); - const cacheKey = params.toString(); try { const res = await this.httpClient.get({ path: "/features/enabled", @@ -238,41 +249,10 @@ export class FeaturesClient { throw new Error("unable to validate response"); } - this.cache.set(cacheKey, { - success: true, - features: typeRes.features, - attemptCount: 0, - }); - return typeRes.features; } catch (e) { this.logger.error("error fetching features: ", e); - - const current = this.cache.get(cacheKey); - if (current) { - // if there is a previous failure cached, increase the attempt count - this.cache.set(cacheKey, { - success: current.success, - features: current.features, - attemptCount: current.attemptCount + 1, - }); - } else { - // otherwise cache if the request failed and there is no previous version to extend - // to avoid having the UI wait and spam the API - this.cache.set(cacheKey, { - success: false, - features: undefined, - attemptCount: 1, - }); - } - - return this.config.fallbackFeatures.reduce((acc, key) => { - acc[key] = { - key, - isEnabled: true, - }; - return acc; - }, {} as APIFeaturesResponse); + return; } } From c5c18f247f3621a4cb3ec3e779faa1844b560539 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Tue, 13 Aug 2024 19:39:56 +0200 Subject: [PATCH 2/8] cleanups --- .../browser-sdk/src/feature/featureCache.ts | 2 -- packages/browser-sdk/src/feature/features.ts | 2 +- .../browser-sdk/test/featureCache.test.ts | 22 ++----------------- packages/browser-sdk/test/features.test.ts | 22 ------------------- 4 files changed, 3 insertions(+), 45 deletions(-) diff --git a/packages/browser-sdk/src/feature/featureCache.ts b/packages/browser-sdk/src/feature/featureCache.ts index 5219dbc6..75d2e816 100644 --- a/packages/browser-sdk/src/feature/featureCache.ts +++ b/packages/browser-sdk/src/feature/featureCache.ts @@ -133,8 +133,6 @@ function validateCacheData(cacheDataInput: any) { if ( typeof cacheEntry.expireAt !== "number" || typeof cacheEntry.staleAt !== "number" || - typeof cacheEntry.success !== "boolean" || - typeof cacheEntry.attemptCount !== "number" || (cacheEntry.features && !parseAPIFeaturesResponse(cacheEntry.features)) ) { return; diff --git a/packages/browser-sdk/src/feature/features.ts b/packages/browser-sdk/src/feature/features.ts index aa4db293..aba53c89 100644 --- a/packages/browser-sdk/src/feature/features.ts +++ b/packages/browser-sdk/src/feature/features.ts @@ -105,7 +105,7 @@ export function clearFeatureCache() { } export const FEATURES_STALE_MS = 60000; // turn stale after 60 seconds, optionally reevaluate in the background -export const FEATURES_EXPIRE_MS = 7 * 24 * 60 * 60 * 1000; // expire entirely after 7 days +export const FEATURES_EXPIRE_MS = 30 * 24 * 60 * 60 * 1000; // expire entirely after 30 days const localStorageCacheKey = `__bucket_features`; diff --git a/packages/browser-sdk/test/featureCache.test.ts b/packages/browser-sdk/test/featureCache.test.ts index 1c7da1b5..b1fe3b54 100644 --- a/packages/browser-sdk/test/featureCache.test.ts +++ b/packages/browser-sdk/test/featureCache.test.ts @@ -48,31 +48,17 @@ describe("cache", () => { test("caches items", async () => { const { cache } = newCache(); - cache.set("key", { success: true, features, attemptCount: 1 }); + cache.set("key", { features }); expect(cache.get("key")).toEqual({ stale: false, - success: true, features, - attemptCount: 1, - } satisfies CacheResult); - }); - - test("caches unsuccessful items", async () => { - const { cache } = newCache(); - - cache.set("key", { success: false, features, attemptCount: 1 }); - expect(cache.get("key")).toEqual({ - stale: false, - success: false, - features, - attemptCount: 1, } satisfies CacheResult); }); test("sets stale", async () => { const { cache } = newCache(); - cache.set("key", { success: true, features, attemptCount: 1 }); + cache.set("key", { features }); vitest.advanceTimersByTime(TEST_STALE_MS + 1); @@ -84,17 +70,13 @@ describe("cache", () => { const { cache, cacheItem } = newCache(); cache.set("first key", { - success: true, features, - attemptCount: 1, }); expect(cacheItem[0]).not.toBeNull(); vitest.advanceTimersByTime(TEST_EXPIRE_MS + 1); cache.set("other key", { - success: true, features, - attemptCount: 1, }); const item = cache.get("key"); diff --git a/packages/browser-sdk/test/features.test.ts b/packages/browser-sdk/test/features.test.ts index ae98eaa4..65b2c8d3 100644 --- a/packages/browser-sdk/test/features.test.ts +++ b/packages/browser-sdk/test/features.test.ts @@ -100,28 +100,6 @@ describe("FeaturesClient unit tests", () => { expect(staleFeatures).toEqual(featuresResult); }); - test("attempts multiple tries before caching negative response", async () => { - const { newFeaturesClient, httpClient } = featuresClientFactory(); - - vi.mocked(httpClient.get).mockRejectedValue( - new Error("Failed to fetch features"), - ); - - for (let i = 0; i < 3; i++) { - const featuresClient = newFeaturesClient({ - staleWhileRevalidate: false, - failureRetryAttempts: 3, - }); - await featuresClient.initialize(); - - expect(httpClient.get).toHaveBeenCalledTimes(i + 1); - } - - const featuresClient = newFeaturesClient({ failureRetryAttempts: 3 }); - await featuresClient.initialize(); - expect(httpClient.get).toHaveBeenCalledTimes(3); - }); - test("disable caching negative response", async () => { const { newFeaturesClient, httpClient } = featuresClientFactory(); From bd8241bf5bfbaa5906453fcc0fa16f4408b6273f Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Wed, 14 Aug 2024 13:21:24 +0200 Subject: [PATCH 3/8] wip --- packages/browser-sdk/src/feature/maskedProxy.ts | 3 +++ packages/browser-sdk/test/usage.test.ts | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/browser-sdk/src/feature/maskedProxy.ts b/packages/browser-sdk/src/feature/maskedProxy.ts index 643d0233..c5005140 100644 --- a/packages/browser-sdk/src/feature/maskedProxy.ts +++ b/packages/browser-sdk/src/feature/maskedProxy.ts @@ -4,6 +4,9 @@ export default function maskedProxy( ) { return new Proxy(obj, { get(target: T, prop) { + if (typeof prop === "symbol") { + return target[prop as K]; + } return valueFunc(target, prop as K); }, set(_target, prop, _value) { diff --git a/packages/browser-sdk/test/usage.test.ts b/packages/browser-sdk/test/usage.test.ts index e274519e..5272bbb4 100644 --- a/packages/browser-sdk/test/usage.test.ts +++ b/packages/browser-sdk/test/usage.test.ts @@ -217,14 +217,19 @@ describe("feedback state management", () => { test("ignores prompt if already seen", async () => { vi.mocked(checkPromptMessageCompleted).mockReturnValue(true); + expect(checkPromptMessageCompleted).not.toHaveBeenCalled(); const callback = vi.fn(); await createBucketInstance(callback); expect(callback).not.toBeCalled; + await vi.waitFor(() => + expect(checkPromptMessageCompleted).toHaveBeenCalled(), + ); + + expect(vi.mocked(checkPromptMessageCompleted).mock.calls).toEqual([]); - expect(checkPromptMessageCompleted).toHaveBeenCalledOnce(); expect(checkPromptMessageCompleted).toHaveBeenCalledWith("foo", "123"); }); From 2f679537ed36a023f51e39b13e222d6b1403e289 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Wed, 14 Aug 2024 13:31:00 +0200 Subject: [PATCH 4/8] fix tests --- packages/browser-sdk/src/client.ts | 12 ++++++++---- packages/browser-sdk/src/feedback/feedback.ts | 1 + packages/browser-sdk/test/usage.test.ts | 14 ++++++++------ 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index 3eefdf66..35724a6d 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -73,6 +73,7 @@ export class BucketClient { private httpClient: HttpClient; private liveSatisfaction: LiveSatisfaction | undefined; + private liveSatisfactionInit: Promise | undefined; private featuresClient: FeaturesClient; constructor( @@ -137,9 +138,11 @@ export class BucketClient { async initialize() { if (this.liveSatisfaction) { // do not block on live satisfaction initialization - this.liveSatisfaction.initialize().catch((e) => { - this.logger.error("error initializing live satisfaction", e); - }); + this.liveSatisfactionInit = this.liveSatisfaction + .initialize() + .catch((e) => { + this.logger.error("error initializing live satisfaction", e); + }); } await this.featuresClient.initialize(); @@ -315,9 +318,10 @@ export class BucketClient { return this.featuresClient.getFeatures(); } - stop() { + async stop() { if (this.liveSatisfaction) { this.liveSatisfaction.stop(); + await this.liveSatisfactionInit; } } } diff --git a/packages/browser-sdk/src/feedback/feedback.ts b/packages/browser-sdk/src/feedback/feedback.ts index 4753ee8e..7663b25b 100644 --- a/packages/browser-sdk/src/feedback/feedback.ts +++ b/packages/browser-sdk/src/feedback/feedback.ts @@ -229,6 +229,7 @@ export class LiveSatisfaction { this.logger.error("feedback prompting already initialized"); return; } + this.initialized = true; const channel = await this.getChannel(); if (!channel) return; diff --git a/packages/browser-sdk/test/usage.test.ts b/packages/browser-sdk/test/usage.test.ts index 5272bbb4..9d31948b 100644 --- a/packages/browser-sdk/test/usage.test.ts +++ b/packages/browser-sdk/test/usage.test.ts @@ -166,10 +166,12 @@ describe("feedback state management", () => { }; let events: string[] = []; + let bucketInstance: BucketClient | null = null; beforeEach(() => { vi.mocked(openAblySSEChannel).mockImplementation(({ callback }) => { callback(message); - return {} as AblySSEChannel; + // eslint-disable-next-line @typescript-eslint/no-empty-function + return { close: () => {} } as AblySSEChannel; }); events = []; server.use( @@ -181,12 +183,14 @@ describe("feedback state management", () => { ); }); - afterEach(() => { + afterEach(async () => { + if (bucketInstance) await bucketInstance.stop(); + vi.resetAllMocks(); }); const createBucketInstance = async (callback: FeedbackPromptHandler) => { - const bucketInstance = new BucketClient( + bucketInstance = new BucketClient( KEY, { user: { id: "foo" } }, { @@ -225,11 +229,9 @@ describe("feedback state management", () => { expect(callback).not.toBeCalled; await vi.waitFor(() => - expect(checkPromptMessageCompleted).toHaveBeenCalled(), + expect(checkPromptMessageCompleted).toHaveBeenCalledOnce(), ); - expect(vi.mocked(checkPromptMessageCompleted).mock.calls).toEqual([]); - expect(checkPromptMessageCompleted).toHaveBeenCalledWith("foo", "123"); }); From ba65f23483de41cef7715d386aef1357c3095b9d Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Wed, 14 Aug 2024 14:07:36 +0200 Subject: [PATCH 5/8] fix cache + stale while revalidate --- packages/browser-sdk/src/feature/features.ts | 10 ++ packages/browser-sdk/test/features.test.ts | 177 +++++++------------ packages/react-sdk/src/index.tsx | 2 +- 3 files changed, 70 insertions(+), 119 deletions(-) diff --git a/packages/browser-sdk/src/feature/features.ts b/packages/browser-sdk/src/feature/features.ts index aba53c89..95417f55 100644 --- a/packages/browser-sdk/src/feature/features.ts +++ b/packages/browser-sdk/src/feature/features.ts @@ -144,6 +144,10 @@ export class FeaturesClient { async initialize() { const features = (await this.maybeFetchFeatures()) || {}; + this.setFeatures(features); + } + + private setFeatures(features: APIFeaturesResponse) { const proxiedFeatures = maskedProxy(features, (fs, key) => { this.sendCheckEvent({ key, @@ -190,6 +194,7 @@ export class FeaturesClient { this.cache.set(cacheKey, { features, }); + this.setFeatures(features); }) .catch(() => { // we don't care about the result, we just want to re-fetch @@ -210,6 +215,11 @@ export class FeaturesClient { return fetchedFlags; } + if (cachedItem) { + // fetch failed, return stale cache + return cachedItem.features; + } + // fetch failed, nothing cached => return fallbacks return this.config.fallbackFeatures.reduce((acc, key) => { acc[key] = { diff --git a/packages/browser-sdk/test/features.test.ts b/packages/browser-sdk/test/features.test.ts index 65b2c8d3..ebfe680c 100644 --- a/packages/browser-sdk/test/features.test.ts +++ b/packages/browser-sdk/test/features.test.ts @@ -1,5 +1,6 @@ import { afterAll, beforeEach, describe, expect, test, vi } from "vitest"; +import { APIFeatureResponse } from "../dist/src/feature/features"; import { FEATURES_EXPIRE_MS, FeaturesClient, @@ -81,148 +82,88 @@ describe("FeaturesClient unit tests", () => { expect(httpClient.get).toBeCalledTimes(1); }); - test("maintains previously successful features on negative response", async () => { + test("use cache when unable to fetch features", async () => { const { newFeaturesClient, httpClient } = featuresClientFactory(); - const featuresClient = newFeaturesClient(); - await featuresClient.initialize(); + const featuresClient = newFeaturesClient({ staleWhileRevalidate: false }); + await featuresClient.initialize(); // cache them initially vi.mocked(httpClient.get).mockRejectedValue( new Error("Failed to fetch features"), ); - // expect(httpClientGetSpy).toBeCalledTimes(0); + expect(httpClient.get).toBeCalledTimes(1); - // vi.mocked(fetch).mockRejectedValue(new Error("Failed to fetch features")); - vi.advanceTimersByTime(60000); + vi.advanceTimersByTime(TEST_STALE_MS + 1); + // fail this time await featuresClient.fetchFeatures(); + expect(httpClient.get).toBeCalledTimes(2); const staleFeatures = featuresClient.getFeatures(); expect(staleFeatures).toEqual(featuresResult); }); - test("disable caching negative response", async () => { - const { newFeaturesClient, httpClient } = featuresClientFactory(); - - vi.mocked(httpClient.get).mockRejectedValue( - new Error("Failed to fetch features"), - ); + test("stale-while-revalidate should cache but start new fetch", async () => { + const response = { + success: true, + features: { + featureB: { + isEnabled: true, + key: "featureB", + targetingVersion: 1, + } satisfies APIFeatureResponse, + }, + }; - for (let i = 0; i < 5; i++) { - const featuresClient = newFeaturesClient({ failureRetryAttempts: false }); - await featuresClient.initialize(); + const { newFeaturesClient, httpClient } = featuresClientFactory(); - expect(httpClient.get).toHaveBeenCalledTimes(i + 1); - } - }); + vi.mocked(httpClient.get).mockResolvedValue({ + status: 200, + ok: true, + json: function () { + return Promise.resolve(response); + }, + } as Response); - describe("stale cache while reevaluating", async () => { - test("when stale cache is success response", async () => { - const response = { - success: true, - features: { - featureB: { isEnabled: true, key: "featureB" }, - }, - }; + const client = newFeaturesClient({ + staleWhileRevalidate: true, + }); + expect(httpClient.get).toHaveBeenCalledTimes(0); - const { newFeaturesClient, httpClient } = featuresClientFactory(); + await client.initialize(); + expect(client.getFeatures()).toEqual({ featureB: true }); - vi.mocked(httpClient.get).mockResolvedValue({ - status: 200, - ok: true, - json: function () { - return Promise.resolve(response); - }, - } as Response); - - const client = newFeaturesClient({ - failureRetryAttempts: false, - staleWhileRevalidate: true, - }); - expect(httpClient.get).toHaveBeenCalledTimes(0); - - await client.initialize(); - - expect(httpClient.get).toHaveBeenCalledTimes(1); - const client2 = newFeaturesClient({ - failureRetryAttempts: false, - staleWhileRevalidate: true, - }); - - // change the response so we can validate that we'll serve the stale cache - vi.mocked(httpClient.get).mockResolvedValue({ - status: 200, - ok: true, - json: () => - Promise.resolve({ - success: true, - features: { - featureA: { isEnabled: true, key: "featureA" }, - }, - }), - } as Response); - - vi.advanceTimersByTime(TEST_STALE_MS + 1); - - await client2.initialize(); - expect(client.getFeatures()).toEqual(client2.getFeatures()); - - // new fetch was fired in the background - expect(httpClient.get).toHaveBeenCalledTimes(2); + expect(httpClient.get).toHaveBeenCalledTimes(1); + const client2 = newFeaturesClient({ + staleWhileRevalidate: true, }); - test("when stale cache is failed response", async () => { - const { newFeaturesClient, httpClient } = featuresClientFactory(); - // when cached response is failure, we should not serve the stale cache - const response = { - success: false, - }; - - expect(httpClient.get).toHaveBeenCalledTimes(0); + // change the response so we can validate that we'll serve the stale cache + vi.mocked(httpClient.get).mockResolvedValue({ + status: 200, + ok: true, + json: () => + Promise.resolve({ + success: true, + features: { + featureA: { + isEnabled: true, + key: "featureA", + targetingVersion: 1, + } satisfies APIFeatureResponse, + }, + }), + } as Response); - vi.mocked(httpClient.get).mockResolvedValue({ - status: 200, - ok: true, - json: function () { - return Promise.resolve(response); - }, - } as Response); - - const client = newFeaturesClient({ - staleWhileRevalidate: true, - failureRetryAttempts: 0, - }); - await client.initialize(); - - expect(httpClient.get).toHaveBeenCalledTimes(1); - - // change the response so we can validate that we'll not serve the stale cache - vi.mocked(httpClient.get).mockResolvedValue({ - status: 200, - ok: true, - json: function () { - return Promise.resolve({ - success: true, - features: { - featureB: { - isEnabled: true, - key: "featureB", - targetingVersion: 1, - }, - }, - }); - }, - } as Response); + vi.advanceTimersByTime(TEST_STALE_MS + 1); - vi.advanceTimersByTime(TEST_STALE_MS + 1); - const client2 = newFeaturesClient(); - await client2.initialize(); + await client2.initialize(); - expect(client2.getFeatures()).toEqual({ featureB: true }); + // new fetch was fired in the background + expect(httpClient.get).toHaveBeenCalledTimes(2); - // new fetch was fired - // stale while validate in the background - expect(httpClient.get).toHaveBeenCalledTimes(2); - }); + await vi.waitFor(() => + expect(client2.getFeatures()).toEqual({ featureA: true }), + ); }); test("expires cache eventually", async () => { diff --git a/packages/react-sdk/src/index.tsx b/packages/react-sdk/src/index.tsx index 571d93ae..e3f08a63 100644 --- a/packages/react-sdk/src/index.tsx +++ b/packages/react-sdk/src/index.tsx @@ -148,7 +148,7 @@ export function BucketProvider({ }); // on umount - return () => client.stop(); + return () => void client.stop(); }, [contextKey]); const track = useCallback( From 514b19f514c1cf690206ec8a2555c12431f54518 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Wed, 14 Aug 2024 14:13:28 +0200 Subject: [PATCH 6/8] ensure fully initialized before stopping --- packages/browser-sdk/src/client.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index 35724a6d..583b6de0 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -320,8 +320,9 @@ export class BucketClient { async stop() { if (this.liveSatisfaction) { - this.liveSatisfaction.stop(); + // ensure fully initialized before stopping await this.liveSatisfactionInit; + this.liveSatisfaction.stop(); } } } From bbe002c5d78cd51b0135cf13e85fa369f9fdf076 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Wed, 14 Aug 2024 14:16:21 +0200 Subject: [PATCH 7/8] linting fixes --- packages/browser-sdk/test/usage.test.ts | 4 ++-- packages/react-sdk/src/index.tsx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/browser-sdk/test/usage.test.ts b/packages/browser-sdk/test/usage.test.ts index 9d31948b..9492199d 100644 --- a/packages/browser-sdk/test/usage.test.ts +++ b/packages/browser-sdk/test/usage.test.ts @@ -94,8 +94,8 @@ describe("feedback prompting", () => { expect(openAblySSEChannel).toBeCalledTimes(1); // call twice, expect only one reset to go through - bucketInstance.stop(); - bucketInstance.stop(); + await bucketInstance.stop(); + await bucketInstance.stop(); expect(closeChannel).toBeCalledTimes(1); }); diff --git a/packages/react-sdk/src/index.tsx b/packages/react-sdk/src/index.tsx index e3f08a63..11e1c074 100644 --- a/packages/react-sdk/src/index.tsx +++ b/packages/react-sdk/src/index.tsx @@ -105,7 +105,7 @@ export function BucketProvider({ // on update of contextKey and on mount if (clientRef.current) { - clientRef.current.stop(); + void clientRef.current.stop(); } const client = newBucketClient(publishableKey, featureContext, { From efb359de66a15222a7fc3a5ed437030094b29fde Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Fri, 16 Aug 2024 14:25:37 +0200 Subject: [PATCH 8/8] update nomenclature --- packages/browser-sdk/src/feature/features.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/browser-sdk/src/feature/features.ts b/packages/browser-sdk/src/feature/features.ts index 95417f55..869a5261 100644 --- a/packages/browser-sdk/src/feature/features.ts +++ b/packages/browser-sdk/src/feature/features.ts @@ -205,14 +205,14 @@ export class FeaturesClient { // if there's no cached item or there is a stale one but `staleWhileRevalidate` is disabled // try fetching a new one - const fetchedFlags = await this.fetchFeatures(); + const fetchedFeatures = await this.fetchFeatures(); - if (fetchedFlags) { + if (fetchedFeatures) { this.cache.set(cacheKey, { - features: fetchedFlags, + features: fetchedFeatures, }); - return fetchedFlags; + return fetchedFeatures; } if (cachedItem) {