Merge pull request #89 from splitio/consumer_mode_timeout

Consumer mode timeout and vulnerability fixes

Author: EmilianoSanchez

CHANGES.txt

@@ -1,3 +1,9 @@
+0.9.6 (June 29, 2023)
+ - Updated some transitive dependencies for vulnerability fixes.
+ - Updated @splitsoftware/splitio-commons package to version 1.8.3 that includes:
+    - Updated SDK_READY_TIMED_OUT event to be emitted immediately when a connection error occurs using pluggable storage (i.e., when the wrapper `connect` promise is rejected) in consumer and partial consumer modes.
+    - Bugfix - The `destroy` method has been updated to immediately flag the SDK client as destroyed, to prevent unexpected behaviours when `getTreatment` and `track` methods are called synchronously after `destroy` method is called.
+
 0.9.5 (May 15, 2023)
  - Updated @splitsoftware/splitio-commons package to version 1.8.2 that includes minor improvements.
     - Updated terminology on the SDKs codebase to be more aligned with current standard without causing a breaking change. The core change is the term split for feature flag on things like logs and IntelliSense comments.

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@splitsoftware/splitio-browserjs",
-  "version": "0.9.5",
+  "version": "0.9.6",
   "description": "Split SDK for JavaScript on Browser",
   "main": "cjs/index.js",
   "module": "esm/index.js",
@@ -64,7 +64,7 @@
   "bugs": "https://github.com/splitio/javascript-browser-client/issues",
   "homepage": "https://github.com/splitio/javascript-browser-client#readme",
   "dependencies": {
-    "@splitsoftware/splitio-commons": "1.8.2",
+    "@splitsoftware/splitio-commons": "1.8.3",
     "@types/google.analytics": "0.0.40",
     "unfetch": "^4.2.0"
   },

package.json

@@ -558,5 +558,4 @@ tape('Browser Consumer mode with pluggable storage', function (t) {
 
   });
 
-
 });

src/__tests__/consumer/browser_consumer.spec.js

@@ -4,7 +4,7 @@ import fetchMock from '../testUtils/fetchMock';
 import { inMemoryWrapperFactory } from '@splitsoftware/splitio-commons/src/storages/pluggable/inMemoryWrapper';
 import { OPTIMIZED } from '@splitsoftware/splitio-commons/src/utils/constants';
 import { SDK_NOT_READY } from '@splitsoftware/splitio-commons/src/utils/labels';
-import { url } from '../testUtils';
+import { url, nearlyEqual } from '../testUtils';
 import { applyOperations } from './wrapper-commands';
 
 import { SplitFactory, PluggableStorage } from '../../';
@@ -341,4 +341,43 @@ tape('Browser Consumer Partial mode with pluggable storage', function (t) {
     });
   });
 
+  t.test('Wrapper connection error timeouts the SDK immediately', (assert) => {
+    // Mock a wrapper connection error
+    sinon.stub(wrapperInstance, 'connect').callsFake(() => { Promise.reject(); });
+    const getSpy = sinon.spy(wrapperInstance, 'get');
+
+    const sdk = SplitFactory(config);
+
+    const client = sdk.client();
+
+    client.ready().then(() => {
+      assert.fail('Ready promise should not be resolved if wrapper connection fails');
+    }, () => {
+      assert.pass('Ready promise should be rejected if wrapper connection fails');
+    });
+
+    const start = Date.now();
+    client.on(client.Event.SDK_READY_TIMED_OUT, async () => {
+      assert.true(nearlyEqual(Date.now() - start, 0), 'SDK_READY_TIMED_OUT event is emitted immediately');
+
+      // Client methods behave as if the SDK is not ready
+      assert.equal(await client.getTreatment('UT_IN_SEGMENT'), 'control', 'treatment is control with label not ready.');
+      assert.true(await client.track('user', 'test.event', 18), 'event is tracked in memory (partial consumer mode).');
+
+      // Shared clients will also timeout immediately and behave as if the SDK is not ready
+      const otherClient = sdk.client('other_user');
+      otherClient.on(otherClient.Event.SDK_READY_TIMED_OUT, async () => {
+        assert.true(nearlyEqual(Date.now() - start, 0), 'SDK_READY_TIMED_OUT event is emitted immediately in shared client');
+
+        assert.equal(await otherClient.getTreatment('UT_IN_SEGMENT'), 'control', 'treatment is control with label not ready.');
+        assert.true(await otherClient.track('user', 'test.event', 18), 'event is tracked in memory (partial consumer mode).');
+
+        await client.destroy();
+        assert.equal(getSpy.callCount, 0, '`getTreatment` shouldn\'t have called wrapper methods if SDK is not ready');
+
+        wrapperInstance.connect.restore();
+        assert.end();
+      });
+    });
+  });
 });

