fix(features): return 400 instead of 500 on feature name race condition

api/features/serializers.py

@@ -11,7 +11,7 @@
     FeatureStateValueSerializer,
 )
 from common.projects.permissions import VIEW_PROJECT
-from django.db import models
+from django.db import IntegrityError, models
 from drf_spectacular.utils import extend_schema_field
 from drf_writable_nested import (  # type: ignore[attr-defined]
     WritableNestedModelSerializer,
@@ -58,6 +58,11 @@
 from .multivariate.models import MultivariateFeatureOption
 from .multivariate.serializers import NestedMultivariateFeatureOptionSerializer
 
+DUPLICATE_FEATURE_NAME_ERROR = (
+    "Feature with that name already exists for this project. Note that feature "
+    "names are case insensitive."
+)
+
 
 class FeatureStateSerializerSmall(serializers.ModelSerializer):  # type: ignore[type-arg]
     feature_state_value = serializers.SerializerMethodField()
@@ -275,7 +280,14 @@ def create(self, validated_data: dict) -> Feature:  # type: ignore[type-arg]
         group_owners: list[UserPermissionGroup] = validated_data.pop("group_owners", [])
 
         user = validated_data.pop("user", None)
-        instance = super(CreateFeatureSerializer, self).create(validated_data)  # type: ignore[no-untyped-call]
+        try:
+            instance = super(CreateFeatureSerializer, self).create(validated_data)  # type: ignore[no-untyped-call]
+        except IntegrityError:
+            # A concurrent request may create a feature with the same name after
+            # `validate_name` has run but before this insert, tripping the
+            # case-insensitive unique constraint. Surface this as a 400 rather
+            # than letting the IntegrityError bubble up as an HTTP 500.
+            raise serializers.ValidationError({"name": [DUPLICATE_FEATURE_NAME_ERROR]})
 
         if owners:
             instance.owners.add(*owners)
@@ -369,11 +381,7 @@ def validate_name(self, name: str):  # type: ignore[no-untyped-def]
             )
 
         if existing_feature_queryset.exists():
-            raise serializers.ValidationError(
-                "Feature with that name already exists for this "
-                "project. Note that feature names are case "
-                "insensitive."
-            )
+            raise serializers.ValidationError(DUPLICATE_FEATURE_NAME_ERROR)
 
         return name
 

api/tests/unit/features/test_unit_features_views.py

@@ -1806,6 +1806,35 @@ def test_create_feature__name_does_not_match_regex__returns_400(
     )
 
 
+def test_create_feature__name_race_condition__returns_400(
+    admin_client_new: APIClient,
+    project: Project,
+    feature: Feature,
+    mocker: MockerFixture,
+) -> None:
+    # Given
+    # Simulate a race where `validate_name` passes (e.g. a concurrent request
+    # creates the feature after validation runs) but the database's
+    # case-insensitive unique constraint rejects the duplicate name on insert.
+    mocker.patch(
+        "features.serializers.CreateFeatureSerializer.validate_name",
+        side_effect=lambda name: name,
+    )
+
+    url = reverse("api-v1:projects:project-features-list", args=[project.id])
+    data = {"name": feature.name, "type": STANDARD, "project": project.id}
+
+    # When
+    response = admin_client_new.post(url, data=data)
+
+    # Then
+    assert response.status_code == status.HTTP_400_BAD_REQUEST
+    assert response.json()["name"][0] == (
+        "Feature with that name already exists for this project. Note that "
+        "feature names are case insensitive."
+    )
+
+
 def test_create_feature__valid_data__creates_audit_log(
     admin_client_new: APIClient,
     project: Project,