This is an automated email from the ASF dual-hosted git repository. Cole-Greer pushed a commit to branch GLVBehaviouralAlignment in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit b225746bad08114a8af7002a248e580caf8d63f9 Author: Cole Greer <[email protected]> AuthorDate: Tue Jun 2 12:37:34 2026 -0700 Add request timeout and empty-response handling to gremlin-go - Add a RequestTimeout option wired to http.Transport.ResponseHeaderTimeout so a server that accepts the connection but never responds surfaces a timeout instead of hanging (tinkerpop-y32). - Treat an empty HTTP response body as an error instead of returning an empty result set (tinkerpop-zar). - Unskip and tighten the corresponding behavioral tests. --- gremlin-go/driver/client.go | 7 +++++++ gremlin-go/driver/client_behavior_test.go | 32 +++++++++++++++++++------------ gremlin-go/driver/connection.go | 18 +++++++++++------ gremlin-go/driver/connection_test.go | 2 ++ 4 files changed, 41 insertions(+), 18 deletions(-) diff --git a/gremlin-go/driver/client.go b/gremlin-go/driver/client.go index 5e658d5ea1..56c4b7aeac 100644 --- a/gremlin-go/driver/client.go +++ b/gremlin-go/driver/client.go @@ -60,6 +60,12 @@ type ClientSettings struct { // Default: 30 seconds. Set to 0 to use the default. KeepAliveInterval time.Duration + // RequestTimeout is the maximum time to wait for a response after sending a request. + // This bounds the time between finishing writing the request and receiving the response + // headers from the server. It is independent of ConnectionTimeout which only governs + // connection establishment. Set to 0 to disable (no timeout). Default: 0 (disabled). + RequestTimeout time.Duration + EnableUserAgentOnConnect bool // RequestInterceptors are functions that modify HTTP requests before sending. @@ -101,6 +107,7 @@ func NewClient(url string, configurations ...func(settings *ClientSettings)) (*C connSettings := &connectionSettings{ tlsConfig: settings.TlsConfig, connectionTimeout: settings.ConnectionTimeout, + requestTimeout: settings.RequestTimeout, maxConnsPerHost: settings.MaximumConcurrentConnections, maxIdleConnsPerHost: settings.MaxIdleConnections, idleConnTimeout: settings.IdleConnectionTimeout, diff --git a/gremlin-go/driver/client_behavior_test.go b/gremlin-go/driver/client_behavior_test.go index dee4e24d8a..51f66ba7bb 100644 --- a/gremlin-go/driver/client_behavior_test.go +++ b/gremlin-go/driver/client_behavior_test.go @@ -159,14 +159,10 @@ func TestShouldHandleEmptyResponseBody(t *testing.T) { done <- submitExpectErr(client, gremlinEmptyBody) }() - // The key requirement is that an empty response body does not hang. - // NOTE: Unlike the Java/Python/JS drivers (which raise an error), the Go - // driver currently treats an empty body as an empty (successful) result - // set rather than an error. This driver gap is flagged in the cross-GLV - // error-message audit (tinkerpop-8lw.6) for further consideration. select { - case <-done: - // completed without hanging - acceptable for now + case submitErr := <-done: + require.Error(t, submitErr) + assert.Contains(t, submitErr.Error(), "empty response body") case <-ctx.Done(): t.Fatal("request hung on empty response body") } @@ -186,11 +182,23 @@ func TestShouldHandleSlowResponse(t *testing.T) { } func TestShouldTimeoutWhenServerNeverResponds(t *testing.T) { - // The Go driver's ConnectionTimeout only governs connection establishment, - // not how long to wait for a response. With no client-side request/read - // timeout, a server that never responds causes an indefinite hang. Skipped - // until the driver supports a request timeout (flagged in tinkerpop-8lw.6). - t.Skip("Go driver lacks a client-side request/read timeout") + url := socketServerURL() + client, err := NewClient(url, func(settings *ClientSettings) { + settings.RequestTimeout = 2 * time.Second + }) + if err != nil { + t.Skip("Socket server not available") + } + defer client.Close() + + // Verify connectivity before testing the no-response scenario + if err := submitExpectErr(client, gremlinSingleVertex); err != nil { + t.Skip("Socket server not available") + } + + err = submitExpectErr(client, gremlinNoResponse) + require.Error(t, err) + assert.Contains(t, err.Error(), "timeout") } func TestShouldHandleAsyncRequestsDuringConnectionClose(t *testing.T) { diff --git a/gremlin-go/driver/connection.go b/gremlin-go/driver/connection.go index ea0183626d..eb91f7490d 100644 --- a/gremlin-go/driver/connection.go +++ b/gremlin-go/driver/connection.go @@ -37,6 +37,7 @@ import ( type connectionSettings struct { tlsConfig *tls.Config connectionTimeout time.Duration + requestTimeout time.Duration maxConnsPerHost int maxIdleConnsPerHost int idleConnTimeout time.Duration @@ -98,11 +99,12 @@ func newConnection(handler *logHandler, url string, connSettings *connectionSett Timeout: connectionTimeout, KeepAlive: keepAliveInterval, }).DialContext, - TLSClientConfig: connSettings.tlsConfig, - MaxConnsPerHost: maxConnsPerHost, - MaxIdleConnsPerHost: maxIdleConnsPerHost, - IdleConnTimeout: idleConnTimeout, - DisableCompression: !connSettings.enableCompression, + TLSClientConfig: connSettings.tlsConfig, + MaxConnsPerHost: maxConnsPerHost, + MaxIdleConnsPerHost: maxIdleConnsPerHost, + IdleConnTimeout: idleConnTimeout, + DisableCompression: !connSettings.enableCompression, + ResponseHeaderTimeout: connSettings.requestTimeout, } return &connection{ @@ -315,7 +317,11 @@ func (c *connection) getReader(resp *http.Response) (io.Reader, io.Closer, error func (c *connection) streamToResultSet(reader io.Reader, rs ResultSet) { d := NewGraphBinaryDeserializer(reader) if err := d.ReadHeader(); err != nil { - if err != io.EOF { + if err == io.EOF { + emptyBodyErr := fmt.Errorf("received empty response body from server") + c.logHandler.logf(Error, failedToReceiveResponse, emptyBodyErr.Error()) + rs.setError(emptyBodyErr) + } else { c.logHandler.logf(Error, failedToReceiveResponse, err.Error()) rs.setError(err) } diff --git a/gremlin-go/driver/connection_test.go b/gremlin-go/driver/connection_test.go index 572cc95460..7a336c3802 100644 --- a/gremlin-go/driver/connection_test.go +++ b/gremlin-go/driver/connection_test.go @@ -1193,6 +1193,7 @@ func TestConnectionPoolSettings(t *testing.T) { idleConnTimeout: 300 * time.Second, keepAliveInterval: 60 * time.Second, connectionTimeout: 30 * time.Second, + requestTimeout: 5 * time.Second, } conn := newConnection(newTestLogHandler(), "http://localhost:8182/gremlin", customSettings) @@ -1203,6 +1204,7 @@ func TestConnectionPoolSettings(t *testing.T) { assert.Equal(t, 256, transport.MaxConnsPerHost, "MaxConnsPerHost should be custom value") assert.Equal(t, 16, transport.MaxIdleConnsPerHost, "MaxIdleConnsPerHost should be custom value") assert.Equal(t, 300*time.Second, transport.IdleConnTimeout, "IdleConnTimeout should be custom value") + assert.Equal(t, 5*time.Second, transport.ResponseHeaderTimeout, "ResponseHeaderTimeout should be custom value") }) t.Run("partial custom settings use defaults for unset values", func(t *testing.T) {
