(tinkerpop) 05/05: Improve empty-response error and add request-encoding tests (gremlin-driver)

[co...@apache.org]

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 00b7d21f53ac61871077b77e220f57257f219b71
Author: Cole Greer <co...@colegreer.ca>
AuthorDate: Tue Jun 2 12:37:56 2026 -0700

    Improve empty-response error and add request-encoding tests (gremlin-driver)
    
    - Produce a clear "Server returned an empty response body" error instead of a
      bare EOFException when the response body is empty (tinkerpop-28f).
    - Add hermetic HttpGremlinRequestEncoder tests validating the User-Agent header
      (present/absent) and that per-request settings (timeout, batchSize,
      materializeProperties) survive encoding into the outgoing request
      (tinkerpop-3tw.8).
---
 .../stream/GraphBinaryStreamResponseReader.java    |  19 ++-
 .../GraphBinaryStreamResponseReaderTest.java       |  23 ++++
 .../handler/HttpGremlinRequestEncoderTest.java     | 133 +++++++++++++++++++++
 3 files changed, 173 insertions(+), 2 deletions(-)

diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/stream/GraphBinaryStreamResponseReader.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/stream/GraphBinaryStreamResponseReader.java
index 11491903e8..ba7370f71e 100644
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/stream/GraphBinaryStreamResponseReader.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/stream/GraphBinaryStreamResponseReader.java
@@ -29,6 +29,7 @@ import org.apache.tinkerpop.gremlin.structure.io.binary.Marker;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.io.EOFException;
 import java.util.concurrent.atomic.AtomicReference;
 
 /**
@@ -95,11 +96,25 @@ public class GraphBinaryStreamResponseReader implements Runnable {
                 resultSet.markError(new ResponseException(HttpResponseStatus.valueOf(statusCode), message, exception));
             }
         } catch (Throwable t) {
-            logger.warn("Error reading streaming response", t);
-            resultSet.markError(t);
+            if (buffer.readerIndex() == 0 && hasCause(t, EOFException.class)) {
+                final RuntimeException empty = new RuntimeException(
+                        "Server returned an empty response body", t);
+                logger.warn("Error reading streaming response", empty);
+                resultSet.markError(empty);
+            } else {
+                logger.warn("Error reading streaming response", t);
+                resultSet.markError(t);
+            }
         } finally {
             pendingResultSet.compareAndSet(resultSet, null);
             buffer.release();
         }
     }
+
+    private static boolean hasCause(final Throwable t, final Class<? extends Throwable> type) {
+        for (Throwable cause = t; cause != null; cause = cause.getCause()) {
+            if (type.isInstance(cause)) return true;
+        }
+        return false;
+    }
 }
diff --git a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/handler/GraphBinaryStreamResponseReaderTest.java b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/handler/GraphBinaryStreamResponseReaderTest.java
index 484ec7d524..2b657a1f9b 100644
--- a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/handler/GraphBinaryStreamResponseReaderTest.java
+++ b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/handler/GraphBinaryStreamResponseReaderTest.java
@@ -223,4 +223,27 @@ public class GraphBinaryStreamResponseReaderTest {
         assertTrue(results.isEmpty());
         assertNull(pending.get());
     }
+
+    @Test
+    public void shouldProduceClearErrorOnEmptyBody() throws Exception {
+        final ResultSet rs = new ResultSet(executor, RequestMessage.build("g.V()").create(), null);
+        final AtomicReference<ResultSet> pending = new AtomicReference<>(rs);
+
+        // Simulate an empty HTTP response body — no bytes offered before end-of-stream
+        final ByteBufQueueInputStream stream = new ByteBufQueueInputStream();
+        stream.signalEndOfStream();
+
+        final InputStreamBuffer buffer = new InputStreamBuffer(stream);
+        new GraphBinaryStreamResponseReader(buffer, reader, rs, pending).run();
+
+        assertTrue(rs.allItemsAvailable());
+        try {
+            rs.all().get();
+            fail("Expected exception");
+        } catch (Exception e) {
+            assertTrue(e.getCause() instanceof RuntimeException);
+            assertEquals("Server returned an empty response body", e.getCause().getMessage());
+        }
+        assertNull(pending.get());
+    }
 }
diff --git a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/handler/HttpGremlinRequestEncoderTest.java b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/handler/HttpGremlinRequestEncoderTest.java
new file mode 100644
index 0000000000..b822bbcab8
--- /dev/null
+++ b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/handler/HttpGremlinRequestEncoderTest.java
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.tinkerpop.gremlin.driver.handler;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.channel.ChannelHandler;
+import io.netty.channel.embedded.EmbeddedChannel;
+import io.netty.handler.codec.http.FullHttpRequest;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.tinkerpop.gremlin.driver.RequestInterceptor;
+import org.apache.tinkerpop.gremlin.driver.RequestOptions;
+import org.apache.tinkerpop.gremlin.driver.UserAgent;
+import org.apache.tinkerpop.gremlin.driver.interceptor.PayloadSerializingInterceptor;
+import org.apache.tinkerpop.gremlin.util.Tokens;
+import org.apache.tinkerpop.gremlin.util.message.RequestMessage;
+import org.apache.tinkerpop.gremlin.util.ser.GraphBinaryMessageSerializerV4;
+import org.junit.Test;
+
+import java.net.InetSocketAddress;
+import java.net.SocketAddress;
+import java.net.URI;
+import java.util.Collections;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+public class HttpGremlinRequestEncoderTest {
+
+    private static final InetSocketAddress REMOTE = new InetSocketAddress("127.0.0.1", 8182);
+
+    private static class TestChannel extends EmbeddedChannel {
+        TestChannel(ChannelHandler... handlers) {
+            super(handlers);
+        }
+
+        @Override
+        public SocketAddress remoteAddress() {
+            return REMOTE;
+        }
+    }
+
+    private EmbeddedChannel createChannel(final boolean userAgentEnabled) {
+        final GraphBinaryMessageSerializerV4 serializer = new GraphBinaryMessageSerializerV4();
+        final List<Pair<String, ? extends RequestInterceptor>> interceptors =
+                Collections.singletonList(Pair.of("serializer", new PayloadSerializingInterceptor(serializer)));
+        final URI uri = URI.create("http://localhost:8182/gremlin");
+        final HttpGremlinRequestEncoder encoder = new HttpGremlinRequestEncoder(
+                serializer, interceptors, userAgentEnabled, false, uri);
+        return new TestChannel(encoder);
+    }
+
+    @Test
+    public void shouldIncludeUserAgentHeader() {
+        final EmbeddedChannel channel = createChannel(true);
+        final RequestMessage msg = RequestMessage.build("g.V()").create();
+
+        assertTrue(channel.writeOutbound(msg));
+        final FullHttpRequest request = channel.readOutbound();
+
+        assertNotNull(request);
+        assertEquals(UserAgent.USER_AGENT, request.headers().get("user-agent"));
+        request.release();
+    }
+
+    @Test
+    public void shouldExcludeUserAgentHeaderWhenDisabled() {
+        final EmbeddedChannel channel = createChannel(false);
+        final RequestMessage msg = RequestMessage.build("g.V()").create();
+
+        assertTrue(channel.writeOutbound(msg));
+        final FullHttpRequest request = channel.readOutbound();
+
+        assertNotNull(request);
+        assertNull(request.headers().get("user-agent"));
+        request.release();
+    }
+
+    @Test
+    public void shouldEncodePerRequestSettingsInBody() throws Exception {
+        final EmbeddedChannel channel = createChannel(true);
+        final GraphBinaryMessageSerializerV4 deserializer = new GraphBinaryMessageSerializerV4();
+
+        // Build RequestMessage the same way Client.submitAsync does from RequestOptions
+        final RequestOptions options = RequestOptions.build()
+                .timeout(5000L)
+                .batchSize(250)
+                .materializeProperties(Tokens.MATERIALIZE_PROPERTIES_TOKENS)
+                .create();
+
+        final RequestMessage.Builder builder = RequestMessage.build("g.V()");
+        builder.addChunkSize(options.getBatchSize().get());
+        options.getTimeout().ifPresent(builder::addTimeoutMillis);
+        options.getMaterializeProperties().ifPresent(builder::addMaterializeProperties);
+        final RequestMessage msg = builder.create();
+
+        assertTrue(channel.writeOutbound(msg));
+        final FullHttpRequest request = channel.readOutbound();
+        assertNotNull(request);
+
+        try {
+            // Deserialize the body back into a RequestMessage to prove round-trip fidelity
+            final ByteBuf body = request.content();
+            assertFalse("body should not be empty", body.readableBytes() == 0);
+            final RequestMessage decoded = deserializer.deserializeBinaryRequest(body);
+
+            assertEquals(5000L, (long) decoded.getField(Tokens.TIMEOUT_MS));
+            assertEquals(250, (int) decoded.getField(Tokens.ARGS_BATCH_SIZE));
+            assertEquals(Tokens.MATERIALIZE_PROPERTIES_TOKENS, decoded.getField(Tokens.ARGS_MATERIALIZE_PROPERTIES));
+        } finally {
+            request.release();
+        }
+    }
+}