mirror of
				https://github.com/Unleash/unleash.git
				synced 2025-10-27 11:02:16 +01:00 
			
		
		
		
	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.
This commit is contained in:
		
							parent
							
								
									494d5fd749
								
							
						
					
					
						commit
						1078dd1c41
					
				@ -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(
 | 
			
		||||
 | 
			
		||||
@ -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')`,
 | 
			
		||||
            );
 | 
			
		||||
        });
 | 
			
		||||
});
 | 
			
		||||
 | 
			
		||||
		Loading…
	
		Reference in New Issue
	
	Block a user