diff --git a/go/fetch/builder.go b/go/fetch/builder.go index abc0d8f..c0bd72b 100644 --- a/go/fetch/builder.go +++ b/go/fetch/builder.go @@ -11,6 +11,7 @@ type ClientBuilder struct { baseHeaders http.Header retryOpts *RetryOptions timeoutOpts *TimeoutOptions + connectTimeout time.Duration rateLimitOpts *RateLimitOptions rateLimitRetryOpts *RateLimitRetryOptions circuitBreakerOpts *CircuitBreakerOptions @@ -48,6 +49,22 @@ func (b *ClientBuilder) WithTimeout(timeout time.Duration) *ClientBuilder { return b } +// WithConnectTimeout bounds only the connection-establishment phase, separate +// from the whole-request WithTimeout. When set (> 0), the SDK-constructed +// *http.Client uses an *http.Transport (cloned from the default, so all other +// settings are preserved) whose dialer times out after d. A black-holed connect +// then fails in ~d and the configured retry can land on a live endpoint, +// instead of stalling until the whole-request timeout. Leaving it unset (0) +// preserves the previous no-connect-timeout behavior byte-for-byte. +// +// Note: if a caller-provided *http.Client is set via WithHTTPClient, that +// client's transport is left untouched — the connect timeout only applies to +// the transport this SDK constructs. +func (b *ClientBuilder) WithConnectTimeout(d time.Duration) *ClientBuilder { + b.connectTimeout = d + return b +} + // WithRetry configures retry behavior. Pass nil to disable retries. func (b *ClientBuilder) WithRetry(opts *RetryOptions) *ClientBuilder { b.retryOpts = opts @@ -156,6 +173,7 @@ func (b *ClientBuilder) Build() *Client { baseHeaders: b.baseHeaders, retry: b.retryOpts, timeout: b.timeoutOpts, + connectTimeout: b.connectTimeout, rateLimitRetry: b.rateLimitRetryOpts, hooks: b.hooks, authProvider: b.authProvider, @@ -167,6 +185,11 @@ func (b *ClientBuilder) Build() *Client { if c.httpClient == nil { c.httpClient = &http.Client{} + // Only bound the connect phase on the transport this SDK constructs; + // a caller-provided *http.Client (via WithHTTPClient) is left untouched. + if b.connectTimeout > 0 { + c.httpClient.Transport = transportWithConnectTimeout(b.connectTimeout) + } } if c.baseHeaders == nil { diff --git a/go/fetch/client.go b/go/fetch/client.go index ad5084e..cf90041 100644 --- a/go/fetch/client.go +++ b/go/fetch/client.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "io" + "net" "net/http" "strings" "time" @@ -18,6 +19,7 @@ type Client struct { baseHeaders http.Header retry *RetryOptions timeout *TimeoutOptions + connectTimeout time.Duration rateLimiter *SlidingWindowRateLimiter rateLimitRetry *RateLimitRetryOptions circuitBreaker *CircuitBreaker @@ -26,6 +28,18 @@ type Client struct { authScheme string } +// transportWithConnectTimeout clones the default transport (preserving proxy, +// TLS, and keep-alive settings) and bounds only the connection-establishment +// phase via the dialer's Timeout. A connect that never completes (e.g. a +// black-holed SYN to a dead pod IP still lingering in a ClusterIP's iptables) +// then fails in ~connectTimeout instead of stalling until the whole-request +// timeout. Slow-but-alive handlers are unaffected. +func transportWithConnectTimeout(connectTimeout time.Duration) *http.Transport { + tr := http.DefaultTransport.(*http.Transport).Clone() + tr.DialContext = (&net.Dialer{Timeout: connectTimeout}).DialContext + return tr +} + // NewClient creates a new Client with default settings (retry + timeout). // Use NewClientBuilder for advanced configuration. func NewClient() *Client { diff --git a/go/fetch/connect_timeout_test.go b/go/fetch/connect_timeout_test.go new file mode 100644 index 0000000..d3b3840 --- /dev/null +++ b/go/fetch/connect_timeout_test.go @@ -0,0 +1,52 @@ +package fetch + +import ( + "context" + "testing" + "time" +) + +// TestConnectTimeoutFailsFastOnBlackHole mirrors the Rust +// connect_timeout_tests.rs regression coverage (SMOODEV-2498 / SMOODEV-2481): +// a bounded connect timeout must fail fast on a black-holed connect instead of +// stalling until the (10x larger) whole-request timeout. +// +// 10.255.255.1 is a non-routable RFC1918 address with (almost certainly) no +// host answering, so the SYN is dropped and the connect never establishes. +func TestConnectTimeoutFailsFastOnBlackHole(t *testing.T) { + const connectTimeout = 500 * time.Millisecond + + client := NewClientBuilder(). + WithConnectTimeout(connectTimeout). + // Whole-request timeout is 10x the connect timeout: if the connect + // timeout is NOT honored, this would take ~5s and the elapsed + // assertion below would fail. + WithTimeout(5 * time.Second). + WithNoRetry(). + Build() + + start := time.Now() + _, err := SimpleGet(context.Background(), client, "http://10.255.255.1:80/anything", nil) + elapsed := time.Since(start) + + if err == nil { + t.Fatal("expected connect to fail against a black hole, got nil error") + } + // Must fail in roughly the connect window, well under the 5s whole timeout. + if elapsed >= 3*time.Second { + t.Fatalf("connect timeout did not fire fast: elapsed %v (connect timeout was %v)", elapsed, connectTimeout) + } +} + +// TestConnectTimeoutUnsetLeavesTransportUntouched verifies the default-OFF +// guarantee: when WithConnectTimeout is not called, the SDK does not construct +// a custom transport, so behavior is byte-identical to before. +func TestConnectTimeoutUnsetLeavesTransportUntouched(t *testing.T) { + client := NewClientBuilder().WithNoRetry().Build() + if client.connectTimeout != 0 { + t.Fatalf("expected connectTimeout 0 when unset, got %v", client.connectTimeout) + } + if client.httpClient.Transport != nil { + t.Fatalf("expected nil (default) transport when connect timeout unset, got %T", client.httpClient.Transport) + } +}