GumpacG opened a new pull request, #3524: URL: https://github.com/apache/tinkerpop/pull/3524
# Align JS and .NET readTimeout with the other GLVs Brings the JavaScript and .NET drivers in line with Java, Python, and Go so that `readTimeout` bounds the wait for the initial server response, not just the response body. Previously a server that accepted the connection but never responded was not bounded by `readTimeout` in JS or .NET, falling through to a fixed framework default instead. ## Problem - JavaScript: `readTimeoutMillis` was wired only to undici `bodyTimeout`, which arms only after response body parsing begins. - .NET: `ReadTimeout` was applied only to the response body stream (via `ReadTimeoutStream`), after response headers are read. - Result: a server that never responds fell through to a fixed framework default (undici `headersTimeout`, 300s, for JS; `HttpClient.Timeout`, 100s, for .NET). - Java, Python, and Go already bound this initial-response wait because their read timeout is armed when the request is sent. JS and .NET were the outliers. ## Changes - JavaScript: in `buildAgentOptions` (lib/driver/dispatcher.ts), map `readTimeoutMillis` to undici `headersTimeout` as well as `bodyTimeout`. Behavior when the option is unset is unchanged. - .NET: wrap the header-wait `SendAsync` (HttpCompletionOption.ResponseHeadersRead) in a per-request `CancellationTokenSource.CancelAfter(ReadTimeout)` linked with the caller token, via a new `SendWithReadTimeoutAsync` helper in Connection.cs. A fired read-timeout is rethrown as `TimeoutException`. The existing per-chunk body-idle `ReadTimeoutStream` path is unchanged. When `ReadTimeout` is disabled (default), behavior is unchanged. - Docs: made the `readTimeout` scope descriptions accurate and consistent across all five GLVs in docs/src/reference/gremlin-variants.asciidoc and the cross-driver bullet in docs/src/upgrade/release-4.x.x.asciidoc. It bounds the wait for the initial server response and the idle gap between chunks, but is not a whole-request deadline. ## Tests - JavaScript: un-skipped the "server never responds" integration test (now uses `readTimeoutMillis`); added unit assertions that `readTimeoutMillis` maps to both `headersTimeout` and `bodyTimeout`, including the `0` (disabled) case. - .NET: added a unit test (a handler that never responds) and an integration test (short `ReadTimeout`, no caller cancellation token); both assert a `TimeoutException` whose message contains "waiting for the initial server response". Assisted-by: Kiro: Claude Opus 4.8 -- 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]
