From b70c5abce8d2180e63ea163975c80a7798eeb846 Mon Sep 17 00:00:00 2001 From: n0ctal <4c866w5fn9@privaterelay.appleid.com> Date: Fri, 14 Aug 2026 22:41:24 +0500 Subject: [PATCH] fix(outbounds): propagate allocation query failures (#6208) * fix(outbounds): propagate allocation query failures * test(outbounds): cover update allocation failure --------- Co-authored-by: n0ctal <293235942+n0ctal@users.noreply.github.com> --- internal/web/service/outbound_subscription.go | 22 +++-- .../web/service/outbound_subscription_test.go | 92 +++++++++++++++++++ 2 files changed, 108 insertions(+), 6 deletions(-) diff --git a/internal/web/service/outbound_subscription.go b/internal/web/service/outbound_subscription.go index 3136e7bad..b0a5dee8b 100644 --- a/internal/web/service/outbound_subscription.go +++ b/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 diff --git a/internal/web/service/outbound_subscription_test.go b/internal/web/service/outbound_subscription_test.go index 2868c2b78..658b822e2 100644 --- a/internal/web/service/outbound_subscription_test.go +++ b/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))