fix: evaluation event being reported with no user ID (#89)

src/__tests__/assert.ts

@@ -0,0 +1,75 @@
+import test from 'ava';
+import { User } from '../objects/user';
+import { assertGetEvaluationRequest } from '../assert';
+
+const assertGetEvaluationRequestTestCases = [
+  {
+    name: 'assertGetEvaluationRequest throws error for invalid user',
+    user: { id: '', data: {} },
+    featureID: 'feature1',
+    errorMessage: 'userID is empty'
+  },
+  {
+    name: 'assertGetEvaluationRequest throws error for missing featureID',
+    user: { id: '123', data: {} },
+    featureID: '',
+    errorMessage: 'featureID is required'
+  },
+  {
+    name: 'assertGetEvaluationRequest throws error for null user',
+    user: null as any,
+    featureID: 'feature1',
+    errorMessage: 'user is null or undefined'
+  },
+  {
+    name: 'assertGetEvaluationRequest throws error for undefined user',
+    user: undefined as any,
+    featureID: 'feature1',
+    errorMessage: 'user is null or undefined'
+  },
+  {
+    name: 'assertGetEvaluationRequest throws error for user with null id',
+    user: { id: null as any, data: {} },
+    featureID: 'feature1',
+    errorMessage: 'userID is empty'
+  },
+  {
+    name: 'assertGetEvaluationRequest throws error for user with undefined id',
+    user: { id: undefined as any, data: {} },
+    featureID: 'feature1',
+    errorMessage: 'userID is empty'
+  },
+  {
+    name: 'assertGetEvaluationRequest throws error for null featureID',
+    user: { id: '123', data: {} },
+    featureID: null as any,
+    errorMessage: 'featureID is required'
+  },
+  {
+    name: 'assertGetEvaluationRequest throws error for undefined featureID',
+    user: { id: '123', data: {} },
+    featureID: undefined as any,
+    errorMessage: 'featureID is required'
+  }
+];
+
+assertGetEvaluationRequestTestCases.forEach(testCase => {
+  test(testCase.name, t => {
+    const error = t.throws(() => {
+      assertGetEvaluationRequest(testCase.user, testCase.featureID);
+    }, { instanceOf: Error });
+    t.is(error.message, testCase.errorMessage);
+  });
+});
+
+test('assertGetEvaluationRequest does not throw error for valid user and featureID', t => {
+  const user: User = {
+    id: '123',
+    data: {}
+  };
+  t.notThrows(() => {
+    assertGetEvaluationRequest(user, 'feature1');
+  });
+});
+
+

src/__tests__/client.ts

@@ -1,7 +1,7 @@
 import { http, HttpResponse } from 'msw';
 import { SetupServer } from 'msw/node';
 import anyTest, { TestFn } from 'ava';
-import { Bucketeer, initialize } from '..';
+import { BKTClientImpl, Bucketeer, initialize } from '..';
 import { Config, User } from '../bootstrap';
 import { DefaultLogger } from '../logger';
 import { setupServerAndListen } from './utils/setup_server';

src/__tests__/client_assert_get_evaluation_rq.ts

@@ -0,0 +1,116 @@
+import anyTest, { TestFn } from 'ava';
+import { BKTClientImpl, Bucketeer, initialize } from '..';
+import { Config, User } from '../bootstrap';
+import { DefaultLogger } from '../logger';
+import { newDefaultBKTEvaluationDetails } from '../evaluationDetails';
+
+const test = anyTest as TestFn<{
+  bktClient: Bucketeer;
+  targetedUser: User;
+  config: Config;
+}>;
+
+test.beforeEach((t) => {
+  const config = {
+    host: 'api.bucketeer.io',
+    token: 'api_key_value',
+    tag: 'feature_tag_value',
+    logger: new DefaultLogger('expected'),
+  };
+  t.context = {
+    bktClient: initialize(config),
+    targetedUser: { id: 'user_id', data: {} },
+    config: config,
+  };
+});
+
+const testCases = [
+  {
+    description: 'return default value when featureID is empty',
+    featureId: '',
+    user: { id: 'user_id', data: {} },
+    expected: newDefaultBKTEvaluationDetails('user_id', '', 'default-test', 'DEFAULT'),
+  },
+  {
+    description: 'return default value when userID is empty',
+    featureId: 'stringEvaluationDetails',
+    user: { id: '', data: {} },
+    expected: newDefaultBKTEvaluationDetails('', 'stringEvaluationDetails', 'default-test', 'DEFAULT'),
+  },
+  {
+    description: 'return default value when userID & featureID is empty',
+    featureId: '',
+    user: { id: '', data: {} },
+    expected: newDefaultBKTEvaluationDetails('', '', 'default-test', 'DEFAULT'),
+  },
+  // Simulate the case where the object is null when passed from JavaScript code.
+  {
+    description: 'return default value when userID is null',
+    featureId: 'featureId',
+    user: null,
+    expected: newDefaultBKTEvaluationDetails('', 'featureId', 'default-test', 'DEFAULT'),
+  },
+  {
+    description: 'return default value when featureId is null',
+    featureId: null,
+    user: { id: 'user_id', data: {} },
+    expected: newDefaultBKTEvaluationDetails('user_id', '', 'default-test', 'DEFAULT'),
+  },
+  {
+    description: 'return default value when featureId & userID is null',
+    featureId: null,
+    user: null,
+    expected: newDefaultBKTEvaluationDetails('', '', 'default-test', 'DEFAULT'),
+  },
+  // Simulate the case where the user.id is null when passed from JavaScript code.
+  {
+    description: 'return default value when user.id is null',
+    featureId: 'featureId',
+    user: { id: null, data: {} },
+    expected: newDefaultBKTEvaluationDetails('', 'featureId', 'default-test', 'DEFAULT'),
+  },
+  // Simulate the case where the user.id is undefined when passed from JavaScript code.
+  {
+    description: 'return default value when user.id is undefined',
+    featureId: 'featureId',
+    user: { id: undefined, data: {} },
+    expected: newDefaultBKTEvaluationDetails('', 'featureId', 'default-test', 'DEFAULT'),
+  },
+  // Simulate the case where the object is undefined when passed from JavaScript code.
+  {
+    description: 'return default value when userID is undefined',
+    featureId: 'featureId',
+    user: undefined,
+    expected: newDefaultBKTEvaluationDetails('', 'featureId', 'default-test', 'DEFAULT'),
+  },
+  {
+    description: 'return default value when featureId is undefined',
+    featureId: undefined,
+    user: { id: 'user_id', data: {} },
+    expected: newDefaultBKTEvaluationDetails('user_id', '', 'default-test', 'DEFAULT'),
+  },
+  {
+    description: 'return default value when featureId & userID is undefined',
+    featureId: undefined,
+    user: undefined,
+    expected: newDefaultBKTEvaluationDetails('', '', 'default-test', 'DEFAULT'),
+  },
+];
+
+for (const testCase of testCases) {
+  test.serial(`getEvaluation: ${testCase.description}`, async (t) => {
+    const client = t.context.bktClient;
+    const clientImpl = client as BKTClientImpl;
+
+    t.deepEqual(
+      // Type cast for simulate the case where the user object is null when passed from JavaScript code.
+      await client.stringVariationDetails(testCase.user as User, testCase.featureId as string, 'default-test'),
+      testCase.expected,
+    );
+    t.true(clientImpl.eventStore.size() == 0, 'eventStore should be empty and not contain any error events');
+  });
+}
+
+test.afterEach.always((t) => {
+  t.context.bktClient.destroy();
+});

src/__tests__/default_bkt_evaluation.ts

@@ -33,3 +33,18 @@ newDefaultBKTEvaluationDetailsTests.forEach((defaultValue, index) => {
     });
   });
 });
