Skip to content

[Rule-based segments] Fixes and polishing #409

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGES.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
2.3.0 (May XXX, 2025)
2.3.0 (May 12, 2025)
- Added support for targeting rules based on rule-based segments.
- Updated the Redis storage to:
- Avoid lazy require of the `ioredis` dependency when the SDK is initialized, and
Expand Down
4 changes: 2 additions & 2 deletions src/dtos/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,8 @@ export interface IRBSegment {
status: 'ACTIVE' | 'ARCHIVED',
conditions?: ISplitCondition[],
excluded?: {
keys?: string[],
segments?: IExcludedSegment[]
keys?: string[] | null,
segments?: IExcludedSegment[] | null
}
}

Expand Down
18 changes: 16 additions & 2 deletions src/evaluator/matchers/__tests__/rbsegment.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ const STORED_RBSEGMENTS: Record<string, IRBSegment> = {
changeNumber: 123,
status: 'ACTIVE',
excluded: {
keys: [],
segments: []
keys: null,
segments: null,
},
conditions: [{
matcherGroup: {
Expand Down Expand Up @@ -167,6 +167,12 @@ const STORED_RBSEGMENTS: Record<string, IRBSegment> = {
}
}
],
},
'rule_based_segment_without_conditions': {
name: 'rule_based_segment_without_conditions',
changeNumber: 123,
status: 'ACTIVE',
conditions: []
}
};

Expand Down Expand Up @@ -291,6 +297,14 @@ describe.each([

// should support feature flag dependency matcher
expect(await matcherTrueAlwaysOn({ key: 'a-key' }, evaluateFeature)).toBe(true); // Parent split returns one of the expected treatments, so the matcher returns true

const matcherTrueRuleBasedSegmentWithoutConditions = matcherFactory(loggerMock, {
type: matcherTypes.IN_RULE_BASED_SEGMENT,
value: 'rule_based_segment_without_conditions'
} as IMatcherDto, mockStorageSync)!;

// should support rule-based segment without conditions
expect(await matcherTrueRuleBasedSegmentWithoutConditions({ key: 'a-key' }, evaluateFeature)).toBe(false);
});

});
3 changes: 3 additions & 0 deletions src/evaluator/matchers/rbsegment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ export function ruleBasedSegmentMatcherContext(segmentName: string, storage: ISt

function matchConditions(rbsegment: IRBSegment) {
const conditions = rbsegment.conditions || [];

if (!conditions.length) return false;

const evaluator = parser(log, conditions, storage);

const evaluation = evaluator(
Expand Down
4 changes: 3 additions & 1 deletion src/services/splitHttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ERROR_HTTP, ERROR_CLIENT_CANNOT_GET_READY } from '../logger/constants';
import { ISettings } from '../types';
import { IPlatform } from '../sdkFactory/types';
import { decorateHeaders, removeNonISO88591 } from './decorateHeaders';
import { timeout } from '../utils/promise/timeout';

const messageNoFetch = 'Global fetch API is not available.';

Expand Down Expand Up @@ -45,7 +46,8 @@ export function splitHttpClientFactory(settings: ISettings, { getOptions, getFet
// https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch#Checking_that_the_fetch_was_successful
.then(response => {
if (!response.ok) {
return response.text().then(message => Promise.reject({ response, message }));
// timeout after 100ms because `text()` promise doesn't settle in some implementations and cases (e.g. no content)
return timeout(100, response.text()).then(message => Promise.reject({ response, message }), () => Promise.reject({ response }));
}
latencyTracker();
return response;
Expand Down
6 changes: 3 additions & 3 deletions src/sync/polling/fetchers/splitChangesFetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { LOG_PREFIX_SYNC_SPLITS } from '../../../logger/constants';
const PROXY_CHECK_INTERVAL_MILLIS_CS = 60 * 60 * 1000; // 1 hour in Client Side
const PROXY_CHECK_INTERVAL_MILLIS_SS = 24 * PROXY_CHECK_INTERVAL_MILLIS_CS; // 24 hours in Server Side

function sdkEndpointOverriden(settings: ISettings) {
function sdkEndpointOverridden(settings: ISettings) {
return settings.urls.sdk !== base.urls.sdk;
}

Expand Down Expand Up @@ -42,8 +42,8 @@ export function splitChangesFetcherFactory(fetchSplitChanges: IFetchSplitChanges
let splitsPromise = fetchSplitChanges(since, noCache, till, settings.sync.flagSpecVersion === FLAG_SPEC_VERSION ? rbSince : undefined)
// Handle proxy error with spec 1.3
.catch((err) => {
if (err.statusCode === 400 && sdkEndpointOverriden(settings) && settings.sync.flagSpecVersion === FLAG_SPEC_VERSION) {
log.error(LOG_PREFIX_SYNC_SPLITS + 'Proxy error detected. If you are using Split Proxy, please upgrade to latest version');
if (err.statusCode === 400 && sdkEndpointOverridden(settings) && settings.sync.flagSpecVersion === FLAG_SPEC_VERSION) {
log.error(LOG_PREFIX_SYNC_SPLITS + 'Proxy error detected. Retrying with spec 1.2. If you are using Split Proxy, please upgrade to latest version');
lastProxyCheckTimestamp = Date.now();
settings.sync.flagSpecVersion = '1.2'; // fallback to 1.2 spec
return fetchSplitChanges(since, noCache, till); // retry request without rbSince
Expand Down