fix: edge case on context value where trim is not a function (#648)

Unleash · Jul 23, 2024 · 2ccb88a · 2ccb88a
1 parent e61e55a
commit 2ccb88a
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 2 deletions.
diff --git a/src/helpers.ts b/src/helpers.ts
@@ -19,7 +19,8 @@ export function createFallbackFunction(
 }
 
 export function resolveContextValue(context: Context, field: string): string | undefined {
-  return context[field]?.toString() ?? context.properties?.[field]?.toString();
+  const contextValue = context[field] ?? context.properties?.[field];
+  return contextValue !== undefined && contextValue !== null ? String(contextValue) : undefined;
 }
 
 export function safeName(str: string = '') {

diff --git a/src/strategy/strategy.ts b/src/strategy/strategy.ts
@@ -93,11 +93,14 @@ const SemverOperator = (constraint: Constraint, context: Context) => {
   const { contextName, operator } = constraint;
   const value = constraint.value as string;
   const contextValue = resolveContextValue(context, contextName);
-  if (!contextValue || !isStrictSemver(contextValue)) {
+  if (!contextValue) {
     return false;
   }
 
   try {
+    if (!isStrictSemver(contextValue)) {
+      return false;
+    }
     if (operator === Operator.SEMVER_EQ) {
       return semverEq(contextValue, value);
     }

diff --git a/src/test/strategy/flexible-rollout-test.ts → src/test/strategy/flexible-rollout.test.ts b/src/test/strategy/flexible-rollout-test.ts → src/test/strategy/flexible-rollout.test.ts
diff --git a/src/test/strategy/strategy-test.ts → src/test/strategy/strategy.test.ts b/src/test/strategy/strategy-test.ts → src/test/strategy/strategy.test.ts
@@ -602,3 +602,24 @@ test('should be enabled for missing field when NOT_IN', (t) => {
   // @ts-expect-error
   t.true(strategy.isEnabledWithConstraints(params, context, constraints));
 });
+
+test('should gracefully handle version with custom toString that does not return string', (t) => {
+  const strategy = new Strategy('test', true);
+  const params = {};
+  const constraints = [{ contextName: 'version', operator: 'SEMVER_EQ', value: '1.2.2' }];
+  const context = {
+    environment: 'dev',
+    properties: { version: { toString: () => 122 } },
+  };
+
+  try {
+    // @ts-expect-error
+    const enabled = strategy.isEnabledWithConstraints(params, context, constraints);
+    t.true(enabled || !enabled); // This will pass if no error is thrown
+  } catch (error: unknown) {
+    // @ts-expect-error
+    t.fail(`Threw error: ${error.message}`);
+  }
+
+  t.pass();
+});