From 85ccf9e22b20bf11a27b7f9a8d491c89858fcd4c Mon Sep 17 00:00:00 2001
From: Bert <albert@lancedb.com>
Date: Tue, 23 Jul 2024 13:02:46 -0400
Subject: [PATCH] feat!: correct timeout argument lancedb nodejs sdk (#1468)

Correct the timeout argument to `connect` in @lancedb/lancedb node SDK.
`RemoteConnectionOptions` specified two fields `connectionTimeout` and
`readTimeout`, probably to be consistent with the python SDK, but only
`connectionTimeout` was being used and it was passed to axios in such a
way that this covered the enture remote request (connect + read). This
change adds a single parameter `timeout` which makes the args to
`connect` consistent with the legacy vectordb sdk.

BREAKING CHANGE: This is a breaking change b/c users who would have
previously been passing `connectionTimeout` will now be expected to pass
`timeout`.
---
 nodejs/lancedb/remote/client.ts     | 15 ++++++---------
 nodejs/lancedb/remote/connection.ts | 14 +++-----------
 2 files changed, 9 insertions(+), 20 deletions(-)

diff --git a/nodejs/lancedb/remote/client.ts b/nodejs/lancedb/remote/client.ts
index a8ccbf5c..4e4a92a3 100644
--- a/nodejs/lancedb/remote/client.ts
+++ b/nodejs/lancedb/remote/client.ts
@@ -27,8 +27,7 @@ export class RestfulLanceDBClient {
   #apiKey: string;
   #hostOverride?: string;
   #closed: boolean = false;
-  #connectionTimeout: number = 12 * 1000; // 12 seconds;
-  #readTimeout: number = 30 * 1000; // 30 seconds;
+  #timeout: number = 12 * 1000; // 12 seconds;
   #session?: import("axios").AxiosInstance;
 
   constructor(
@@ -36,15 +35,13 @@ export class RestfulLanceDBClient {
     apiKey: string,
     region: string,
     hostOverride?: string,
-    connectionTimeout?: number,
-    readTimeout?: number,
+    timeout?: number,
   ) {
     this.#dbName = dbName;
     this.#apiKey = apiKey;
     this.#region = region;
     this.#hostOverride = hostOverride ?? this.#hostOverride;
-    this.#connectionTimeout = connectionTimeout ?? this.#connectionTimeout;
-    this.#readTimeout = readTimeout ?? this.#readTimeout;
+    this.#timeout = timeout ?? this.#timeout;
   }
 
   // todo: cache the session.
@@ -59,7 +56,7 @@ export class RestfulLanceDBClient {
           Authorization: `Bearer ${this.#apiKey}`,
         },
         transformResponse: decodeErrorData,
-        timeout: this.#connectionTimeout,
+        timeout: this.#timeout,
       });
     }
   }
@@ -111,7 +108,7 @@ export class RestfulLanceDBClient {
         params,
       });
     } catch (e) {
-      if (e instanceof AxiosError) {
+      if (e instanceof AxiosError && e.response) {
         response = e.response;
       } else {
         throw e;
@@ -165,7 +162,7 @@ export class RestfulLanceDBClient {
         params: new Map(Object.entries(additional.params ?? {})),
       });
     } catch (e) {
-      if (e instanceof AxiosError) {
+      if (e instanceof AxiosError && e.response) {
         response = e.response;
       } else {
         throw e;
diff --git a/nodejs/lancedb/remote/connection.ts b/nodejs/lancedb/remote/connection.ts
index 20bc0458..4d914c54 100644
--- a/nodejs/lancedb/remote/connection.ts
+++ b/nodejs/lancedb/remote/connection.ts
@@ -20,8 +20,7 @@ export interface RemoteConnectionOptions {
   apiKey?: string;
   region?: string;
   hostOverride?: string;
-  connectionTimeout?: number;
-  readTimeout?: number;
+  timeout?: number;
 }
 
 export class RemoteConnection extends Connection {
@@ -33,13 +32,7 @@ export class RemoteConnection extends Connection {
 
   constructor(
     url: string,
-    {
-      apiKey,
-      region,
-      hostOverride,
-      connectionTimeout,
-      readTimeout,
-    }: RemoteConnectionOptions,
+    { apiKey, region, hostOverride, timeout }: RemoteConnectionOptions,
   ) {
     super();
     apiKey = apiKey ?? process.env.LANCEDB_API_KEY;
@@ -68,8 +61,7 @@ export class RemoteConnection extends Connection {
       this.#apiKey,
       this.#region,
       hostOverride,
-      connectionTimeout,
-      readTimeout,
+      timeout,
     );
   }