JSCO-16: Do not allow negative timeout values

Changes
=======
* Throw an Error if a negative timeout is provided
* Add tests to confirm functionality

lib/cluster.ts

@@ -279,6 +279,8 @@ export class Cluster {
 
     if (!options.timeoutOptions) {
       options.timeoutOptions = {}
+    } else {
+      this._validateTimeoutOptions(options.timeoutOptions)
     }
 
     this._connStr = connStr
@@ -353,6 +355,10 @@ export class Cluster {
       options = {}
     }
 
+    if (options.timeout && options.timeout < 0) {
+      throw new Error('timeout must be non-negative.')
+    }
+
     const exec = new QueryExecutor(this, options.abortSignal)
     return exec.query(statement, options)
   }
@@ -370,6 +376,41 @@ export class Cluster {
     }, callback)
   }
 
+  /**
+   * @internal
+   */
+  private _validateTimeoutOptions(timeoutOptions: TimeoutOptions): void {
+    if (timeoutOptions.connectTimeout && timeoutOptions.connectTimeout < 0) {
+      throw new Error('connectTimeout must be non-negative.')
+    }
+
+    if (timeoutOptions.dispatchTimeout && timeoutOptions.dispatchTimeout < 0) {
+      throw new Error('dispatchTimeout must be non-negative.')
+    }
+
+    if (
+      timeoutOptions.managementTimeout &&
+      timeoutOptions.managementTimeout < 0
+    ) {
+      throw new Error('managementTimeout must be non-negative.')
+    }
+
+    if (timeoutOptions.queryTimeout && timeoutOptions.queryTimeout < 0) {
+      throw new Error('queryTimeout must be non-negative.')
+    }
+
+    if (timeoutOptions.resolveTimeout && timeoutOptions.resolveTimeout < 0) {
+      throw new Error('resolveTimeout must be non-negative.')
+    }
+
+    if (
+      timeoutOptions.socketConnectTimeout &&
+      timeoutOptions.socketConnectTimeout < 0
+    ) {
+      throw new Error('socketConnectTimeout must be non-negative.')
+    }
+  }
+
   private _connect() {
     const dsnObj = ConnSpec.parse(this._connStr)
 
@@ -400,15 +441,15 @@ export class Cluster {
     const securityOpts: CppClusterSecurityOptions = {}
     if (this._securityOptions) {
       const trustOptionsCount =
-          (this._securityOptions.trustOnlyCapella ? 1 : 0) +
-          (this._securityOptions.trustOnlyPemFile ? 1 : 0) +
-          (this._securityOptions.trustOnlyPemString ? 1 : 0) +
-          (this._securityOptions.trustOnlyPlatform ? 1 : 0) +
-          (this._securityOptions.trustOnlyCertificates ? 1 : 0)
+        (this._securityOptions.trustOnlyCapella ? 1 : 0) +
+        (this._securityOptions.trustOnlyPemFile ? 1 : 0) +
+        (this._securityOptions.trustOnlyPemString ? 1 : 0) +
+        (this._securityOptions.trustOnlyPlatform ? 1 : 0) +
+        (this._securityOptions.trustOnlyCertificates ? 1 : 0)
 
       if (trustOptionsCount > 1) {
         throw new Error(
-            'Only one of trustOnlyCapella, trustOnlyPemFile, trustOnlyPemString, trustOnlyPlatform, or trustOnlyCertificates can be set.'
+          'Only one of trustOnlyCapella, trustOnlyPemFile, trustOnlyPemString, trustOnlyPlatform, or trustOnlyCertificates can be set.'
         )
       }
 

lib/scope.ts

@@ -78,6 +78,10 @@ export class Scope {
       options = {}
     }
 
+    if (options.timeout && options.timeout < 0) {
+      throw new Error('timeout must be non-negative.')
+    }
+
     const exec = new QueryExecutor(
       this.cluster,
       options.abortSignal,

test/cluster.test.js

@@ -20,7 +20,7 @@
 const assert = require('chai').assert
 const H = require('./harness')
 
-const { PassthroughDeserializer } = require("../lib/deserializers");
+const { PassthroughDeserializer } = require('../lib/deserializers')
 
 describe('#Cluster', function () {
   it('should correctly set timeouts', function () {
@@ -47,35 +47,104 @@ describe('#Cluster', function () {
     assert.equal(cluster.resolveTimeout, 30000)
   })
 
+  it('should raise error on negative connectTimeout', function () {
+    let options = {
+      timeoutOptions: {
+        connectTimeout: -1,
+      },
+    }
+
+    H.throwsHelper(() => {
+      H.lib.Cluster.createInstance(H.connStr, H.credentials, options)
+    }, Error)
+  })
+
+  it('should raise error on negative dispatchTimeout', function () {
+    let options = {
+      timeoutOptions: {
+        dispatchTimeout: -1,
+      },
+    }
+
+    H.throwsHelper(() => {
+      H.lib.Cluster.createInstance(H.connStr, H.credentials, options)
+    }, Error)
+  })
+
+  it('should raise error on negative managementTimeout', function () {
+    let options = {
+      timeoutOptions: {
+        managementTimeout: -1,
+      },
+    }
+
+    H.throwsHelper(() => {
+      H.lib.Cluster.createInstance(H.connStr, H.credentials, options)
+    }, Error)
+  })
+
+  it('should raise error on negative queryTimeout', function () {
+    let options = {
+      timeoutOptions: {
+        queryTimeout: -1,
+      },
+    }
+
+    H.throwsHelper(() => {
+      H.lib.Cluster.createInstance(H.connStr, H.credentials, options)
+    }, Error)
+  })
+
+  it('should raise error on negative resolveTimeout', function () {
+    let options = {
+      timeoutOptions: {
+        resolveTimeout: -1,
+      },
+    }
+
+    H.throwsHelper(() => {
+      H.lib.Cluster.createInstance(H.connStr, H.credentials, options)
+    }, Error)
+  })
+
+  it('should raise error on negative socketConnectTimeout', function () {
+    let options = {
+      timeoutOptions: {
+        socketConnectTimeout: -1,
+      },
+    }
+
+    H.throwsHelper(() => {
+      H.lib.Cluster.createInstance(H.connStr, H.credentials, options)
+    }, Error)
+  })
+
   it('should correctly set security options', function () {
     let options = {
       securityOptions: {
         trustOnlyPlatform: true,
         verifyServerCertificates: false,
-        cipherSuites: ["suite"]
-      }
+        cipherSuites: ['suite'],
+      },
     }
 
     const cluster = H.lib.Cluster.createInstance(
-        H.connStr,
-        H.credentials,
-        options
+      H.connStr,
+      H.credentials,
+      options
     )
 
     assert.isTrue(cluster._securityOptions.trustOnlyPlatform)
     assert.isFalse(cluster._securityOptions.verifyServerCertificates)
-    assert.deepEqual(cluster._securityOptions.cipherSuites, ["suite"])
+    assert.deepEqual(cluster._securityOptions.cipherSuites, ['suite'])
     assert.isUndefined(cluster._securityOptions.trustOnlyCapella)
     assert.isUndefined(cluster._securityOptions.trustOnlyPemFile)
     assert.isUndefined(cluster._securityOptions.trustOnlyCertificates)
     assert.isUndefined(cluster._securityOptions.trustOnlyPemString)
   })
 
   it('should default to trustOnlyCapella if no options are set', function () {
-    const cluster = H.lib.Cluster.createInstance(
-        H.connStr,
-        H.credentials,
-    )
+    const cluster = H.lib.Cluster.createInstance(H.connStr, H.credentials)
 
     assert.isTrue(cluster._securityOptions.trustOnlyCapella)
     assert.isUndefined(cluster._securityOptions.trustOnlyPemFile)
@@ -90,48 +159,44 @@ describe('#Cluster', function () {
     let options = {
       securityOptions: {
         trustOnlyCapella: true,
-        trustOnlyPemFile: "pemFile",
-      }
+        trustOnlyPemFile: 'pemFile',
+      },
     }
 
     H.throwsHelper(() => {
-      H.lib.Cluster.createInstance(
-          H.connStr,
-          H.credentials,
-          options
-      )
+      H.lib.Cluster.createInstance(H.connStr, H.credentials, options)
     }, Error)
   })
 
   it('should correctly set dns options', function () {
     let options = {
       dnsConfig: {
-        nameserver: "localhost",
+        nameserver: 'localhost',
         port: 12345,
-        dnsSrvTimeout: 3000
-      }
+        dnsSrvTimeout: 3000,
+      },
     }
 
     const cluster = H.lib.Cluster.createInstance(
-        H.connStr,
-        H.credentials,
-        options
+      H.connStr,
+      H.credentials,
+      options
     )
 
-    assert.strictEqual(cluster._dnsConfig.nameserver, "localhost")
+    assert.strictEqual(cluster._dnsConfig.nameserver, 'localhost')
     assert.strictEqual(cluster._dnsConfig.port, 12345)
     assert.strictEqual(cluster._dnsConfig.dnsSrvTimeout, 3000)
   })
 
-  it ('should correctly set cluster-level deserializer', function () {
+  it('should correctly set cluster-level deserializer', function () {
     let options = {
-      deserializer: new PassthroughDeserializer()
+      deserializer: new PassthroughDeserializer(),
     }
 
     const cluster = H.lib.Cluster.createInstance(
-        H.connStr,
-        H.credentials,
-        options
+      H.connStr,
+      H.credentials,
+      options
     )
     assert.instanceOf(cluster.deserializer, PassthroughDeserializer)
   })

test/query.test.js

@@ -126,6 +126,14 @@ function genericTests(instance) {
       assert.isTrue(results.at(0)['$1'])
     })
 
+    it('should should raise error on negative timeout', async function () {
+      await H.throwsHelper(async () => {
+        await instance().executeQuery("SELECT 'FOO' AS message", {
+          timeout: -1,
+        })
+      }, Error)
+    })
+
     it('should work with positional parameters', async function () {
       const results = []
       const qs = `SELECT $2=1`