From 669323f926f00d6fa26e65c23ac97fb5dd6bd658 Mon Sep 17 00:00:00 2001 From: sagitchu Date: Sat, 7 Mar 2026 17:19:28 +0800 Subject: [PATCH] fix: improve bind-conflict detection and forward cleanup reliability - Normalize whitespace in error messages to handle collapsed variants (e.g., 'address alreadyin use') - Delete all forward service name variants (_tcp, _udp, base) during cleanup instead of stopping after first success - Add comprehensive test coverage for edge cases --- .../internal/http/handler/control_plane.go | 43 +++++++++++++------ .../http/handler/control_plane_test.go | 33 ++++++++++++++ .../tests/contract/forward_contract_test.go | 2 +- .../016-tunnel-runtime-bind-conflict-retry.md | 4 ++ 4 files changed, 68 insertions(+), 14 deletions(-) diff --git a/go-backend/internal/http/handler/control_plane.go b/go-backend/internal/http/handler/control_plane.go index 2b4b144..82942d7 100644 --- a/go-backend/internal/http/handler/control_plane.go +++ b/go-backend/internal/http/handler/control_plane.go @@ -527,15 +527,17 @@ func controlForwardServiceCommand(bases []string, commandType string, send func( } func deleteForwardServiceCandidates(bases []string, send func(name string) error) error { - handled, lastNotFoundErr, err := controlForwardServiceCommand(bases, "DeleteService", send) - if err != nil { - return err - } - if handled { - return nil - } - if lastNotFoundErr != nil { - return nil + for _, base := range bases { + for _, name := range append([]string{base + "_tcp", base + "_udp", base}, []string{}...) { + err := send(name) + if err == nil { + continue + } + if isNotFoundError(err) { + continue + } + return err + } } return nil } @@ -1477,10 +1479,11 @@ func isAlreadyExistsMessage(message string) bool { if msg == "" { return false } - if strings.Contains(msg, "address already in use") { + if isAddressAlreadyInUseMessage(msg) { return false } - return strings.Contains(msg, "already exists") || strings.Contains(msg, "已存在") + compact := compactErrorMessage(msg) + return strings.Contains(msg, "already exists") || strings.Contains(msg, "已存在") || strings.Contains(compact, "alreadyexists") } func isBindAddressInUseError(err error) bool { @@ -1505,7 +1508,10 @@ func isAddressAlreadyInUseMessage(msg string) bool { if msg == "" { return false } - return strings.Contains(msg, "address already in use") + if strings.Contains(msg, "address already in use") { + return true + } + return strings.Contains(compactErrorMessage(msg), "addressalreadyinuse") } func isCannotAssignRequestedAddressError(err error) bool { @@ -1516,7 +1522,18 @@ func isCannotAssignRequestedAddressError(err error) bool { if msg == "" { return false } - return strings.Contains(msg, "cannot assign requested address") + if strings.Contains(msg, "cannot assign requested address") { + return true + } + return strings.Contains(compactErrorMessage(msg), "cannotassignrequestedaddress") +} + +func compactErrorMessage(msg string) string { + msg = strings.TrimSpace(msg) + if msg == "" { + return "" + } + return strings.Join(strings.Fields(strings.ToLower(msg)), "") } func buildForwardServiceConfigs(baseName string, forward *forwardRecord, tunnel *tunnelRecord, node *nodeRecord, port int, bindIP string, limiterID *int64, tunnelTLSProtocol bool) []map[string]interface{} { diff --git a/go-backend/internal/http/handler/control_plane_test.go b/go-backend/internal/http/handler/control_plane_test.go index 55a6da5..a693e0e 100644 --- a/go-backend/internal/http/handler/control_plane_test.go +++ b/go-backend/internal/http/handler/control_plane_test.go @@ -180,6 +180,27 @@ func TestDeleteForwardServiceBasesOnNodeRetriesLegacyZeroResidue(t *testing.T) { } } +func TestDeleteForwardServiceCandidatesDeletesAllMatchingVariants(t *testing.T) { + bases := []string{"57_7_7", "57_7_0"} + called := make([]string, 0) + err := deleteForwardServiceCandidates(bases, func(name string) error { + called = append(called, name) + switch name { + case "57_7_7_tcp", "57_7_7_udp", "57_7_0_tcp", "57_7_0_udp": + return nil + default: + return errors.New("service " + name + " not found") + } + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := []string{"57_7_7_tcp", "57_7_7_udp", "57_7_7", "57_7_0_tcp", "57_7_0_udp", "57_7_0"} + if !reflect.DeepEqual(called, want) { + t.Fatalf("expected calls %v, got %v", want, called) + } +} + func TestValidateForwardPortAvailabilityRejectsOtherForwardOccupancy(t *testing.T) { h := &Handler{repo: nil} node := &nodeRecord{ID: 9, Name: "test-node"} @@ -235,9 +256,15 @@ func TestIsAlreadyExistsMessage(t *testing.T) { if !isAlreadyExistsMessage("服务已存在") { t.Fatalf("expected Chinese already exists message to be tolerated") } + if !isAlreadyExistsMessage("service demo alreadyexists") { + t.Fatalf("missing-space alreadyexists should be tolerated") + } if isAlreadyExistsMessage("listen tcp [::]:10001: bind: address already in use") { t.Fatalf("address already in use must not be treated as already exists") } + if isAlreadyExistsMessage("create service 57_7_7_tcp failed: listen tcp4 0.0.0.0:46222: bind: address alreadyin use") { + t.Fatalf("alreadyin-use variant must not be treated as already exists") + } } func TestIsBindAddressInUseError(t *testing.T) { @@ -259,6 +286,9 @@ func TestIsAddressAlreadyInUseError(t *testing.T) { if !isAddressAlreadyInUseError(errors.New("listen tcp [::]:10001: bind: address already in use")) { t.Fatalf("address already in use should be detected") } + if !isAddressAlreadyInUseError(errors.New("create service 57_7_7_tcp failed: listen tcp4 0.0.0.0:46222: bind: address alreadyin use")) { + t.Fatalf("missing-space alreadyin-use variant should be detected") + } if isAddressAlreadyInUseError(errors.New("listen tcp4 13.228.170.187:16765: bind: cannot assign requested address")) { t.Fatalf("cannot assign requested address should not be treated as address-in-use") } @@ -268,6 +298,9 @@ func TestIsCannotAssignRequestedAddressError(t *testing.T) { if !isCannotAssignRequestedAddressError(errors.New("listen tcp4 13.228.170.187:16765: bind: cannot assign requested address")) { t.Fatalf("cannot assign requested address should be detected") } + if !isCannotAssignRequestedAddressError(errors.New("listen tcp4 13.228.170.187:16765: bind: cannotassignrequestedaddress")) { + t.Fatalf("missing-space cannotassignrequestedaddress variant should be detected") + } if isCannotAssignRequestedAddressError(errors.New("listen tcp [::]:10001: bind: address already in use")) { t.Fatalf("address already in use should not be treated as cannot-assign") } diff --git a/go-backend/tests/contract/forward_contract_test.go b/go-backend/tests/contract/forward_contract_test.go index c2dd0de..ac23b92 100644 --- a/go-backend/tests/contract/forward_contract_test.go +++ b/go-backend/tests/contract/forward_contract_test.go @@ -1006,7 +1006,7 @@ func TestForwardUpdateRecoversFromAddressInUseContract(t *testing.T) { } mu.Unlock() if shouldFail { - return true, "listen tcp 10.42.0.9:44001: bind: address already in use" + return true, "create service 57_7_7_tcp failed: listen tcp4 0.0.0.0:46222: bind: address alreadyin use" } return false, "" }) diff --git a/plans/016-tunnel-runtime-bind-conflict-retry.md b/plans/016-tunnel-runtime-bind-conflict-retry.md index b737a00..f1e6100 100644 --- a/plans/016-tunnel-runtime-bind-conflict-retry.md +++ b/plans/016-tunnel-runtime-bind-conflict-retry.md @@ -20,8 +20,12 @@ - Result: passed. - Command: `cd go-backend && go test ./tests/contract/... -run 'TestForwardUpdateRecoversFromAddressInUseContract|TestTunnelUpdateRecoversFromAddressInUseContract'` - Result: passed. +- Command: `cd go-backend && go test ./internal/http/handler/... && go test ./tests/contract/... -run 'TestForwardUpdateRecoversFromAddressInUseContract|TestTunnelUpdateRecoversFromAddressInUseContract'` +- Result: passed. ## Investigation Note - Forward update still has its own independent `address already in use` recovery path in `syncForwardServicesWithWarnings` / `rebindForwardServiceOnSelfOccupiedPort`; tunnel update linkage is not the only possible source of the symptom. - Tunnel update also triggers downstream forward `UpdateService` for bound forwards, so users can still observe the same error around a tunnel edit even when the failing runtime is on the tunnel side. +- Real node output can collapse spaces into variants like `address alreadyin use` / `cannotassignrequestedaddress`; bind-conflict detection now normalizes whitespace before classifying the error. +- Forward self-heal cleanup now deletes every candidate runtime name variant instead of stopping after the first successful delete, which avoids leaving sibling `_tcp`/`_udp` services behind to keep the port occupied.