THRIFT-5453: Defer DNS from NewTSocketConf to TSocket.Open

Client: go

We used to do DNS lookups in NewTSocketConf, without any timeout checks.
Stop doing that and do DNS lookups in TSocket.Open instead, which
already checks for ConnectTimeout set in TConfiguration.

Also remove the error return from NewTSocketConf.

7 files changed, 44 insertions, 31 deletions

diff --git a/CHANGES.md b/CHANGES.md
index a03204ba7..bfd8cb090 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -15,6 +15,7 @@
 
 - [THRIFT-5381](https://issues.apache.org/jira/browse/THRIFT-5381) - possible collisions at VOID type with some 3rd-party libraries on Haxe cpp targets
 - [THRIFT-5396](https://issues.apache.org/jira/browse/THRIFT-5396) - deprecate netstd "Async" method postfix
+- [THRIFT-5453](https://issues.apache.org/jira/browse/THRIFT-5453) - go: NewTSocketConf no longer returns an error
 
 ### AS3
 
@@ -78,6 +79,8 @@
 - [THRIFT-5369](https://issues.apache.org/jira/browse/THRIFT-5369) - Malformed payload can still cause huge allocations
 - [THRIFT-5389](https://issues.apache.org/jira/browse/THRIFT-5389) - The compiler now generates correct go code with thrift constant defined on optional enum/typedef fields
 - [THRIFT-5404](https://issues.apache.org/jira/browse/THRIFT-5404) - TTransportException.Timeout would correctly return true when it's connect timeout during TSocket.Open call
+- [THRIFT-4797](https://issues.apache.org/jira/browse/THRIFT-4797) - The compiler now correctly auto renames import thrift namespaces when they collide with system imports
+- [THRIFT-5453](https://issues.apache.org/jira/browse/THRIFT-5453) - Defer DNS lookups from NewTSocketConf (without any timeout check) to TSocket.Open (subject to ConnectTimeout set in TConfiguration)
 
 ### Haskell
 
diff --git a/go.sum b/go.sum
index 646b11a85..cfde58466 100644
--- a/go.sum
+++ b/go.sum
@@ -2,6 +2,7 @@ github.com/golang/mock v1.5.0 h1:jlYHihg//f7RRwuPfptm04yp4s7O6Kw8EZiVYIGcH0g=
 github.com/golang/mock v1.5.0/go.mod h1:CWnOUgYIOo4TcNZ0wHX3YZCqsaM1I1Jvs6v3mP3KVu8=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
+golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=
 golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
 golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
@@ -9,6 +10,8 @@ golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJ
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
+golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e h1:aZzprAO9/8oim3qStq3wc1Xuxx4QmAGriC4VU4ojemQ=
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898 h1:/atklqdjdhuosWIl6AIbOeHJjicWYPqR9bpxqxYG2pA=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
diff --git a/lib/go/thrift/configuration.go b/lib/go/thrift/configuration.go
index 454d9f377..de27edd67 100644
--- a/lib/go/thrift/configuration.go
+++ b/lib/go/thrift/configuration.go
@@ -56,7 +56,7 @@ const (
 //
 // For example, say you want to migrate this old code into using TConfiguration:
 //
-//     sccket := thrift.NewTSocketTimeout("host:port", time.Second)
+//     sccket, err := thrift.NewTSocketTimeout("host:port", time.Second, time.Second)
 //     transFactory := thrift.NewTFramedTransportFactoryMaxLength(
 //         thrift.NewTTransportFactory(),
 //         1024 * 1024 * 256,
diff --git a/lib/go/thrift/socket.go b/lib/go/thrift/socket.go
index 0cf59a0f5..eeac4f1a4 100644
--- a/lib/go/thrift/socket.go
+++ b/lib/go/thrift/socket.go
@@ -29,16 +29,26 @@ type TSocket struct {
 	conn *socketConn
 	addr net.Addr
 	cfg  *TConfiguration
+}
+
+// tcpAddr is a naive implementation of net.Addr that does nothing extra.
+type tcpAddr string
+
+var _ net.Addr = tcpAddr("")
+
+func (ta tcpAddr) Network() string {
+	return "tcp"
+}
 
-	connectTimeout time.Duration
-	socketTimeout  time.Duration
+func (ta tcpAddr) String() string {
+	return string(ta)
 }
 
 // Deprecated: Use NewTSocketConf instead.
 func NewTSocket(hostPort string) (*TSocket, error) {
 	return NewTSocketConf(hostPort, &TConfiguration{
 		noPropagation: true,
-	})
+	}), nil
 }
 
 // NewTSocketConf creates a net.Conn-backed TTransport, given a host and port.
@@ -49,12 +59,8 @@ func NewTSocket(hostPort string) (*TSocket, error) {
 //         ConnectTimeout: time.Second, // Use 0 for no timeout
 //         SocketTimeout:  time.Second, // Use 0 for no timeout
 //     })
-func NewTSocketConf(hostPort string, conf *TConfiguration) (*TSocket, error) {
-	addr, err := net.ResolveTCPAddr("tcp", hostPort)
-	if err != nil {
-		return nil, err
-	}
-	return NewTSocketFromAddrConf(addr, conf), nil
+func NewTSocketConf(hostPort string, conf *TConfiguration) *TSocket {
+	return NewTSocketFromAddrConf(tcpAddr(hostPort), conf)
 }
 
 // Deprecated: Use NewTSocketConf instead.
@@ -64,7 +70,7 @@ func NewTSocketTimeout(hostPort string, connTimeout time.Duration, soTimeout tim
 		SocketTimeout:  soTimeout,
 
 		noPropagation: true,
-	})
+	}), nil
 }
 
 // NewTSocketFromAddrConf creates a TSocket from a net.Addr
@@ -172,6 +178,7 @@ func (p *TSocket) Open() error {
 			msg:    err.Error(),
 		}
 	}
+	p.addr = p.conn.RemoteAddr()
 	return nil
 }
 
diff --git a/test/go/genmock.sh b/test/go/genmock.sh
index bccfdf351..27cd0c43e 100644
--- a/test/go/genmock.sh
+++ b/test/go/genmock.sh
@@ -9,4 +9,4 @@ GO111MODULE=on go install -mod=mod github.com/golang/mock/mockgen
 
 `go env GOPATH`/bin/mockgen -build_flags "-mod=mod" -destination=src/common/mock_handler.go -package=common github.com/apache/thrift/test/go/src/gen/thrifttest ThriftTest
 
-rm -Rf $GOPATH
+chmod a+w -R $GOPATH && rm -Rf $GOPATH
diff --git a/test/go/src/bin/stress/main.go b/test/go/src/bin/stress/main.go
index 3ff0a3969..9f3267654 100644
--- a/test/go/src/bin/stress/main.go
+++ b/test/go/src/bin/stress/main.go
@@ -158,13 +158,11 @@ func main() {
 }
 
 func client(protocolFactory thrift.TProtocolFactory) {
-	trans, err := thrift.NewTSocket(hostPort)
-	if err != nil {
-		log.Fatalf("Unable to create server socket: %s", err)
-	}
+	ctx := context.Background()
+	trans := thrift.NewTSocketConf(hostPort, nil)
 	btrans := thrift.NewTBufferedTransport(trans, 2048)
 	client := stress.NewServiceClientFactory(btrans, protocolFactory)
-	err = trans.Open()
+	err := trans.Open()
 	if err != nil {
 		log.Fatalf("Unable to open connection: %s", err)
 	}
@@ -173,45 +171,45 @@ func client(protocolFactory thrift.TProtocolFactory) {
 	switch callType {
 	case echoVoid:
 		for i := 0; i < *loop; i++ {
-			client.EchoVoid()
+			client.EchoVoid(ctx)
 			atomic.AddInt64(&clicounter, 1)
 		}
 	case echoByte:
 		for i := 0; i < *loop; i++ {
-			client.EchoByte(42)
+			client.EchoByte(ctx, 42)
 			atomic.AddInt64(&clicounter, 1)
 		}
 	case echoI32:
 		for i := 0; i < *loop; i++ {
-			client.EchoI32(4242)
+			client.EchoI32(ctx, 4242)
 			atomic.AddInt64(&clicounter, 1)
 		}
 	case echoI64:
 		for i := 0; i < *loop; i++ {
-			client.EchoI64(424242)
+			client.EchoI64(ctx, 424242)
 			atomic.AddInt64(&clicounter, 1)
 		}
 	case echoString:
 		for i := 0; i < *loop; i++ {
-			client.EchoString("TestString")
+			client.EchoString(ctx, "TestString")
 			atomic.AddInt64(&clicounter, 1)
 		}
 	case echiList:
 		l := []int8{-10, -9, -8, -7, -6, -5, -4, -3, -2, -1, 0, 1, 2, 3, 4, 5, 6, 7, 8}
 		for i := 0; i < *loop; i++ {
-			client.EchoList(l)
+			client.EchoList(ctx, l)
 			atomic.AddInt64(&clicounter, 1)
 		}
 	case echoSet:
-		s := map[int8]struct{}{-10: {}, -9: {}, -8: {}, -7: {}, -6: {}, -5: {}, -4: {}, -3: {}, -2: {}, -1: {}, 0: {}, 1: {}, 2: {}, 3: {}, 4: {}, 5: {}, 6: {}, 7: {}, 8: {}}
+		s := []int8{-10, -9, -8, -7, -6, -5, -4, -3, -2, -1, 0, 1, 2, 3, 4, 5, 6, 7, 8}
 		for i := 0; i < *loop; i++ {
-			client.EchoSet(s)
+			client.EchoSet(ctx, s)
 			atomic.AddInt64(&clicounter, 1)
 		}
 	case echoMap:
 		m := map[int8]int8{-10: 10, -9: 9, -8: 8, -7: 7, -6: 6, -5: 5, -4: 4, -3: 3, -2: 2, -1: 1, 0: 0, 1: 1, 2: 2, 3: 3, 4: 4, 5: 5, 6: 6, 7: 7, 8: 8}
 		for i := 0; i < *loop; i++ {
-			client.EchoMap(m)
+			client.EchoMap(ctx, m)
 			atomic.AddInt64(&clicounter, 1)
 		}
 	}
@@ -245,7 +243,7 @@ func (h *handler) EchoList(ctx context.Context, arg []int8) (r []int8, err error
 	atomic.AddInt64(&counter, 1)
 	return arg, nil
 }
-func (h *handler) EchoSet(ctx context.Context, arg map[int8]struct{}) (r map[int8]struct{}, err error) {
+func (h *handler) EchoSet(ctx context.Context, arg []int8) (r []int8, err error) {
 	atomic.AddInt64(&counter, 1)
 	return arg, nil
 }
diff --git a/test/go/src/common/client.go b/test/go/src/common/client.go
index 15973d82c..201503538 100644
--- a/test/go/src/common/client.go
+++ b/test/go/src/common/client.go
@@ -62,15 +62,17 @@ func StartClient(
 		return nil, nil, fmt.Errorf("Invalid protocol specified %s", protocol)
 	}
 	if debugClientProtocol {
-		protocolFactory = thrift.NewTDebugProtocolFactory(protocolFactory, "client:")
+		protocolFactory = thrift.NewTDebugProtocolFactoryWithLogger(protocolFactory, "client:", thrift.StdLogger(nil))
 	}
 	if ssl {
-		trans, err = thrift.NewTSSLSocket(hostPort, &tls.Config{InsecureSkipVerify: true})
+		trans, err = thrift.NewTSSLSocketConf(hostPort, &thrift.TConfiguration{
+			TLSConfig: &tls.Config{InsecureSkipVerify: true},
+		})
 	} else {
 		if domain_socket != "" {
-			trans, err = thrift.NewTSocket(domain_socket)
+			trans = thrift.NewTSocketConf(domain_socket, nil)
 		} else {
-			trans, err = thrift.NewTSocket(hostPort)
+			trans = thrift.NewTSocketConf(hostPort, nil)
 		}
 	}
 	if err != nil {