From 1078dd1c41786d90c5a0877e521eec370d7c4c81 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nuno=20G=C3=B3is?= <github@nunogois.com>
Date: Tue, 8 Apr 2025 09:32:40 +0100
Subject: [PATCH] fix: return bad data error when failing to patch env variants
 (#9708)

https://linear.app/unleash/issue/2-3483/variants-endpoint-should-return-400-or-other-more-appropriate-status

Throws BadDataError (returns 400) instead of 500 by wrapping the patch
logic with a try catch.

Added a test that validates the new behavior.
---
 .../feature-toggle/feature-toggle-service.ts  | 34 +++++++++-----
 .../admin/project/variants-sunset.e2e.test.ts | 47 +++++++++++++++++++
 2 files changed, 68 insertions(+), 13 deletions(-)

diff --git a/src/lib/features/feature-toggle/feature-toggle-service.ts b/src/lib/features/feature-toggle/feature-toggle-service.ts
index 96e96f2735..fa078bcb95 100644
--- a/src/lib/features/feature-toggle/feature-toggle-service.ts
+++ b/src/lib/features/feature-toggle/feature-toggle-service.ts
@@ -2226,19 +2226,27 @@ class FeatureToggleService {
             featureName,
             environment,
         );
-        const { newDocument } = await applyPatch(
-            deepClone(oldVariants),
-            newVariants,
-        );
-        return this.crProtectedSaveVariantsOnEnv(
-            project,
-            featureName,
-            environment,
-            newDocument,
-            user,
-            auditUser,
-            oldVariants,
-        );
+
+        try {
+            const { newDocument } = await applyPatch(
+                deepClone(oldVariants),
+                newVariants,
+            );
+
+            return this.crProtectedSaveVariantsOnEnv(
+                project,
+                featureName,
+                environment,
+                newDocument,
+                user,
+                auditUser,
+                oldVariants,
+            );
+        } catch (e) {
+            throw new BadDataError(
+                `Could not apply provided patch: ${e.message}`,
+            );
+        }
     }
 
     async saveVariants(
diff --git a/src/test/e2e/api/admin/project/variants-sunset.e2e.test.ts b/src/test/e2e/api/admin/project/variants-sunset.e2e.test.ts
index 554f89c732..8c6fa8bf4f 100644
--- a/src/test/e2e/api/admin/project/variants-sunset.e2e.test.ts
+++ b/src/test/e2e/api/admin/project/variants-sunset.e2e.test.ts
@@ -108,3 +108,50 @@ test('Can add environment variants when existing ones exist for this feature', a
         .send(patch)
         .expect(200);
 });
+
+test('Patching variants with an invalid patch payload should return a BadDataError', async () => {
+    const featureName = 'feature-variants-patch';
+
+    await db.stores.featureToggleStore.create('default', {
+        name: featureName,
+        createdByUserId: 9999,
+    });
+    await db.stores.featureEnvironmentStore.addEnvironmentToFeature(
+        featureName,
+        'development',
+        true,
+    );
+    await db.stores.featureToggleStore.saveVariants('default', featureName, [
+        {
+            name: 'existing-variant',
+            stickiness: 'default',
+            weight: 1000,
+            weightType: WeightType.VARIABLE,
+        },
+    ]);
+
+    const patch = [
+        {
+            op: 'add',
+            path: '/2/overrides', // Invalid path
+            value: {
+                name: 'a',
+                weightType: WeightType.VARIABLE,
+                weight: 1000,
+            },
+        },
+    ];
+
+    await app.request
+        .patch(
+            `/api/admin/projects/default/features/${featureName}/environments/development/variants`,
+        )
+        .send(patch)
+        .expect(400)
+        .expect((res) => {
+            expect(res.body.name).toBe('BadDataError');
+            expect(res.body.message).toBe(
+                `Request validation failed: your request body or params contain invalid data: Could not apply provided patch: Cannot set properties of undefined (setting 'overrides')`,
+            );
+        });
+});