+
+test('newDefaultBKTEvaluationDetails should return correct reason `DEFAULT`', (t) => {
+  const featureId = 'test_flag';
+  const userId = 'user1';
+  let output = newDefaultBKTEvaluationDetails(userId, featureId, 'default true', 'DEFAULT');
+  t.deepEqual(output, {
+    featureId: featureId,
+    featureVersion: 0,
+    userId: userId,
+    variationId: '',
+    variationName: '',
+    variationValue: 'default true',
+    reason: 'DEFAULT',
+  });
+});

src/assert.ts

@@ -0,0 +1,24 @@
+import { User } from './objects/user'
+
+/**
+ * Asserts that the provided user and featureID are valid for an evaluation request.
+ *
+ * @param user - The user object to be validated.
+ * @param featureID - The feature ID to be validated.
+ * @throws Will throw an error if the user is invalid or if the featureID is not provided.
+ */
+function assertGetEvaluationRequest(user: User, featureID: string) {
+  if (!user) {
+    throw new Error('user is null or undefined')
+  }
+
+  if (!user.id) {
+    throw new Error('userID is empty')
+  }
+
+  if (!featureID) {
+    throw new Error('featureID is required')
+  }
+}
+
+export { assertGetEvaluationRequest }

src/evaluationDetails.ts