src/__tests__/consumer/browser_consumer_partial.spec.js

@@ -114,8 +114,7 @@ tape('SDK destroy for BrowserJS', async function (assert) {
   assert.ok(manager.names().length > 0, 'control assertion');
   assert.notOk(client2.track('tt', 'eventType', 2), 'After destroy, track calls return false.');
 
-  await client.destroy();
-  fetchMock.restore();
+  const destroyPromise = client.destroy();
 
   assert.equal(client.getTreatment('Single_Test'), 'control', 'After destroy, getTreatment returns control for every destroyed client.');
   assert.deepEqual(client.getTreatments(['Single_Test']), { 'Single_Test': 'control' }, 'After destroy, getTreatments returns map of controls for every destroyed client.');
@@ -125,5 +124,8 @@ tape('SDK destroy for BrowserJS', async function (assert) {
   assert.equal(manager.names().length, 0, 'After the main client is destroyed, manager.names will return empty array');
   assert.equal(manager.split('Single_Test'), null, 'After the main client is destroyed, manager.split will return null');
 
+  await destroyPromise;
+  fetchMock.restore();
+
   assert.end();
 });

src/__tests__/destroy/browser.spec.js

@@ -2,7 +2,7 @@ import { LogLevels, isLogLevelString } from '@splitsoftware/splitio-commons/src/
 import { ConsentStatus, LogLevel } from '@splitsoftware/splitio-commons/src/types';
 import { CONSENT_GRANTED } from '@splitsoftware/splitio-commons/src/utils/constants';
 
-const packageVersion = '0.9.5';
+const packageVersion = '0.9.6';
 
 /**
  * In browser, the default debug level, can be set via the `localStorage.splitio_debug` item.

src/settings/defaults.ts

@@ -1342,14 +1342,14 @@ declare namespace SplitIO {
      */
     setAttribute(attributeName: string, attributeValue: AttributeType): boolean,
     /**
-     * Returns the attribute with the given key.
+     * Returns the attribute with the given name.
      *
      * @param {string} attributeName Attribute name
-     * @returns {AttributeType} Attribute with the given key
+     * @returns {AttributeType} Attribute with the given name
      */
     getAttribute(attributeName: string): AttributeType,
     /**
-     * Removes from client's in memory attributes storage the attribute with the given key.
+     * Removes from client's in memory attributes storage the attribute with the given name.
      *
      * @param {string} attributeName
      * @returns {boolean} true if attribute was removed and false otherwise
@@ -1432,14 +1432,14 @@ declare namespace SplitIO {
      */
     setAttribute(attributeName: string, attributeValue: AttributeType): boolean,
     /**
-     * Returns the attribute with the given key.
+     * Returns the attribute with the given name.
      *
      * @param {string} attributeName Attribute name
-     * @returns {AttributeType} Attribute with the given key
+     * @returns {AttributeType} Attribute with the given name
      */
     getAttribute(attributeName: string): AttributeType,
     /**
-     * Removes from client's in memory attributes storage the attribute with the given key.
+     * Removes from client's in memory attributes storage the attribute with the given name.
      *
      * @param {string} attributeName
      * @returns {boolean} true if attribute was removed and false otherwise