Solve comments and clean code

Solve comments and clean code

agent_udpmux_test.go

@@ -35,17 +35,11 @@ func TestMuxAgent(t *testing.T) {
 			require.NoError(t, err)
 
 			loggerFactory := logging.NewDefaultLoggerFactory()
-			udpMux, err := NewUDPMuxDefault(UDPMuxParams{
+			udpMux := NewUDPMuxDefault(UDPMuxParams{
 				Logger:  loggerFactory.NewLogger("ice"),
 				UDPConn: c,
 			})
 
-			require.NoError(t, err)
-			defer func() {
-				_ = udpMux.Close()
-				_ = c.Close()
-			}()
-
 			muxedA, err := NewAgent(&AgentConfig{
 				UDPMux:         udpMux,
 				CandidateTypes: []CandidateType{CandidateTypeHost},

errors.go

@@ -140,6 +140,9 @@ var (
 	errNotImplemented                = errors.New("not implemented yet")
 	errNoUDPMuxAvailable             = errors.New("no UDP mux is available")
 	errNoTCPMuxAvailable             = errors.New("no TCP mux is available")
+	errInvalidAddress                = errors.New("invalid address")
+
+	// UDPMuxDefault should not listen on unspecified address, but to keep backward compatibility, don't return error now.
+	// will be used in the future.
 	// errListenUnspecified             = errors.New("can't listen on unspecified address")
-	errInvalidAddress = errors.New("invalid address")
 )

gather_test.go

@@ -507,8 +507,7 @@ func TestMultiUDPMuxUsage(t *testing.T) {
 		}()
 
 		expectedPorts = append(expectedPorts, port)
-		muxDefault, err := NewUDPMuxDefault(UDPMuxParams{UDPConn: conn})
-		assert.NoError(t, err)
+		muxDefault := NewUDPMuxDefault(UDPMuxParams{UDPConn: conn})
 		udpMuxInstances = append(udpMuxInstances, muxDefault)
 		idx := i
 		defer func() {

tcp_mux_multi.go

@@ -6,7 +6,7 @@ package ice
 import "net"
 
 // AllConnsGetter allows multiple fixed TCP ports to be used,
-// each which is multiplexed like TCPMux. AllConnsGetter also acts as
+// each of which is multiplexed like TCPMux. AllConnsGetter also acts as
 // a TCPMux, in which case it will return a single connection for one
 // of the ports.
 type AllConnsGetter interface {

udp_mux.go

@@ -52,16 +52,15 @@ type UDPMuxParams struct {
 }
 
 // NewUDPMuxDefault creates an implementation of UDPMux
-func NewUDPMuxDefault(params UDPMuxParams) (*UDPMuxDefault, error) {
+func NewUDPMuxDefault(params UDPMuxParams) *UDPMuxDefault {
 	if params.Logger == nil {
 		params.Logger = logging.NewDefaultLoggerFactory().NewLogger("ice")
 	}
 
 	var localAddrsForUnspecified []net.Addr
 	if addr, ok := params.UDPConn.LocalAddr().(*net.UDPAddr); !ok {
-		return nil, errInvalidAddress
+		params.Logger.Errorf("LocalAddr is not a net.UDPAddr, got %T", params.UDPConn.LocalAddr())
 	} else if ok && addr.IP.IsUnspecified() {
-		// return nil, errListenUnspecified
 		// For unspecified addresses, the correct behavior is to return errListenUnspecified, but
 		// it will break the applications that are already using unspecified UDP connection
 		// with UDPMuxDefault, so print a warn log and create a local address list for mux.
@@ -75,15 +74,18 @@ func NewUDPMuxDefault(params UDPMuxParams) (*UDPMuxDefault, error) {
 			networks = []NetworkType{NetworkTypeUDP4, NetworkTypeUDP6}
 
 		default:
-			return nil, errInvalidAddress
+			params.Logger.Errorf("LocalAddr expected IPV4 or IPV6, got %T", params.UDPConn.LocalAddr())
 		}
-		muxNet := vnet.NewNet(nil)
-		ips, err := localInterfaces(muxNet, nil, nil, networks)
-		if err != nil {
-			return nil, err
-		}
-		for _, ip := range ips {
-			localAddrsForUnspecified = append(localAddrsForUnspecified, &net.UDPAddr{IP: ip, Port: addr.Port})
+		if len(networks) > 0 {
+			muxNet := vnet.NewNet(nil)
+			ips, err := localInterfaces(muxNet, nil, nil, networks)
+			if err == nil {
+				for _, ip := range ips {
+					localAddrsForUnspecified = append(localAddrsForUnspecified, &net.UDPAddr{IP: ip, Port: addr.Port})
+				}
+			} else {
+				params.Logger.Errorf("failed to get local interfaces for unspecified addr: %v", err)
+			}
 		}
 	}
 
@@ -103,7 +105,7 @@ func NewUDPMuxDefault(params UDPMuxParams) (*UDPMuxDefault, error) {
 
 	go m.connWorker()
 
-	return m, nil
+	return m
 }
 
 // LocalAddr returns the listening address of this UDPMuxDefault

udp_mux_multi.go

@@ -111,21 +111,10 @@ func NewMultiUDPMuxFromPort(port int, opts ...UDPMuxFromPortOption) (*MultiUDPMu
 
 	muxs := make([]UDPMux, 0, len(conns))
 	for _, conn := range conns {
-		mux, muxErr := NewUDPMuxDefault(UDPMuxParams{Logger: params.logger, UDPConn: conn})
-		if muxErr != nil {
-			err = muxErr
-			break
-		}
+		mux := NewUDPMuxDefault(UDPMuxParams{Logger: params.logger, UDPConn: conn})
 		muxs = append(muxs, mux)
 	}
 
-	if err != nil {
-		for _, mux := range muxs {
-			_ = mux.Close()
-		}
-		return nil, err
-	}
-
 	return NewMultiUDPMuxDefault(muxs...), nil
 }
 

udp_mux_multi_test.go

@@ -34,15 +34,12 @@ func TestMultiUDPMux(t *testing.T) {
 	}
 
 	muxes := []UDPMux{}
-	muxV41, err := NewUDPMuxDefault(UDPMuxParams{UDPConn: conn1})
-	require.NoError(t, err)
+	muxV41 := NewUDPMuxDefault(UDPMuxParams{UDPConn: conn1})
 	muxes = append(muxes, muxV41)
-	muxV42, err := NewUDPMuxDefault(UDPMuxParams{UDPConn: conn2})
-	require.NoError(t, err)
+	muxV42 := NewUDPMuxDefault(UDPMuxParams{UDPConn: conn2})
 	muxes = append(muxes, muxV42)
 	if conn3 != nil {
-		muxV6, v6err := NewUDPMuxDefault(UDPMuxParams{UDPConn: conn3})
-		require.NoError(t, v6err)
+		muxV6 := NewUDPMuxDefault(UDPMuxParams{UDPConn: conn3})
 		muxes = append(muxes, muxV6)
 	}
 

udp_mux_test.go

@@ -62,13 +62,11 @@ func TestUDPMux(t *testing.T) {
 			continue
 		}
 		t.Run(subTest.name, func(t *testing.T) {
-			udpMux, err := NewUDPMuxDefault(UDPMuxParams{
+			udpMux := NewUDPMuxDefault(UDPMuxParams{
 				Logger:  nil,
 				UDPConn: conn,
 			})
 
-			require.NoError(t, err)
-
 			defer func() {
 				_ = udpMux.Close()
 				_ = conn.Close()

udp_mux_universal.go

@@ -38,7 +38,7 @@ type UniversalUDPMuxParams struct {
 }
 
 // NewUniversalUDPMuxDefault creates an implementation of UniversalUDPMux embedding UDPMux
-func NewUniversalUDPMuxDefault(params UniversalUDPMuxParams) (*UniversalUDPMuxDefault, error) {
+func NewUniversalUDPMuxDefault(params UniversalUDPMuxParams) *UniversalUDPMuxDefault {
 	if params.Logger == nil {
 		params.Logger = logging.NewDefaultLoggerFactory().NewLogger("ice")
 	}
@@ -64,13 +64,9 @@ func NewUniversalUDPMuxDefault(params UniversalUDPMuxParams) (*UniversalUDPMuxDe
 		Logger:  params.Logger,
 		UDPConn: m.params.UDPConn,
 	}
-	muxDefault, err := NewUDPMuxDefault(udpMuxParams)
-	if err != nil {
-		return nil, err
-	}
-	m.UDPMuxDefault = muxDefault
+	m.UDPMuxDefault = NewUDPMuxDefault(udpMuxParams)
 
-	return m, nil
+	return m
 }
 
 // udpConn is a wrapper around UDPMux conn that overrides ReadFrom and handles STUN/TURN packets
@@ -107,7 +103,8 @@ func (c *udpConn) ReadFrom(p []byte) (n int, addr net.Addr, err error) {
 
 		if err = msg.Decode(); err != nil {
 			c.logger.Warnf("Failed to handle decode ICE from %s: %v", addr.String(), err)
-			return n, addr, nil
+			err = nil
+			return
 		}
 
 		udpAddr, ok := addr.(*net.UDPAddr)
@@ -120,12 +117,12 @@ func (c *udpConn) ReadFrom(p []byte) (n int, addr net.Addr, err error) {
 			err = c.mux.handleXORMappedResponse(udpAddr, msg)
 			if err != nil {
 				c.logger.Debugf("%w: %v", errGetXorMappedAddrResponse, err)
-				return n, addr, nil
+				err = nil
 			}
 			return
 		}
 	}
-	return n, addr, err
+	return
 }
 
 // isXORMappedResponse indicates whether the message is a XORMappedAddress and is coming from the known STUN server.

udp_mux_universal_test.go

@@ -17,12 +17,11 @@ func TestUniversalUDPMux(t *testing.T) {
 	conn, err := net.ListenUDP(udp, &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1)})
 	require.NoError(t, err)
 
-	udpMux, err := NewUniversalUDPMuxDefault(UniversalUDPMuxParams{
+	udpMux := NewUniversalUDPMuxDefault(UniversalUDPMuxParams{
 		Logger:  nil,
 		UDPConn: conn,
 	})
 
-	require.NoError(t, err)
 	defer func() {
 		_ = udpMux.Close()
 		_ = conn.Close()