fix: return invalid argument for update feature validation errors (#2765)

Author: cre8ivejp

pkg/feature/api/feature.go

@@ -605,7 +605,10 @@ func (s *FeatureService) UpdateFeature(
 		return err
 	})
 	if err != nil {
-		return nil, err
+		// Convert domain errors to gRPC statuses; otherwise validation errors
+		// (e.g. variation value schema violations) surface as codes.Unknown
+		// (HTTP 500) without the structured details the console relies on.
+		return nil, s.convUpdateFeatureError(err)
 	}
 	if errs := s.publishDomainEvents(ctx, []*eventproto.Event{event}); len(errs) > 0 {
 		s.logger.Error(

pkg/feature/api/feature_test.go

@@ -23,7 +23,9 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/mock/gomock"
+	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/metadata"
+	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/types/known/wrapperspb"
 
 	"github.com/bucketeer-io/bucketeer/v2/pkg/api/api"
@@ -619,6 +621,14 @@ func TestConvUpdateFeatureError(t *testing.T) {
 		err := fs.convUpdateFeatureError(p.input)
 		assert.Equal(t, p.expectedErr, err)
 	}
+
+	// UpdateFeature relies on this conversion to surface domain validation
+	// errors (e.g. variation value schema violations) as InvalidArgument
+	// with structured details instead of Unknown without details.
+	fs := &FeatureService{}
+	schemaErr := fs.convUpdateFeatureError(pkgErr.NewErrorInvalidArgNotMatchFormat(
+		pkgErr.FeaturePackageName, "feature: variation value does not match schema", "VariationValueSchema"))
+	assert.Equal(t, codes.InvalidArgument, status.Code(schemaErr))
 }
 
 func TestEvaluateFeatures(t *testing.T) {
@@ -3012,6 +3022,80 @@ func TestUpdateFeature(t *testing.T) {
 			},
 			expectedErr: statusInvalidArchive.Err(),
 		},
+		{
+			// Regression test: domain validation errors from the update
+			// transaction must surface as InvalidArgument with structured
+			// details, not as codes.Unknown (HTTP 500).
+			desc: "fail: variation value schema violation returns InvalidArgument",
+			setup: func(s *FeatureService) {
+				vID1 := newUUID(t)
+				vID2 := newUUID(t)
+				s.experimentClient.(*exprclientmock.MockClient).EXPECT().ListExperiments(gomock.Any(), gomock.Any()).Return(
+					&exprproto.ListExperimentsResponse{},
+					nil,
+				)
+				s.environmentClient.(*envclientmock.MockClient).EXPECT().GetEnvironmentV2(
+					gomock.Any(),
+					&envproto.GetEnvironmentV2Request{Id: "eid"},
+				).Return(
+					&envproto.GetEnvironmentV2Response{Environment: &envproto.EnvironmentV2{RequireComment: true}},
+					nil,
+				)
+				// Run the real transaction callback so the domain error
+				// propagates through UpdateFeature's error conversion.
+				s.dbClient.(*databasemock.MockClient).EXPECT().RunInTransactionV2(
+					gomock.Any(), gomock.Any(),
+				).DoAndReturn(func(ctx context.Context, fn func(ctx context.Context) error) error {
+					return fn(ctx)
+				})
+				s.featureStorage.(*mock.MockFeatureStorage).EXPECT().ListFeatures(
+					gomock.Any(), gomock.Any(),
+				).Return([]*featureproto.Feature{
+					{
+						Id:            "fid",
+						VariationType: featureproto.Feature_STRING,
+						Variations: []*featureproto.Variation{
+							{
+								Id:    vID1,
+								Value: "true",
+								Name:  "true",
+							},
+							{
+								Id:    vID2,
+								Value: "false",
+								Name:  "false",
+							},
+						},
+						OffVariation: vID2,
+						DefaultStrategy: &featureproto.Strategy{
+							Type: featureproto.Strategy_FIXED,
+							FixedStrategy: &featureproto.FixedStrategy{
+								Variation: vID1,
+							},
+						},
+						Tags: []string{"test"},
+					},
+				}, 0, int64(0), nil)
+			},
+			ctx: createContextWithToken(),
+			input: &featureproto.UpdateFeatureRequest{
+				EnvironmentId: "eid",
+				Comment:       "comment",
+				Id:            "fid",
+				// The existing variation values (true/false) are not in the
+				// enum, so the schema update must be rejected.
+				VariationValueSchema: &featureproto.VariationValueSchema{
+					Type: featureproto.VariationValueSchema_ENUM,
+					Validator: &featureproto.VariationValueSchema_EnumValidator_{
+						EnumValidator: &featureproto.VariationValueSchema_EnumValidator{
+							Values: []string{"a", "b"},
+						},
+					},
+				},
+			},
+			expectedErr: api.NewGRPCStatus(pkgErr.NewErrorInvalidArgNotMatchFormat(
+				pkgErr.FeaturePackageName, "feature: variation value does not match schema", "VariationValueSchema")).Err(),
+		},
 		{
 			desc: "success",
 			setup: func(s *FeatureService) {

pkg/feature/domain/feature.go

@@ -97,11 +97,11 @@ var (
 	errVariationTypeUnmatched = pkgErr.NewErrorInvalidArgNotMatchFormat(
 		pkgErr.FeaturePackageName, "feature: variation value and type are unmatched", "variation")
 	errVariationValueSchemaInvalid = pkgErr.NewErrorInvalidArgNotMatchFormat(
-		pkgErr.FeaturePackageName, "feature: variation value schema is invalid", "variation_value_schema")
+		pkgErr.FeaturePackageName, "feature: variation value schema is invalid", "VariationValueSchema")
 	errVariationValueSchemaTypeUnmatched = pkgErr.NewErrorInvalidArgNotMatchFormat(
-		pkgErr.FeaturePackageName, "feature: variation value schema and type are unmatched", "variation_value_schema")
+		pkgErr.FeaturePackageName, "feature: variation value schema and type are unmatched", "VariationValueSchema")
 	errVariationValueSchemaViolation = pkgErr.NewErrorInvalidArgNotMatchFormat(
-		pkgErr.FeaturePackageName, "feature: variation value does not match schema", "variation_value_schema")
+		pkgErr.FeaturePackageName, "feature: variation value does not match schema", "VariationValueSchema")
 	errStrategyRequired = pkgErr.NewErrorInvalidArgEmpty(
 		pkgErr.FeaturePackageName, "feature: strategy required", "strategy")
 	errUnsupportedStrategy = pkgErr.NewErrorInvalidArgNotMatchFormat(

ui/dashboard/src/@locales/en/backend.json

@@ -110,6 +110,7 @@
         "TimeRange": "Time range",
         "TrialProject": "Trial project",
         "VariationId": "Variation ID",
+        "VariationValueSchema": "Variation value schema",
         "VariationWeight": "Variation weight",
         "Webhook": "Webhook",
         "WebhookURL": "Webhook URL",

ui/dashboard/src/@locales/ja/backend.json

@@ -128,6 +128,7 @@
         "UserId": "ユーザーID",
         "Variation": "バリエーション",
         "VariationId": "バリエーションID",
+        "VariationValueSchema": "バリエーション値スキーマ",
         "VariationWeight": "バリエーションの重み",
         "Webhook": "Webhook",
         "WebhookURL": "Webhook URL",