Merge pull request #409 from splitio/rb_segments_fixes

[Rule-based segments] Fixes and polishing

Author: EmilianoSanchez

CHANGES.txt

@@ -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

src/dtos/types.ts

@@ -210,8 +210,8 @@ export interface IRBSegment {
   status: 'ACTIVE' | 'ARCHIVED',
   conditions?: ISplitCondition[],
   excluded?: {
-    keys?: string[],
-    segments?: IExcludedSegment[]
+    keys?: string[] | null,
+    segments?: IExcludedSegment[] | null
   }
 }
 

src/evaluator/matchers/__tests__/rbsegment.spec.ts

@@ -95,8 +95,8 @@ const STORED_RBSEGMENTS: Record<string, IRBSegment> = {
     changeNumber: 123,
     status: 'ACTIVE',
     excluded: {
-      keys: [],
-      segments: []
+      keys: null,
+      segments: null,
     },
     conditions: [{
       matcherGroup: {
@@ -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: []
   }
 };
 
@@ -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);
   });
 
 });

src/evaluator/matchers/rbsegment.ts

@@ -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(

src/services/splitHttpClient.ts

@@ -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.';
 
@@ -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;

src/sync/polling/fetchers/splitChangesFetcher.ts

@@ -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;
 }
 
@@ -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