Cole-Greer opened a new pull request, #3519: URL: https://github.com/apache/tinkerpop/pull/3519
This PR is a followup to the audit findings from https://github.com/apache/tinkerpop/pull/3436. Improves error-handling consistency for adversarial HTTP scenarios (empty responses, malformed responses, dead connections) across the Gremlin drivers, and reconciles a few timeout-related test gaps against master's connection-options standardization work. ### Java (gremlin-driver) - **Problem:** An empty HTTP response body surfaced as a bare EOFException with no context. - **Fix:** GraphBinaryStreamResponseReader now detects this case and raises a clear "Server returned an empty response body" error. ### Go (gremlin-go) - **Problem:** Same empty-body issue as Java — the driver silently returned an empty result set instead of an error. - **Fix:** Empty response bodies now produce a clear error. - **Problem:** TestShouldTimeoutWhenServerNeverResponds was skipped outright since the driver had no client-side read/request timeout at the time. - **Fix:** Un-skipped, now uses the standardized ReadTimeout option (confirmed it correctly bounds this case, since Go's per-read deadline is armed before the first read). ### Python (gremlin_python) - **Problem:** Transport-level failures (server closes connection, partial response, empty body) leaked raw aiohttp exception types (ClientConnectionError, ClientPayloadError, ServerDisconnectedError) with no Gremlin-level context. - **Fix:** Wraps these in a new GremlinConnectionError with an actionable message. - **Problem:** A half-closed connection wasn't evicted from the pool after an empty response, so the client could keep reusing a dead connection. - **Fix:** Releases the response on error so the connection is evicted and the pool recovers. ### .NET (Gremlin.Net) - **Problem:** A malformed response yielded a non-deterministic exception type (IOException or KeyNotFoundException depending on where deserialization failed), forcing the test to accept either type. - **Fix:** Deserialization failures are now wrapped consistently. Reuses the existing ResponseException (via a new constructor overload with a NoStatusCode sentinel) rather than introducing a separate exception type, keeping one exception type per driver for response-related failures. ### JavaScript (gremlin-javascript) - **Problem:** No client-side timeout existed for a server that never responds at all. - **Fix:** Investigated using the newly standardized readTimeoutMillis option, but confirmed it only maps to undici's bodyTimeout, which doesn't start ticking until response parsing begins — so it never fires for a server that sends nothing. The test is left skipped, but with updated context. VOTE +1 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
