From 2ccb88a744c913cb21b9853e3d3c3ab9f03f36c0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nuno=20G=C3=B3is?= <github@nunogois.com>
Date: Tue, 23 Jul 2024 11:44:33 +0100
Subject: [PATCH] fix: edge case on context value where trim is not a function
 (#648)

---
 src/helpers.ts                                |  3 ++-
 src/strategy/strategy.ts                      |  5 ++++-
 ...llout-test.ts => flexible-rollout.test.ts} |  0
 .../{strategy-test.ts => strategy.test.ts}    | 21 +++++++++++++++++++
 4 files changed, 27 insertions(+), 2 deletions(-)
 rename src/test/strategy/{flexible-rollout-test.ts => flexible-rollout.test.ts} (100%)
 rename src/test/strategy/{strategy-test.ts => strategy.test.ts} (96%)

diff --git a/src/helpers.ts b/src/helpers.ts
index 97a21c9a..f96decd9 100644
--- 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
index 5cbf9858..583cd040 100644
--- 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 b/src/test/strategy/flexible-rollout.test.ts
similarity index 100%
rename from src/test/strategy/flexible-rollout-test.ts
rename to src/test/strategy/flexible-rollout.test.ts
diff --git a/src/test/strategy/strategy-test.ts b/src/test/strategy/strategy.test.ts
similarity index 96%
rename from src/test/strategy/strategy-test.ts
rename to src/test/strategy/strategy.test.ts
index 1d7d0c6c..fc75d0c7 100644
--- a/src/test/strategy/strategy-test.ts
+++ b/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();
+});