(tinkerpop) 02/09: Add request timeout and wrap transport errors in gremlin-javascript

[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 9c5a8df23bb197de7f7ba33f2676d43cc55fd422
Author: Cole Greer <co...@colegreer.ca>
AuthorDate: Tue Jun 2 12:37:34 2026 -0700

    Add request timeout and wrap transport errors in gremlin-javascript
    
    - Add a requestTimeout option bounding time-to-first-byte (cleared once response
      headers arrive so slow streams are unaffected); reject with a ResponseError
      when the server never responds (tinkerpop-uvl).
    - Wrap low-level fetch/transport errors in a Gremlin-aware ResponseError with an
      actionable message, preserving the original error as the cause (tinkerpop-1fn).
    - Tighten behavioral test assertions.
---
 .../gremlin-javascript/lib/driver/connection.ts    | 62 ++++++++++++++++++----
 .../test/integration/client-behavior-tests.js      | 24 +++++++--
 2 files changed, 72 insertions(+), 14 deletions(-)

diff --git a/gremlin-js/gremlin-javascript/lib/driver/connection.ts b/gremlin-js/gremlin-javascript/lib/driver/connection.ts
index fc4e1c0bda..27e6af37f5 100644
--- a/gremlin-js/gremlin-javascript/lib/driver/connection.ts
+++ b/gremlin-js/gremlin-javascript/lib/driver/connection.ts
@@ -80,6 +80,8 @@ export type ConnectionOptions = {
   /** An optional auth interceptor. As a convenience, this is always appended to the end of the
    *  interceptor list so it runs last, after any user interceptors have modified the request. */
   auth?: RequestInterceptor;
+  /** Maximum time in milliseconds to wait for a server response before aborting the request. Undefined means no timeout. */
+  requestTimeout?: number;
 };
 
 /** The default per-request batch size used when neither the request nor the connection sets one. */
@@ -322,20 +324,60 @@ export default class Connection extends EventEmitter {
 
     this._log('debug', `Sending ${httpRequest.method} request to ${httpRequest.url}`);
 
-    return httpFetch.fetch(httpRequest.url, {
-      method: httpRequest.method,
-      headers: httpRequest.headers,
-      body: httpRequest.body,
-      signal,
-      // Node only: the undici dispatcher carries the connection-pool options. In the browser
-      // it is undefined and the field is omitted, letting the user agent manage the transport.
-      ...(this._dispatcher ? { dispatcher: this._dispatcher } : {}),
-    } as RequestInit);
+    let effectiveSignal = signal;
+    const timeoutMs = this.options.requestTimeout;
+    let timeoutId: ReturnType<typeof setTimeout> | undefined;
+    let timeoutController: AbortController | undefined;
+
+    if (timeoutMs !== undefined) {
+      timeoutController = new AbortController();
+      timeoutId = setTimeout(() => timeoutController!.abort(new DOMException('TimeoutError', 'TimeoutError')), timeoutMs);
+      effectiveSignal = signal ? AbortSignal.any([signal, timeoutController.signal]) : timeoutController.signal;
+    }
+
+    try {
+      return await httpFetch.fetch(httpRequest.url, {
+        method: httpRequest.method,
+        headers: httpRequest.headers,
+        body: httpRequest.body,
+        signal: effectiveSignal,
+        // Node only: the undici dispatcher carries the connection-pool options. In the browser
+        // it is undefined and the field is omitted, letting the user agent manage the transport.
+        ...(this._dispatcher ? { dispatcher: this._dispatcher } : {}),
+      } as RequestInit);
+    } catch (err: any) {
+      if (timeoutController?.signal.aborted) {
+        const e = new ResponseError(
+          `Request timed out after ${timeoutMs}ms - the server did not respond in time`,
+          { code: 598, message: `Request timeout: ${timeoutMs}ms exceeded` },
+        );
+        e.cause = err;
+        throw e;
+      }
+      const e = new ResponseError(
+        'Connection to server closed unexpectedly. Ensure that the server is still reachable and the connection has not been closed by the server or a network device.',
+        { code: 599, message: err.message || 'Connection failed' },
+      );
+      e.cause = err;
+      throw e;
+    } finally {
+      if (timeoutId !== undefined) clearTimeout(timeoutId);
+    }
   }
 
   async #handleResponse(response: Response) {
+    let buffer: Buffer;
+    try {
+      buffer = Buffer.from(await response.arrayBuffer());
+    } catch (err: any) {
+      const e = new ResponseError(
+        'Connection to server closed unexpectedly. Ensure that the server is still reachable and the connection has not been closed by the server or a network device.',
+        { code: 599, message: err.message || 'Connection failed' },
+      );
+      e.cause = err;
+      throw e;
+    }
     const contentType = response.headers.get("Content-Type");
-    const buffer = Buffer.from(await response.arrayBuffer());
     const reader = this.#getReaderForContentType(contentType);
 
     if (!response.ok) {
diff --git a/gremlin-js/gremlin-javascript/test/integration/client-behavior-tests.js b/gremlin-js/gremlin-javascript/test/integration/client-behavior-tests.js
index 98f6537aa0..53be66adbc 100644
--- a/gremlin-js/gremlin-javascript/test/integration/client-behavior-tests.js
+++ b/gremlin-js/gremlin-javascript/test/integration/client-behavior-tests.js
@@ -67,7 +67,11 @@ describe('Client Behavior', function () {
   });
 
   it('should handle connection close before response and recover', async function () {
-    await assert.rejects(client.submit(GREMLIN_CLOSE_CONNECTION), /fetch failed/);
+    await assert.rejects(client.submit(GREMLIN_CLOSE_CONNECTION), (err) => {
+      assert.strictEqual(err.name, 'ResponseError');
+      assert.match(err.message, /Connection to server closed unexpectedly/);
+      return true;
+    });
     const result = await client.submit(GREMLIN_SINGLE_VERTEX);
     assert.strictEqual(result.length, 1);
   });
@@ -93,7 +97,11 @@ describe('Client Behavior', function () {
   });
 
   it('should handle partial content close and recover', async function () {
-    await assert.rejects(client.submit(GREMLIN_PARTIAL_CONTENT_CLOSE), /terminated/);
+    await assert.rejects(client.submit(GREMLIN_PARTIAL_CONTENT_CLOSE), (err) => {
+      assert.strictEqual(err.name, 'ResponseError');
+      assert.match(err.message, /Connection to server closed unexpectedly/);
+      return true;
+    });
     const result = await client.submit(GREMLIN_SINGLE_VERTEX);
     assert.strictEqual(result.length, 1);
   });
@@ -117,10 +125,18 @@ describe('Client Behavior', function () {
     assert.ok(result.length > 0);
   });
 
-  it.skip('should timeout when server never responds - JS driver lacks client-side idle timeout', async function () {
+  it('should timeout when server never responds', async function () {
+    this.timeout(5000);
     const shortTimeoutClient = createClient({ requestTimeout: 1000 });
     try {
-      await assert.rejects(shortTimeoutClient.submit(GREMLIN_NO_RESPONSE));
+      await assert.rejects(
+        shortTimeoutClient.submit(GREMLIN_NO_RESPONSE),
+        (err) => {
+          assert.match(err.message, /timed out/i);
+          assert.strictEqual(err.name, 'ResponseError');
+          return true;
+        },
+      );
       const result = await shortTimeoutClient.submit(GREMLIN_SINGLE_VERTEX);
       assert.strictEqual(result.length, 1);
     } finally {