fix(gclient): add timeout configuration methods and improve client initialization (#4779)

net/gclient/gclient.go

@@ -11,7 +11,6 @@ import (
 	"crypto/rand"
 	"crypto/tls"
 	"fmt"
-	"net"
 	"net/http"
 	"os"
 	"time"
@@ -58,31 +57,56 @@ var (
 	defaultClientAgent = fmt.Sprintf(`GClient %s at %s`, gf.VERSION, hostname)
 )
 
-// New creates and returns a new HTTP client object.
+// New creates and returns a new HTTP client object with a default timeout of 30 seconds.
 func New() *Client {
+	return NewWithTimeout(30 * time.Second)
+}
+
+// NewWithTimeout creates and returns a new HTTP client object with specified timeout.
+//
+// The transport is cloned from http.DefaultTransport to inherit standard library defaults
+// (such as Proxy, HTTP/2 knobs, and future Go defaults), then customized with the project's
+// own TLS, keep-alive, and connection pool settings.
+func NewWithTimeout(timeout time.Duration) *Client {
+	// Clone from http.DefaultTransport to inherit standard library defaults,
+	// then override with project-specific settings.
+	var transport *http.Transport
+	if defaultTransport, ok := http.DefaultTransport.(*http.Transport); ok {
+		transport = defaultTransport.Clone()
+	} else {
+		// Fallback to manual construction if DefaultTransport is not *http.Transport
+		// (e.g., if the application replaced it with a custom RoundTripper)
+		transport = &http.Transport{
+			DisableKeepAlives:     true,
+			MaxIdleConns:          100,
+			MaxIdleConnsPerHost:   50,
+			MaxConnsPerHost:       100,
+			IdleConnTimeout:       90 * time.Second,
+			TLSHandshakeTimeout:   10 * time.Second,
+			ExpectContinueTimeout: 1 * time.Second,
+			ForceAttemptHTTP2:     true,
+		}
+	}
+	// No validation for https certification of the server in default.
+	transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
+	transport.DisableKeepAlives = true
+	transport.MaxIdleConnsPerHost = 50
+	transport.MaxConnsPerHost = 100
+	defaultClient := http.Client{
+		Transport: transport,
+		Timeout:   timeout,
+	}
+	return NewWithHttpClient(&defaultClient)
+}
+
+// NewWithHttpClient creates and returns a new Client with given http.Client.
+// It panics if client is nil.
+func NewWithHttpClient(client *http.Client) *Client {
+	if client == nil {
+		panic(`gclient: client must not be nil`)
+	}
 	c := &Client{
-		Client: http.Client{
-			Transport: &http.Transport{
-				// No validation for https certification of the server in default.
-				TLSClientConfig: &tls.Config{
-					InsecureSkipVerify: true,
-				},
-				DisableKeepAlives:     true,
-				MaxIdleConns:          100,
-				MaxIdleConnsPerHost:   50,
-				MaxConnsPerHost:       100,
-				IdleConnTimeout:       90 * time.Second,
-				ResponseHeaderTimeout: 30 * time.Second,
-				ExpectContinueTimeout: 1 * time.Second,
-				TLSHandshakeTimeout:   10 * time.Second,
-				ForceAttemptHTTP2:     true,
-				DisableCompression:    false,
-				DialContext: (&net.Dialer{
-					Timeout:   30 * time.Second,
-					KeepAlive: 30 * time.Second,
-				}).DialContext,
-			},
-		},
+		Client:    *client,
 		header:    make(map[string]string),
 		cookies:   make(map[string]string),
 		builder:   gsel.GetBuilder(),

net/gclient/gclient_config.go

@@ -95,11 +95,68 @@ func (c *Client) SetPrefix(prefix string) *Client {
 }
 
 // SetTimeout sets the request timeout for the client.
+// It only updates the client timeout, not transport timeouts.
+// Use SetTransportTimeout to configure transport-level timeouts.
+//
+// Note: If a SOCKS5 proxy has been configured via SetProxy, the proxy dialer
+// snapshots the client timeout at setup time. Call SetTimeout before SetProxy
+// to ensure the proxy dialer uses the updated timeout value.
 func (c *Client) SetTimeout(t time.Duration) *Client {
 	c.Client.Timeout = t
 	return c
 }
 
+// SetTransportTimeout sets the transport-level timeouts for the client.
+// It configures ResponseHeaderTimeout, TLSHandshakeTimeout, and ExpectContinueTimeout.
+// Use this method to set fine-grained timeouts for different phases of the request.
+//
+// Note: This is a no-op if c.Transport is not a *http.Transport (for example,
+// after calling SetTransport with a custom RoundTripper).
+func (c *Client) SetTransportTimeout(responseHeaderTimeout, tlsHandshakeTimeout, expectContinueTimeout time.Duration) *Client {
+	if transport, ok := c.Transport.(*http.Transport); ok {
+		transport.ResponseHeaderTimeout = responseHeaderTimeout
+		transport.TLSHandshakeTimeout = tlsHandshakeTimeout
+		transport.ExpectContinueTimeout = expectContinueTimeout
+	}
+	return c
+}
+
+// SetResponseHeaderTimeout sets the timeout for receiving response headers.
+// This is the maximum time to wait for the server to send response headers.
+//
+// Note: This is a no-op if c.Transport is not a *http.Transport (for example,
+// after calling SetTransport with a custom RoundTripper).
+func (c *Client) SetResponseHeaderTimeout(t time.Duration) *Client {
+	if transport, ok := c.Transport.(*http.Transport); ok {
+		transport.ResponseHeaderTimeout = t
+	}
+	return c
+}
+
+// SetTLSHandshakeTimeout sets the timeout for TLS handshake.
+// This is the maximum time to wait for TLS handshake to complete.
+//
+// Note: This is a no-op if c.Transport is not a *http.Transport (for example,
+// after calling SetTransport with a custom RoundTripper).
+func (c *Client) SetTLSHandshakeTimeout(t time.Duration) *Client {
+	if transport, ok := c.Transport.(*http.Transport); ok {
+		transport.TLSHandshakeTimeout = t
+	}
+	return c
+}
+
+// SetExpectContinueTimeout sets the timeout for Expect: 100-continue.
+// This is the maximum time to wait for the server to respond to Expect: 100-continue header.
+//
+// Note: This is a no-op if c.Transport is not a *http.Transport (for example,
+// after calling SetTransport with a custom RoundTripper).
+func (c *Client) SetExpectContinueTimeout(t time.Duration) *Client {
+	if transport, ok := c.Transport.(*http.Transport); ok {
+		transport.ExpectContinueTimeout = t
+	}
+	return c
+}
+
 // SetBasicAuth sets HTTP basic authentication information for the client.
 func (c *Client) SetBasicAuth(user, pass string) *Client {
 	c.authUser = user
@@ -216,3 +273,9 @@ func (c *Client) SetBuilder(builder gsel.Builder) {
 func (c *Client) SetDiscovery(discovery gsvc.Discovery) {
 	c.discovery = discovery
 }
+
+// SetTransport sets the transport for the client.
+func (c *Client) SetTransport(transport http.RoundTripper) *Client {
+	c.Transport = transport
+	return c
+}

net/gclient/gclient_z_unit_test.go

@@ -719,3 +719,213 @@ func TestClient_NoUrlEncode(t *testing.T) {
 		t.Assert(c.NoUrlEncode().GetContent(ctx, `/`, params), `path=/data/binlog`)
 	})
 }
+
+func TestClient_NewWithHttpClient_NilPanics(t *testing.T) {
+	gtest.C(t, func(t *gtest.T) {
+		var panicked bool
+		func() {
+			defer func() {
+				if r := recover(); r != nil {
+					panicked = true
+					t.Assert(fmt.Sprintf("%v", r), `gclient: client must not be nil`)
+				}
+			}()
+			gclient.NewWithHttpClient(nil)
+		}()
+		t.Assert(panicked, true)
+	})
+}
+
+func TestClient_SetTransportTimeout_ShouldUpdateTransportTimeouts(t *testing.T) {
+	gtest.C(t, func(t *gtest.T) {
+		client := gclient.New()
+		responseHeaderTimeout := 5 * time.Second
+		tlsHandshakeTimeout := 3 * time.Second
+		expectContinueTimeout := 2 * time.Second
+
+		// Set transport timeouts
+		client.SetTransportTimeout(responseHeaderTimeout, tlsHandshakeTimeout, expectContinueTimeout)
+
+		// Verify transport timeouts
+		transport, ok := client.Transport.(*http.Transport)
+		t.Assert(ok, true)
+		t.Assert(transport.ResponseHeaderTimeout, responseHeaderTimeout)
+		t.Assert(transport.TLSHandshakeTimeout, tlsHandshakeTimeout)
+		t.Assert(transport.ExpectContinueTimeout, expectContinueTimeout)
+
+		// Verify that client timeout remains at default (30s from New())
+		t.Assert(client.Client.Timeout, 30*time.Second)
+
+		// Verify that IdleConnTimeout is not changed (should remain default from DefaultTransport)
+		defaultTransport, ok := http.DefaultTransport.(*http.Transport)
+		t.Assert(ok, true)
+		t.Assert(transport.IdleConnTimeout, defaultTransport.IdleConnTimeout)
+	})
+}
+
+func TestClient_SetTimeout_ShouldOnlyUpdateClientTimeout(t *testing.T) {
+	gtest.C(t, func(t *gtest.T) {
+		client := gclient.New()
+		timeout := 5 * time.Second
+
+		// Set timeout
+		client.SetTimeout(timeout)
+
+		// Verify client timeout
+		t.Assert(client.Client.Timeout, timeout)
+
+		// Verify transport timeouts are not changed (should remain defaults)
+		transport, ok := client.Transport.(*http.Transport)
+		t.Assert(ok, true)
+		defaultTransport, ok := http.DefaultTransport.(*http.Transport)
+		t.Assert(ok, true)
+		// ResponseHeaderTimeout should match DefaultTransport
+		t.Assert(transport.ResponseHeaderTimeout, defaultTransport.ResponseHeaderTimeout)
+		// TLSHandshakeTimeout should match DefaultTransport
+		t.Assert(transport.TLSHandshakeTimeout, defaultTransport.TLSHandshakeTimeout)
+		// ExpectContinueTimeout should match DefaultTransport
+		t.Assert(transport.ExpectContinueTimeout, defaultTransport.ExpectContinueTimeout)
+	})
+}
+
+func TestClient_SetTimeout_WithDifferentValues(t *testing.T) {
+	gtest.C(t, func(t *gtest.T) {
+		client := gclient.New()
+
+		// Test with various timeout values
+		testCases := []time.Duration{
+			1 * time.Second,
+			10 * time.Second,
+			30 * time.Second,
+			1 * time.Minute,
+		}
+
+		for _, timeout := range testCases {
+			client.SetTimeout(timeout)
+
+			// Verify client timeout is set correctly
+			t.Assert(client.Client.Timeout, timeout)
+
+			// Verify transport timeouts remain unchanged (defaults)
+			transport, ok := client.Transport.(*http.Transport)
+			t.Assert(ok, true)
+			// ResponseHeaderTimeout should remain 0 (not set by default)
+			t.Assert(transport.ResponseHeaderTimeout, 0*time.Second)
+		}
+	})
+}
+
+func TestClient_Clone_ShouldPreserveTimeoutSettings(t *testing.T) {
+	gtest.C(t, func(t *gtest.T) {
+		original := gclient.New()
+		timeout := 5 * time.Second
+		original.SetTimeout(timeout)
+
+		// Clone the client
+		cloned := original.Clone()
+
+		// Verify cloned client has same timeout
+		t.Assert(cloned.Client.Timeout, timeout)
+
+		// Verify transport timeouts are preserved (should be defaults from DefaultTransport)
+		transport, ok := cloned.Transport.(*http.Transport)
+		t.Assert(ok, true)
+		defaultTransport, ok := http.DefaultTransport.(*http.Transport)
+		t.Assert(ok, true)
+		// ResponseHeaderTimeout should match DefaultTransport
+		t.Assert(transport.ResponseHeaderTimeout, defaultTransport.ResponseHeaderTimeout)
+		// TLSHandshakeTimeout should match DefaultTransport
+		t.Assert(transport.TLSHandshakeTimeout, defaultTransport.TLSHandshakeTimeout)
+
+		// Modify cloned client's timeout
+		newTimeout := 10 * time.Second
+		cloned.SetTimeout(newTimeout)
+
+		// Verify original client's timeout is unchanged
+		t.Assert(original.Client.Timeout, timeout)
+
+		// Verify cloned client has new timeout
+		t.Assert(cloned.Client.Timeout, newTimeout)
+	})
+
+	gtest.C(t, func(t *gtest.T) {
+		client := gclient.New()
+		client.SetResponseHeaderTimeout(5 * time.Second)
+		transport, ok := client.Transport.(*http.Transport)
+		t.Assert(ok, true)
+		t.Assert(transport.ResponseHeaderTimeout, 5*time.Second)
+
+		client.SetTLSHandshakeTimeout(3 * time.Second)
+		t.Assert(transport.TLSHandshakeTimeout, 3*time.Second)
+
+		client.SetExpectContinueTimeout(2 * time.Second)
+		t.Assert(transport.ExpectContinueTimeout, 2*time.Second)
+
+		// Verify client timeout is still at default (30s from New())
+		t.Assert(client.Client.Timeout, 30*time.Second)
+	})
+}
+
+func TestClient_SetTransportTimeout_WithIndividualMethods(t *testing.T) {
+	gtest.C(t, func(t *gtest.T) {
+		client := gclient.New()
+
+		// Test SetResponseHeaderTimeout
+		client.SetResponseHeaderTimeout(5 * time.Second)
+		transport, ok := client.Transport.(*http.Transport)
+		t.Assert(ok, true)
+		t.Assert(transport.ResponseHeaderTimeout, 5*time.Second)
+
+		// Test SetTLSHandshakeTimeout
+		client.SetTLSHandshakeTimeout(3 * time.Second)
+		t.Assert(transport.TLSHandshakeTimeout, 3*time.Second)
+
+		// Test SetExpectContinueTimeout
+		client.SetExpectContinueTimeout(2 * time.Second)
+		t.Assert(transport.ExpectContinueTimeout, 2*time.Second)
+
+		// Verify client timeout is still at default (30s from New())
+		t.Assert(client.Client.Timeout, 30*time.Second)
+
+		// Verify that other timeouts are preserved from DefaultTransport
+		defaultTransport, ok := http.DefaultTransport.(*http.Transport)
+		t.Assert(ok, true)
+		t.Assert(transport.IdleConnTimeout, defaultTransport.IdleConnTimeout)
+	})
+}
+
+func TestClient_SetTransportTimeout_ShouldNotAffectClientTimeout(t *testing.T) {
+	gtest.C(t, func(t *gtest.T) {
+		client := gclient.New()
+		clientTimeout := 10 * time.Second
+		client.SetTimeout(clientTimeout)
+
+		// Set transport timeouts
+		client.SetTransportTimeout(5*time.Second, 3*time.Second, 2*time.Second)
+
+		// Verify client timeout is unchanged
+		t.Assert(client.Client.Timeout, clientTimeout)
+
+		// Verify transport timeouts are set
+		transport, ok := client.Transport.(*http.Transport)
+		t.Assert(ok, true)
+		t.Assert(transport.ResponseHeaderTimeout, 5*time.Second)
+		t.Assert(transport.TLSHandshakeTimeout, 3*time.Second)
+		t.Assert(transport.ExpectContinueTimeout, 2*time.Second)
+	})
+}
+
+func TestClient_SetTransportTimeout_ZeroValues(t *testing.T) {
+	gtest.C(t, func(t *gtest.T) {
+		client := gclient.New()
+
+		// Set zero values (effectively disable timeouts)
+		client.SetTransportTimeout(0, 0, 0)
+
+		transport, ok := client.Transport.(*http.Transport)
+		t.Assert(ok, true)
+		t.Assert(transport.ResponseHeaderTimeout, 0*time.Second)
+		t.Assert(transport.TLSHandshakeTimeout, 0*time.Second)
+		t.Assert(transport.ExpectContinueTimeout, 0*time.Second)
+	})
+}