This is an automated email from the ASF dual-hosted git repository. xiazcy pushed a commit to branch go-http-fix in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit d8e43c3ca985b23a1ab3d8e86ea7937bb1807a20 Author: Yang Xia <[email protected]> AuthorDate: Tue Mar 24 20:43:13 2026 -0700 add proper error handling for non-graphbinary cases --- gremlin-go/driver/connection.go | 44 +++++++++++- gremlin-go/driver/connection_test.go | 127 +++++++++++++++++++++++++++++++++++ 2 files changed, 168 insertions(+), 3 deletions(-) diff --git a/gremlin-go/driver/connection.go b/gremlin-go/driver/connection.go index 184efb681b..5086965a8d 100644 --- a/gremlin-go/driver/connection.go +++ b/gremlin-go/driver/connection.go @@ -22,13 +22,13 @@ package gremlingo import ( "bytes" "compress/zlib" - "crypto/sha256" "crypto/tls" - "encoding/hex" + "encoding/json" + "fmt" "io" "net" "net/http" - "net/url" + "strings" "time" ) @@ -230,6 +230,28 @@ func (c *connection) executeAndStream(data []byte, rs ResultSet) { } }() + // If the HTTP status indicates an error and the response is not GraphBinary, + // read the body as a text/JSON error message instead of attempting binary + // deserialization which would produce cryptic errors. + contentType := resp.Header.Get(HeaderContentType) + if resp.StatusCode >= 400 && !strings.Contains(contentType, graphBinaryMimeType) { + bodyBytes, readErr := io.ReadAll(resp.Body) + if readErr != nil { + c.logHandler.logf(Error, failedToReceiveResponse, readErr.Error()) + rs.setError(fmt.Errorf("Gremlin Server returned HTTP %d and failed to read body: %w", + resp.StatusCode, readErr)) + return + } + errorBody := string(bodyBytes) + errorMsg := tryExtractJSONError(errorBody) + if errorMsg == "" { + errorMsg = fmt.Sprintf("Gremlin Server returned HTTP %d: %s", resp.StatusCode, errorBody) + } + c.logHandler.logf(Error, failedToReceiveResponse, errorMsg) + rs.setError(fmt.Errorf("%s", errorMsg)) + return + } + reader, zlibReader, err := c.getReader(resp) if err != nil { c.logHandler.logf(Error, failedToReceiveResponse, err.Error()) @@ -308,6 +330,22 @@ func (c *connection) streamToResultSet(reader io.Reader, rs ResultSet) { } } +// tryExtractJSONError attempts to extract an error message from a JSON response body. +// The server sometimes responds with a JSON object containing a "message" field +// even when it cannot produce a GraphBinary response. +func tryExtractJSONError(body string) string { + var obj map[string]interface{} + if err := json.Unmarshal([]byte(body), &obj); err != nil { + return "" + } + if msg, ok := obj["message"]; ok { + if s, ok := msg.(string); ok { + return s + } + } + return "" +} + func (c *connection) close() { c.httpClient.CloseIdleConnections() } diff --git a/gremlin-go/driver/connection_test.go b/gremlin-go/driver/connection_test.go index a222448f2c..a0e8414223 100644 --- a/gremlin-go/driver/connection_test.go +++ b/gremlin-go/driver/connection_test.go @@ -993,10 +993,137 @@ func TestConnectionWithMockServer(t *testing.T) { _, _ = rs.All() // drain }) + + t.Run("returns plain text error for non-GraphBinary 500 response", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/plain") + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte("Internal Server Error")) + })) + defer server.Close() + + conn := newConnection(newTestLogHandler(), server.URL, &connectionSettings{}) + rs, err := conn.submit(&request{gremlin: "g.V()", fields: map[string]interface{}{}}) + require.NoError(t, err) + + _, _ = rs.All() + rsErr := rs.GetError() + require.Error(t, rsErr) + assert.Contains(t, rsErr.Error(), "HTTP 500") + assert.Contains(t, rsErr.Error(), "Internal Server Error") + }) + + t.Run("extracts message from JSON error response", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnauthorized) + w.Write([]byte(`{"message":"Authentication required"}`)) + })) + defer server.Close() + + conn := newConnection(newTestLogHandler(), server.URL, &connectionSettings{}) + rs, err := conn.submit(&request{gremlin: "g.V()", fields: map[string]interface{}{}}) + require.NoError(t, err) + + _, _ = rs.All() + rsErr := rs.GetError() + require.Error(t, rsErr) + assert.Equal(t, "Authentication required", rsErr.Error()) + }) + + t.Run("falls back to raw body for non-JSON error response", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html") + w.WriteHeader(http.StatusBadGateway) + w.Write([]byte("<html>Bad Gateway</html>")) + })) + defer server.Close() + + conn := newConnection(newTestLogHandler(), server.URL, &connectionSettings{}) + rs, err := conn.submit(&request{gremlin: "g.V()", fields: map[string]interface{}{}}) + require.NoError(t, err) + + _, _ = rs.All() + rsErr := rs.GetError() + require.Error(t, rsErr) + assert.Contains(t, rsErr.Error(), "HTTP 502") + assert.Contains(t, rsErr.Error(), "<html>Bad Gateway</html>") + }) + + t.Run("falls through to GraphBinary deserialization for GraphBinary error responses", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", graphBinaryMimeType) + w.WriteHeader(http.StatusInternalServerError) + // Write invalid GraphBinary — the point is that we don't short-circuit + // to the text error path when Content-Type is GraphBinary + w.Write([]byte{0x00}) + })) + defer server.Close() + + conn := newConnection(newTestLogHandler(), server.URL, &connectionSettings{}) + rs, err := conn.submit(&request{gremlin: "g.V()", fields: map[string]interface{}{}}) + require.NoError(t, err) + + _, _ = rs.All() + rsErr := rs.GetError() + // Should get a deserialization error, NOT an "HTTP 500" text error + if rsErr != nil { + assert.NotContains(t, rsErr.Error(), "HTTP 500") + } + }) + + t.Run("interceptors run before serialization is checked", func(t *testing.T) { + var interceptorHeaders http.Header + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + conn := newConnection(newTestLogHandler(), server.URL, &connectionSettings{}) + conn.AddInterceptor(func(req *HttpRequest) error { + interceptorHeaders = req.Headers.Clone() + return nil + }) + + rs, err := conn.submit(&request{gremlin: "g.V()", fields: map[string]interface{}{}}) + require.NoError(t, err) + _, _ = rs.All() + + // Interceptor should see the default headers + assert.Equal(t, graphBinaryMimeType, interceptorHeaders.Get("Content-Type")) + assert.Equal(t, graphBinaryMimeType, interceptorHeaders.Get("Accept")) + }) } // Tests for connection pool configuration settings +func TestTryExtractJSONError(t *testing.T) { + t.Run("extracts message from valid JSON", func(t *testing.T) { + result := tryExtractJSONError(`{"message":"auth failed","code":401}`) + assert.Equal(t, "auth failed", result) + }) + + t.Run("returns empty for JSON without message field", func(t *testing.T) { + result := tryExtractJSONError(`{"error":"something went wrong"}`) + assert.Equal(t, "", result) + }) + + t.Run("returns empty for invalid JSON", func(t *testing.T) { + result := tryExtractJSONError("not json at all") + assert.Equal(t, "", result) + }) + + t.Run("returns empty for HTML content", func(t *testing.T) { + result := tryExtractJSONError("<html><body>Error</body></html>") + assert.Equal(t, "", result) + }) + + t.Run("returns empty for empty string", func(t *testing.T) { + result := tryExtractJSONError("") + assert.Equal(t, "", result) + }) +} + func TestConnectionPoolSettings(t *testing.T) { t.Run("default values are applied when settings are 0", func(t *testing.T) { // Create connection with empty settings (all zeros)
