Skip to content

Commit ad23063

Browse files
EmilianoSanchezEmilianoSanchez
authored
Merge pull request #454 from splitio/polishing
Updated storage operations order to prevent inconsistent states when using localStorage.
2 parents 92b613b + 0176b3b commit ad23063

File tree

13 files changed

+95
-116
lines changed

13 files changed

+95
-116
lines changed

‎CHANGES.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
2.8.1 (November 25, 2025)
2+
- Updated the order of storage operations to prevent inconsistent states when using the `LOCALSTORAGE` storage type and the browser’s `localStorage` fails due to quota limits.
3+
14
2.8.0 (October 30, 2025)
25
- Added new configuration for Fallback Treatments, which allows setting a treatment value and optional config to be returned in place of "control", either globally or by flag. Read more in our docs.
36
- Added `client.getStatus()` method to retrieve the client readiness status properties (`isReady`, `isReadyFromCache`, etc).

‎src/readiness/__tests__/readinessManager.spec.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,10 @@ import { EventEmitter } from '../../utils/MinEvents';
33
import { IReadinessManager } from '../types';
44
import { SDK_READY, SDK_UPDATE, SDK_SPLITS_ARRIVED, SDK_SEGMENTS_ARRIVED, SDK_READY_FROM_CACHE, SDK_SPLITS_CACHE_LOADED, SDK_READY_TIMED_OUT } from '../constants';
55
import { ISettings } from '../../types';
6-
import { STORAGE_LOCALSTORAGE } from '../../utils/constants';
76

87
const settings = {
98
startup: {
109
readyTimeout: 0,
11-
},
12-
storage: {
13-
type: STORAGE_LOCALSTORAGE
1410
}
1511
} as unknown as ISettings;
1612

