fix(outbounds): propagate allocation query failures (#6208)

* fix(outbounds): propagate allocation query failures

* test(outbounds): cover update allocation failure

---------

Co-authored-by: n0ctal <[email protected]>

internal/web/service/outbound_subscription.go

@@ -156,10 +156,12 @@ func defaultPrefixNumber(subs []*model.OutboundSubscription, excludeId int) int
 // nextDefaultSubPrefix builds the default "subN-" prefix for a new/edited
 // subscription, picking the smallest free N (excludeId skips a subscription's
 // own current prefix when editing).
-func (s *OutboundSubscriptionService) nextDefaultSubPrefix(excludeId int) string {
+func (s *OutboundSubscriptionService) nextDefaultSubPrefix(excludeId int) (string, error) {
 	var subs []*model.OutboundSubscription
-	_ = database.GetDB().Find(&subs).Error
-	return fmt.Sprintf("sub%d-", defaultPrefixNumber(subs, excludeId))
+	if err := database.GetDB().Find(&subs).Error; err != nil {
+		return "", err
+	}
+	return fmt.Sprintf("sub%d-", defaultPrefixNumber(subs, excludeId)), nil
 }
 
 func (s *OutboundSubscriptionService) Create(remark, rawURL, tagPrefix string, enabled bool, updateInterval int, allowPrivate, prepend, allowInsecure bool) (*model.OutboundSubscription, error) {
@@ -175,11 +177,16 @@ func (s *OutboundSubscriptionService) Create(remark, rawURL, tagPrefix string, e
 	}
 	prefix := strings.TrimSpace(tagPrefix)
 	if prefix == "" {
-		prefix = s.nextDefaultSubPrefix(0)
+		prefix, err = s.nextDefaultSubPrefix(0)
+		if err != nil {
+			return nil, err
+		}
 	}
 	// New subscriptions go to the end of the priority order.
 	var count int64
-	database.GetDB().Model(&model.OutboundSubscription{}).Count(&count)
+	if err := database.GetDB().Model(&model.OutboundSubscription{}).Count(&count).Error; err != nil {
+		return nil, err
+	}
 	sub := &model.OutboundSubscription{
 		Remark:         strings.TrimSpace(remark),
 		Url:            cleanURL,
@@ -215,7 +222,10 @@ func (s *OutboundSubscriptionService) Update(id int, remark, rawURL, tagPrefix s
 	}
 	prefix := strings.TrimSpace(tagPrefix)
 	if prefix == "" {
-		prefix = s.nextDefaultSubPrefix(sub.Id)
+		prefix, err = s.nextDefaultSubPrefix(sub.Id)
+		if err != nil {
+			return err
+		}
 	}
 	sub.Remark = strings.TrimSpace(remark)
 	sub.Url = cleanURL

internal/web/service/outbound_subscription_test.go

@@ -5,10 +5,102 @@ import (
 	"errors"
 	"testing"
 
+	"gorm.io/gorm"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/database"
 	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
 	"github.com/mhsanaei/3x-ui/v3/internal/util/link"
 )
 
+func TestOutboundSubscriptionCreatePropagatesAllocationDatabaseFailures(t *testing.T) {
+	setupSettingTestDB(t)
+	db := database.GetDB()
+	const callback = "test:fail_outbound_subscription_query"
+	errInjected := errors.New("injected outbound subscription query failure")
+	if err := db.Callback().Query().Before("gorm:query").Register(callback, func(tx *gorm.DB) {
+		if tx.Statement != nil && tx.Statement.Table == "outbound_subscriptions" {
+			tx.AddError(errInjected)
+		}
+	}); err != nil {
+		t.Fatalf("register query callback: %v", err)
+	}
+	t.Cleanup(func() {
+		if err := db.Callback().Query().Remove(callback); err != nil {
+			t.Errorf("remove query callback: %v", err)
+		}
+	})
+
+	for _, tc := range []struct {
+		name      string
+		tagPrefix string
+		operation string
+	}{
+		{name: "default prefix query", tagPrefix: "", operation: "prefix allocation"},
+		{name: "priority count query", tagPrefix: "custom-", operation: "priority allocation"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			created, err := (&OutboundSubscriptionService{}).Create("test", "https://1.1.1.1/sub", tc.tagPrefix, true, 600, false, false, false)
+			if !errors.Is(err, errInjected) {
+				t.Fatalf("Create error = %v, want injected %s query failure", err, tc.operation)
+			}
+			if created != nil {
+				t.Fatalf("Create returned row %+v after %s query failure", created, tc.operation)
+			}
+		})
+	}
+}
+
+func TestOutboundSubscriptionUpdatePropagatesPrefixQueryFailureWithoutMutation(t *testing.T) {
+	setupSettingTestDB(t)
+	db := database.GetDB()
+	original := &model.OutboundSubscription{
+		Remark: "before", Url: "https://1.1.1.1/original", TagPrefix: "custom-",
+		Enabled: true, UpdateInterval: 600,
+	}
+	if err := db.Create(original).Error; err != nil {
+		t.Fatalf("seed subscription: %v", err)
+	}
+
+	errInjected := errors.New("injected update prefix query failure")
+	queryCount := 0
+	const callback = "test:fail_update_prefix_query"
+	if err := db.Callback().Query().Before("gorm:query").Register(callback, func(tx *gorm.DB) {
+		if tx.Statement == nil || tx.Statement.Table != "outbound_subscriptions" {
+			return
+		}
+		queryCount++
+		if queryCount == 2 {
+			tx.AddError(errInjected)
+		}
+	}); err != nil {
+		t.Fatalf("register query callback: %v", err)
+	}
+	t.Cleanup(func() {
+		if err := db.Callback().Query().Remove(callback); err != nil {
+			t.Errorf("remove query callback: %v", err)
+		}
+	})
+
+	err := (&OutboundSubscriptionService{}).Update(
+		original.Id, "after", "https://1.1.1.1/changed", "", false, 1200, false, false, false,
+	)
+	if !errors.Is(err, errInjected) {
+		t.Fatalf("Update error = %v, want injected prefix query failure", err)
+	}
+	if queryCount != 2 {
+		t.Fatalf("outbound subscription queries = %d, want Get plus prefix allocation", queryCount)
+	}
+
+	var got model.OutboundSubscription
+	if err := db.First(&got, original.Id).Error; err != nil {
+		t.Fatalf("reload subscription: %v", err)
+	}
+	if got.Remark != original.Remark || got.Url != original.Url || got.TagPrefix != original.TagPrefix ||
+		got.Enabled != original.Enabled || got.UpdateInterval != original.UpdateInterval {
+		t.Fatalf("subscription changed after failed allocation: got %+v, want %+v", got, *original)
+	}
+}
+
 func TestReadBoundedOutboundSubscriptionBody(t *testing.T) {
 	t.Run("accepts body at the limit", func(t *testing.T) {
 		want := bytes.Repeat([]byte("a"), int(maxOutboundSubscriptionBytes))