Skip to content

Commit 8f51339

Browse files
authored
fix: Do not cache rejected KV loads in Fastly EdgeFeatureStore (#1796)
1 parent 18deecd commit 8f51339

2 files changed

Lines changed: 39 additions & 1 deletion

File tree

packages/sdk/fastly/__tests__/api/EdgeFeatureStore.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,30 @@ describe('EdgeFeatureStore', () => {
113113
});
114114
});
115115

116+
describe('load caching', () => {
117+
it('does not cache a rejected load and retries on the next call', async () => {
118+
mockGet.mockRejectedValueOnce(new Error('transient KV error'));
119+
120+
// The first read hits the transient error and falls back to null.
121+
const first = await asyncFeatureStore.get({ namespace: 'features' }, 'testFlag1');
122+
expect(first).toBeNull();
123+
124+
// A later read must retry the provider (the beforeEach success impl) rather
125+
// than reuse the cached rejected promise, and should succeed.
126+
const second = await asyncFeatureStore.get({ namespace: 'features' }, 'testFlag1');
127+
expect(second).toMatchObject(testData.flags.testFlag1);
128+
expect(mockGet).toHaveBeenCalledTimes(2);
129+
});
130+
131+
it('caches a successful load and reuses it across calls', async () => {
132+
await asyncFeatureStore.get({ namespace: 'features' }, 'testFlag1');
133+
await asyncFeatureStore.get({ namespace: 'segments' }, 'testSegment1');
134+
await asyncFeatureStore.all({ namespace: 'features' });
135+
136+
expect(mockGet).toHaveBeenCalledTimes(1);
137+
});
138+
});
139+
116140
describe('init & getDescription', () => {
117141
it('can initialize', (done) => {
118142
const cb = jest.fn(() => {

packages/sdk/fastly/src/api/EdgeFeatureStore.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export class EdgeFeatureStore implements LDFeatureStore {
3232
*/
3333
private async _getKVData(): Promise<LDFeatureStoreDataStorage | null> {
3434
if (!this._deserializedPromise) {
35-
this._deserializedPromise = (async (): Promise<LDFeatureStoreDataStorage | null> => {
35+
const promise = (async (): Promise<LDFeatureStoreDataStorage | null> => {
3636
this._logger.debug('No cached data found, loading from KV store');
3737
const kvData = await this._edgeProvider.get(this._rootKey);
3838
if (!kvData) {
@@ -56,6 +56,20 @@ export class EdgeFeatureStore implements LDFeatureStore {
5656

5757
return deserializedData;
5858
})();
59+
60+
// Do not cache a rejected load. If the KV read throws (for example a
61+
// transient backend error), clear the cached promise so the next call
62+
// retries instead of reusing the rejected promise for the rest of the
63+
// request, which would make every flag evaluation fall back to defaults.
64+
// A successful load that resolves to null (missing or undeserializable
65+
// data) is still cached, since that is a stable state within a request.
66+
promise.catch(() => {
67+
if (this._deserializedPromise === promise) {
68+
this._deserializedPromise = null;
69+
}
70+
});
71+
72+
this._deserializedPromise = promise;
5973
} else {
6074
this._logger.debug('Using cached deserialized data');
6175
}

0 commit comments

Comments
 (0)