‎src/storages/AbstractMySegmentsCacheSync.ts‎

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,10 @@ export abstract class AbstractMySegmentsCacheSync implements ISegmentsCacheSync
4949
* For client-side synchronizer: it resets or updates the cache.
5050
*/
5151
resetSegments(segmentsData: MySegmentsData | IMySegmentsResponse): boolean {
52-
this.setChangeNumber(segmentsData.cn);
53-
52+
let isDiff = false;
5453
const { added, removed } = segmentsData as MySegmentsData;
5554

5655
if (added && removed) {
57-
let isDiff = false;
5856

5957
added.forEach(segment => {
6058
isDiff = this.addSegment(segment) || isDiff;
@@ -63,32 +61,40 @@ export abstract class AbstractMySegmentsCacheSync implements ISegmentsCacheSync
6361
removed.forEach(segment => {
6462
isDiff = this.removeSegment(segment) || isDiff;
6563
});
64+
} else {
6665

67-
return isDiff;
68-
}
66+
const names = ((segmentsData as IMySegmentsResponse).k || []).map(s => s.n).sort();
67+
const storedSegmentKeys = this.getRegisteredSegments().sort();
6968

70-
const names = ((segmentsData as IMySegmentsResponse).k || []).map(s => s.n).sort();
71-
const storedSegmentKeys = this.getRegisteredSegments().sort();
69+
// Extreme fast => everything is empty
70+
if (!names.length && !storedSegmentKeys.length) {
71+
isDiff = false;
72+
} else {
7273

73-
// Extreme fast => everything is empty
74-
if (!names.length && !storedSegmentKeys.length) return false;
74+
let index = 0;
7575

76-
let index = 0;
76+
while (index < names.length && index < storedSegmentKeys.length && names[index] === storedSegmentKeys[index]) index++;
7777

78-
while (index < names.length && index < storedSegmentKeys.length && names[index] === storedSegmentKeys[index]) index++;
78+
// Quick path => no changes
79+
if (index === names.length && index === storedSegmentKeys.length) {
80+
isDiff = false;
81+
} else {
7982

80-
// Quick path => no changes
81-
if (index === names.length && index === storedSegmentKeys.length) return false;
83+
// Slowest path => add and/or remove segments
84+
for (let removeIndex = index; removeIndex < storedSegmentKeys.length; removeIndex++) {
85+
this.removeSegment(storedSegmentKeys[removeIndex]);
86+
}
8287

83-
// Slowest path => add and/or remove segments
84-
for (let removeIndex = index; removeIndex < storedSegmentKeys.length; removeIndex++) {
85-
this.removeSegment(storedSegmentKeys[removeIndex]);
86-
}
88+
for (let addIndex = index; addIndex < names.length; addIndex++) {
89+
this.addSegment(names[addIndex]);
90+
}
8791

88-
for (let addIndex = index; addIndex < names.length; addIndex++) {
89-
this.addSegment(names[addIndex]);
92+
isDiff = true;
93+
}
94+
}
9095
}
9196

92-
return true;
97+
this.setChangeNumber(segmentsData.cn);
98+
return isDiff;
9399
}
94100
}

‎src/storages/AbstractSplitsCacheSync.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@ export abstract class AbstractSplitsCacheSync implements ISplitsCacheSync {
1414
protected abstract setChangeNumber(changeNumber: number): boolean | void
1515

1616
update(toAdd: ISplit[], toRemove: ISplit[], changeNumber: number): boolean {
17+
let updated = toAdd.map(addedFF => this.addSplit(addedFF)).some(result => result);
18+
updated = toRemove.map(removedFF => this.removeSplit(removedFF.name)).some(result => result) || updated;
1719
this.setChangeNumber(changeNumber);
18-
const updated = toAdd.map(addedFF => this.addSplit(addedFF)).some(result => result);
19-
return toRemove.map(removedFF => this.removeSplit(removedFF.name)).some(result => result) || updated;
20+
return updated;
2021
}
2122

2223
abstract getSplit(name: string): ISplit | null

‎src/storages/inLocalStorage/MySegmentsCacheInLocal.ts‎

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { ILogger } from '../../logger/types';
22
import { isNaNNumber } from '../../utils/lang';
33
import { AbstractMySegmentsCacheSync } from '../AbstractMySegmentsCacheSync';
44
import type { MySegmentsKeyBuilder } from '../KeyBuilderCS';
5-
import { LOG_PREFIX, DEFINED } from './constants';
5+
import { DEFINED } from './constants';
66
import { StorageAdapter } from '../types';
77

88
export class MySegmentsCacheInLocal extends AbstractMySegmentsCacheSync {
@@ -16,33 +16,22 @@ export class MySegmentsCacheInLocal extends AbstractMySegmentsCacheSync {
1616
this.log = log;
1717
this.keys = keys;
1818
this.storage = storage;
19-
// There is not need to flush segments cache like splits cache, since resetSegments receives the up-to-date list of active segments
2019
}
2120

2221
protected addSegment(name: string): boolean {
2322
const segmentKey = this.keys.buildSegmentNameKey(name);
2423

25-
try {
26-
if (this.storage.getItem(segmentKey) === DEFINED) return false;
27-
this.storage.setItem(segmentKey, DEFINED);
28-
return true;
29-
} catch (e) {
30-
this.log.error(LOG_PREFIX + e);
31-
return false;
32-
}
24+
if (this.storage.getItem(segmentKey) === DEFINED) return false;
25+
this.storage.setItem(segmentKey, DEFINED);
26+
return true;
3327
}
3428

3529
protected removeSegment(name: string): boolean {
3630
const segmentKey = this.keys.buildSegmentNameKey(name);
3731

38-
try {
39-
if (this.storage.getItem(segmentKey) !== DEFINED) return false;
40-
this.storage.removeItem(segmentKey);
41-
return true;
42-
} catch (e) {
43-
this.log.error(LOG_PREFIX + e);
44-
return false;
45-
}
32+
if (this.storage.getItem(segmentKey) !== DEFINED) return false;
33+
this.storage.removeItem(segmentKey);
34+
return true;
4635
}
4736

4837
isInSegment(name: string): boolean {
@@ -63,12 +52,8 @@ export class MySegmentsCacheInLocal extends AbstractMySegmentsCacheSync {
6352
}
6453

6554
protected setChangeNumber(changeNumber?: number) {
66-
try {
67-
if (changeNumber) this.storage.setItem(this.keys.buildTillKey(), changeNumber + '');
68-
else this.storage.removeItem(this.keys.buildTillKey());
69-
} catch (e) {
70-
this.log.error(e);
71-
}
55+
if (changeNumber) this.storage.setItem(this.keys.buildTillKey(), changeNumber + '');
56+
else this.storage.removeItem(this.keys.buildTillKey());
7257
}
7358

7459
getChangeNumber() {

‎src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts‎

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,10 @@ export class RBSegmentsCacheInLocal implements IRBSegmentsCacheSync {
2626
}
2727

2828
update(toAdd: IRBSegment[], toRemove: IRBSegment[], changeNumber: number): boolean {
29+
let updated = toAdd.map(toAdd => this.add(toAdd)).some(result => result);
30+
updated = toRemove.map(toRemove => this.remove(toRemove.name)).some(result => result) || updated;
2931
this.setChangeNumber(changeNumber);
30-
const updated = toAdd.map(toAdd => this.add(toAdd)).some(result => result);
31-
return toRemove.map(toRemove => this.remove(toRemove.name)).some(result => result) || updated;
32+
return updated;
3233
}
3334

3435
private setChangeNumber(changeNumber: number) {
@@ -48,40 +49,30 @@ export class RBSegmentsCacheInLocal implements IRBSegmentsCacheSync {
4849
}
4950

5051
private add(rbSegment: IRBSegment): boolean {
51-
try {
52-
const name = rbSegment.name;
53-
const rbSegmentKey = this.keys.buildRBSegmentKey(name);
54-
const rbSegmentFromStorage = this.storage.getItem(rbSegmentKey);
55-
const previous = rbSegmentFromStorage ? JSON.parse(rbSegmentFromStorage) : null;
52+
const name = rbSegment.name;
53+
const rbSegmentKey = this.keys.buildRBSegmentKey(name);
54+
const rbSegmentFromStorage = this.storage.getItem(rbSegmentKey);
55+
const previous = rbSegmentFromStorage ? JSON.parse(rbSegmentFromStorage) : null;
5656

57-
this.storage.setItem(rbSegmentKey, JSON.stringify(rbSegment));
57+
this.storage.setItem(rbSegmentKey, JSON.stringify(rbSegment));
5858

59-
let usesSegmentsDiff = 0;
60-
if (previous && usesSegments(previous)) usesSegmentsDiff--;
61-
if (usesSegments(rbSegment)) usesSegmentsDiff++;
62-
if (usesSegmentsDiff !== 0) this.updateSegmentCount(usesSegmentsDiff);
59+
let usesSegmentsDiff = 0;
60+
if (previous && usesSegments(previous)) usesSegmentsDiff--;
61+
if (usesSegments(rbSegment)) usesSegmentsDiff++;
62+
if (usesSegmentsDiff !== 0) this.updateSegmentCount(usesSegmentsDiff);
6363

64-
return true;
65-
} catch (e) {
66-
this.log.error(LOG_PREFIX + e);
67-
return false;
68-
}
64+
return true;
6965
}
7066

7167
private remove(name: string): boolean {
72-
try {
73-
const rbSegment = this.get(name);
74-
if (!rbSegment) return false;
68+
const rbSegment = this.get(name);
69+
if (!rbSegment) return false;
7570

76-
this.storage.removeItem(this.keys.buildRBSegmentKey(name));
71+
this.storage.removeItem(this.keys.buildRBSegmentKey(name));
7772

78-
if (usesSegments(rbSegment)) this.updateSegmentCount(-1);
73+
if (usesSegments(rbSegment)) this.updateSegmentCount(-1);
7974

80-
return true;
81-
} catch (e) {
82-
this.log.error(LOG_PREFIX + e);
83-
return false;
84-
}
75+
return true;
8576
}
8677

8778
private getNames(): string[] {

‎src/storages/inLocalStorage/SplitsCacheInLocal.ts‎

Lines changed: 22 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -80,44 +80,34 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync {
8080
}
8181

8282
addSplit(split: ISplit) {
83-
try {
84-
const name = split.name;
85-
const splitKey = this.keys.buildSplitKey(name);
86-
const splitFromStorage = this.storage.getItem(splitKey);
87-
const previousSplit = splitFromStorage ? JSON.parse(splitFromStorage) : null;
88-
89-
if (previousSplit) {
90-
this._decrementCounts(previousSplit);
91-
this.removeFromFlagSets(previousSplit.name, previousSplit.sets);
92-
}
83+
const name = split.name;
84+
const splitKey = this.keys.buildSplitKey(name);
85+
const splitFromStorage = this.storage.getItem(splitKey);
86+
const previousSplit = splitFromStorage ? JSON.parse(splitFromStorage) : null;
87+
88+
if (previousSplit) {
89+
this._decrementCounts(previousSplit);
90+
this.removeFromFlagSets(previousSplit.name, previousSplit.sets);
91+
}
9392

94-
this.storage.setItem(splitKey, JSON.stringify(split));
93+
this.storage.setItem(splitKey, JSON.stringify(split));
9594

96-
this._incrementCounts(split);
97-
this.addToFlagSets(split);
95+
this._incrementCounts(split);
96+
this.addToFlagSets(split);
9897

99-
return true;
100-
} catch (e) {
101-
this.log.error(LOG_PREFIX + e);
102-
return false;
103-
}
98+
return true;
10499
}
105100

106101
removeSplit(name: string): boolean {
107-
try {
108-
const split = this.getSplit(name);
109-
if (!split) return false;
102+
const split = this.getSplit(name);
103+
if (!split) return false;
110104

111-
this.storage.removeItem(this.keys.buildSplitKey(name));
105+
this.storage.removeItem(this.keys.buildSplitKey(name));
112106

113-
this._decrementCounts(split);
114-
this.removeFromFlagSets(split.name, split.sets);
107+
this._decrementCounts(split);
108+
this.removeFromFlagSets(split.name, split.sets);
115109

116-
return true;
117-
} catch (e) {
118-
this.log.error(LOG_PREFIX + e);
119-
return false;
120-
}
110+
return true;
121111
}
122112

123113
getSplit(name: string): ISplit | null {
@@ -206,6 +196,9 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync {
206196
const flagSetFromStorage = this.storage.getItem(flagSetKey);
207197

208198
const flagSetCache = new Set(flagSetFromStorage ? JSON.parse(flagSetFromStorage) : []);
199+
200+
if (flagSetCache.has(featureFlag.name)) return;
201+
209202
flagSetCache.add(featureFlag.name);
210203

211204
this.storage.setItem(flagSetKey, JSON.stringify(setToArray(flagSetCache)));

‎src/storages/inLocalStorage/__tests__/validateCache.spec.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ describe.each(storages)('validateCache', (storage) => {
4747
test('if there is cache and it must not be cleared, it should return true', async () => {
4848
storage.setItem(keys.buildSplitsTillKey(), '1');
4949
storage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH);
50+
await storage.save && storage.save();
5051

5152
expect(await validateCache({}, storage, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(true);
5253

@@ -66,6 +67,7 @@ describe.each(storages)('validateCache', (storage) => {
6667
storage.setItem(keys.buildSplitsTillKey(), '1');
6768
storage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH);
6869
storage.setItem(keys.buildLastUpdatedKey(), Date.now() - 1000 * 60 * 60 * 24 * 2 + ''); // 2 days ago
70+
await storage.save && storage.save();
6971

7072
expect(await validateCache({ expirationDays: 1 }, storage, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
7173

@@ -83,6 +85,7 @@ describe.each(storages)('validateCache', (storage) => {
8385
test('if there is cache and its hash has changed, it should clear cache and return false', async () => {
8486
storage.setItem(keys.buildSplitsTillKey(), '1');
8587
storage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH);
88+
await storage.save && storage.save();
8689

8790
expect(await validateCache({}, storage, { ...fullSettings, core: { ...fullSettings.core, authorizationKey: 'another-sdk-key' } }, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
8891

@@ -99,8 +102,10 @@ describe.each(storages)('validateCache', (storage) => {
99102

100103
test('if there is cache and clearOnInit is true, it should clear cache and return false', async () => {
101104
// Older cache version (without last clear)
105+
storage.removeItem(keys.buildLastClear());
102106
storage.setItem(keys.buildSplitsTillKey(), '1');
103107
storage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH);
108+
await storage.save && storage.save();
104109

105110
expect(await validateCache({ clearOnInit: true }, storage, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false);
106111

‎src/storages/inMemory/RBSegmentsCacheInMemory.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,10 @@ export class RBSegmentsCacheInMemory implements IRBSegmentsCacheSync {
1616
}
1717

1818
update(toAdd: IRBSegment[], toRemove: IRBSegment[], changeNumber: number): boolean {
19+
let updated = toAdd.map(toAdd => this.add(toAdd)).some(result => result);
20+
updated = toRemove.map(toRemove => this.remove(toRemove.name)).some(result => result) || updated;
1921
this.changeNumber = changeNumber;
20-
const updated = toAdd.map(toAdd => this.add(toAdd)).some(result => result);
21-
return toRemove.map(toRemove => this.remove(toRemove.name)).some(result => result) || updated;
22+
return updated;
2223
}
2324

2425
private add(rbSegment: IRBSegment): boolean {

‎src/storages/inRedis/SegmentsCacheInRedis.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { ILogger } from '../../logger/types';
22
import { isNaNNumber } from '../../utils/lang';
3-
import { LOG_PREFIX } from '../inLocalStorage/constants';
3+
import { LOG_PREFIX } from './constants';
44
import { KeyBuilderSS } from '../KeyBuilderSS';
55
import { ISegmentsCacheAsync } from '../types';
66
import type { RedisAdapter } from './RedisAdapter';

0 commit comments

Comments
 (0)