@@ -14,6 +14,7 @@ export const newDefaultBKTEvaluationDetails = <T extends BKTValue>(
   userId: string,
   featureId: string,
   defaultValue: T,
+  reason: 'DEFAULT' | 'CLIENT' = 'CLIENT'
 ): BKTEvaluationDetails<T> => {
   return {
     featureId: featureId,
@@ -22,6 +23,6 @@ export const newDefaultBKTEvaluationDetails = <T extends BKTValue>(
     variationId: '',
     variationName: '',
     variationValue: defaultValue,
-    reason: 'CLIENT',
+    reason: reason,
   } satisfies BKTEvaluationDetails<T>;
 };

src/index.ts

@@ -25,6 +25,7 @@ import {
   StringToTypeConverter,
 } from './converter';
 import { error } from 'console';
+import { assertGetEvaluationRequest } from './assert';
 
 export interface BuildInfo {
   readonly GIT_REVISION: string;
@@ -57,7 +58,6 @@ export interface Bucketeer {
    */
   getNumberVariation(user: User, featureId: string, defaultValue: number): Promise<number>;
 
-
   /**
    * booleanVariation returns variation as boolean.
    * If a variation returned by server is not boolean, defaultValue is retured.
@@ -75,13 +75,13 @@ export interface Bucketeer {
   ): Promise<BKTEvaluationDetails<boolean>>;
 
   /**
- * stringVariation returns variation as string.
- * If a variation returned by server is not string, defaultValue is retured.
- * @param user User information.
- * @param featureId Feature flag ID to use.
- * @param defaultValue The variation value that is retured if SDK fails to fetch the variation or the variation is not string.
- * @returns The variation value returned from server or default value.
- */
+   * stringVariation returns variation as string.
+   * If a variation returned by server is not string, defaultValue is retured.
+   * @param user User information.
+   * @param featureId Feature flag ID to use.
+   * @param defaultValue The variation value that is retured if SDK fails to fetch the variation or the variation is not string.
+   * @returns The variation value returned from server or default value.
+   */
   stringVariation(user: User, featureId: string, defaultValue: string): Promise<string>;
 
   stringVariationDetails(
@@ -307,6 +307,17 @@ export class BKTClientImpl implements Bucketeer {
     defaultValue: T,
     typeConverter: StringToTypeConverter<T>,
   ): Promise<BKTEvaluationDetails<T>> {
+    try {
+      assertGetEvaluationRequest(user, featureId);
+    } catch (error) {
+      this.config.logger?.error('getVariationDetails failed', error);
+      return newDefaultBKTEvaluationDetails(
+        user && user.id ? user.id : '',
+        featureId ?? '',
+        defaultValue,
+        'DEFAULT');
+    }
+
     const evaluation = await this.getEvaluation(user, featureId);
     const variationValue = evaluation?.variationValue;