Sfoglia il codice sorgente

fix(clients): finish deleting from every inbound when one fails

Delete aborted its per-inbound loop on the first error, so a client
attached to inbounds across several nodes lost at most one per attempt:
the loop never reached the remaining nodes, the record cleanup after
the loop never ran, and each retry started over with whatever was left.
Operators with many nodes had to delete the same client once per node.

Collect per-inbound failures and keep going so every reachable inbound
and node is cleaned in a single pass, then keep the client record only
when something failed - its settings JSON still holds the client there,
so the next delete retries exactly the leftovers - and return the
joined failures instead of silently reporting success. DeleteByEmail's
legacy fallback loop gets the same treatment.

Closes #5845
MHSanaei 1 giorno fa
parent
commit
6aa87f4e57

+ 16 - 3
internal/web/service/client_crud.go

@@ -493,12 +493,14 @@ func (s *ClientService) Delete(inboundSvc *InboundService, id int, keepTraffic b
 	}
 
 	needRestart := false
+	var delErrs []error
 	for _, ibId := range inboundIds {
 		if _, getErr := inboundSvc.GetInbound(ibId); getErr != nil {
 			if errors.Is(getErr, gorm.ErrRecordNotFound) {
 				continue
 			}
-			return needRestart, getErr
+			delErrs = append(delErrs, fmt.Errorf("inbound %d: %w", ibId, getErr))
+			continue
 		}
 
 		// Always delete by email — the client's stable identity. This removes
@@ -515,12 +517,18 @@ func (s *ClientService) Delete(inboundSvc *InboundService, id int, keepTraffic b
 			if errors.Is(delErr, ErrClientNotInInbound) {
 				continue
 			}
-			return needRestart, delErr
+			delErrs = append(delErrs, fmt.Errorf("inbound %d: %w", ibId, delErr))
+			continue
 		}
 		if nr {
 			needRestart = true
 		}
 	}
+	// A failed inbound still holds the client in its settings JSON: keep the
+	// record so the next delete retries exactly the leftovers, and report it.
+	if len(delErrs) > 0 {
+		return needRestart, errors.Join(delErrs...)
+	}
 
 	db := database.GetDB()
 	if err := db.Transaction(func(tx *gorm.DB) error {
@@ -668,18 +676,23 @@ func (s *ClientService) DeleteByEmail(inboundSvc *InboundService, email string,
 		return false, common.NewError(fmt.Sprintf("client %q not found in any inbound or client record", email))
 	}
 	needRestart := false
+	var delErrs []error
 	for _, ibId := range inboundIds {
 		nr, delErr := s.DelInboundClientByEmail(inboundSvc, ibId, email, false, true)
 		if delErr != nil {
 			if errors.Is(delErr, ErrClientNotInInbound) {
 				continue
 			}
-			return needRestart, delErr
+			delErrs = append(delErrs, fmt.Errorf("inbound %d: %w", ibId, delErr))
+			continue
 		}
 		if nr {
 			needRestart = true
 		}
 	}
+	if len(delErrs) > 0 {
+		return needRestart, errors.Join(delErrs...)
+	}
 	if !keepTraffic {
 		db := database.GetDB()
 		if err := db.Where("email = ?", email).Delete(&xray.ClientTraffic{}).Error; err != nil {

+ 64 - 0
internal/web/service/client_delete_continue_test.go

@@ -0,0 +1,64 @@
+package service
+
+import (
+	"strconv"
+	"strings"
+	"testing"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/database"
+	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
+)
+
+func TestDeleteContinuesPastFailedInbound(t *testing.T) {
+	setupBulkDB(t)
+	svc := &ClientService{}
+	inboundSvc := &InboundService{}
+
+	source := []model.Client{{Email: "spread@x", ID: "cccccccc-0000-0000-0000-000000000001", SubID: "sub-spread", Enable: true}}
+	ib1 := mkInbound(t, 23001, model.VLESS, clientsSettings(t, source))
+	ib2 := mkInbound(t, 23002, model.VLESS, clientsSettings(t, source))
+	ib3 := mkInbound(t, 23003, model.VLESS, clientsSettings(t, source))
+	for _, ib := range []*model.Inbound{ib1, ib2, ib3} {
+		if err := svc.SyncInbound(nil, ib.Id, source); err != nil {
+			t.Fatalf("seed linkage for %d: %v", ib.Id, err)
+		}
+	}
+	rec := lookupClientRecord(t, "spread@x")
+
+	missingNode := 9999
+	if err := database.GetDB().Model(&model.Inbound{}).Where("id = ?", ib2.Id).
+		Update("node_id", missingNode).Error; err != nil {
+		t.Fatalf("point inbound 2 at a missing node: %v", err)
+	}
+
+	_, err := svc.Delete(inboundSvc, rec.Id, false)
+	if err == nil {
+		t.Fatalf("Delete with a failing inbound succeeded, want error")
+	}
+	if !strings.Contains(err.Error(), "inbound "+strconv.Itoa(ib2.Id)) {
+		t.Fatalf("Delete error = %q, want it to name inbound %d", err, ib2.Id)
+	}
+
+	for _, ib := range []*model.Inbound{ib1, ib3} {
+		if settingsHoldUUID(t, inboundSvc, ib.Id, "spread@x") {
+			t.Fatalf("inbound %d still holds the client after Delete", ib.Id)
+		}
+	}
+	if _, err := svc.GetByID(rec.Id); err != nil {
+		t.Fatalf("record removed despite a failed inbound: %v", err)
+	}
+
+	if err := database.GetDB().Model(&model.Inbound{}).Where("id = ?", ib2.Id).
+		Update("node_id", nil).Error; err != nil {
+		t.Fatalf("repair inbound 2: %v", err)
+	}
+	if _, err := svc.Delete(inboundSvc, rec.Id, false); err != nil {
+		t.Fatalf("retry Delete: %v", err)
+	}
+	if _, err := svc.GetByID(rec.Id); err == nil {
+		t.Fatalf("record still present after successful retry")
+	}
+	if settingsHoldUUID(t, inboundSvc, ib2.Id, "spread@x") {
+		t.Fatalf("inbound 2 still holds the client after retry")
+	}
+}