Set max session idle time (#642)

* set max session idle time

* get rid of auto formatter changes

* get rid of auto formatter changes take 2

* handle negative values

* fix for node 12 syntax compatability

* fix

* fix comment

* remove logging and clarify documentation

* add stream test

* Beef up test a little.

* prep for release

* documentation changes

* update test

* simplify getHttp2IdleTime

* fix test flakiness

Co-authored-by: Cleve Stuart <cleve.stuart@fauna.com>

CHANGELOG.md

@@ -1,3 +1,8 @@
+## 4.6.0
+- Enforce a maximum value of 5000 ms for the `http2SessionIdleTime` option [#642](https://github.com/fauna/faunadb-js/pull/642)
+- Add checks to `http2SessionIdleTime` so that sane defaults are used in case an invalid value is configured
+- Add the missing Native.ROLES type [#638](https://github.com/fauna/faunadb-js/pull/638)
+
 ## 4.5.4
 - Disable ability to configure a client and the query method from returning metrics when calling query - fixing bug introduced in 4.5.3 that breaks backward compatibility. Continue supporting queryWithMetrics. [#633](https://github.com/fauna/faunadb-js/pull/633).
 

README.md

@@ -155,7 +155,9 @@ time, and transaction retires consumed by your query:
   } // usage data
 }
 ```
+
 Metrics returned in the response will be of `number` data type.
+
 #### Pagination Helpers
 
 This driver contains helpers to provide a simpler API for consuming paged
@@ -280,14 +282,17 @@ have been resolved, the client will keep the session open for a period of time
 (500ms by default) to be reused for any new requests.
 
 The `http2SessionIdleTime` parameter may be used to control how long the HTTP/2
-session remains open while the connection is idle. To save on the overhead of
-closing and re-opening the session, set `http2SessionIdleTime` to a longer time
---- or even `Infinity`, to keep the session alive indefinitely.
+session remains open while the query connection is idle. To save on the overhead of
+closing and re-opening the session, set `http2SessionIdleTime` to a longer time.
+The default value is 500ms and the maximum value is 5000ms.
 
-While an HTTP/2 session is alive, the client will hold the Node.js event loop
+Note that `http2SessionIdleTime` has no effect on a stream connection: a stream
+is a long-lived connection that is intended to be held open indefinitely.
+
+While an HTTP/2 session is alive, the client holds the Node.js event loop
 open; this prevents the process from terminating. Call `Client#close` to manually
 close the session and allow the process to terminate. This is particularly
-important if `http2SessionIdleTime` is long or `Infinity`:
+important if `http2SessionIdleTime` is long:
 
 ```javascript
 // sample.js (run it with "node sample.js" command)
@@ -296,21 +301,18 @@ const { Client, query: Q } = require('faunadb')
 async function main() {
   const client = new Client({
     secret: 'YOUR_FAUNADB_SECRET',
-    http2SessionIdleTime: Infinity,
-    //                    ^^^ Infinity or non-negative integer
+    http2SessionIdleTime: 1000, // Must be a non-negative integer
   })
   const output = await client.query(Q.Add(1, 1))
 
   console.log(output)
 
   client.close()
-  //     ^^^ If it's not called then the process won't terminate
 }
 
 main().catch(console.error)
 ```
 
-
 ## Known issues
 
 ### Using with Cloudflare Workers

package.json

@@ -1,6 +1,6 @@
 {
   "name": "faunadb",
-  "version": "4.5.4",
+  "version": "4.6.0",
   "apiVersion": "4",
   "description": "FaunaDB Javascript driver for Node.JS and Browsers",
   "homepage": "https://fauna.com",

src/Client.js

@@ -158,9 +158,10 @@ var values = require('./values')
  *   Sets the maximum amount of time (in milliseconds) for query execution on the server
  * @param {?number} options.http2SessionIdleTime
  *   Sets the maximum amount of time (in milliseconds) an HTTP2 session may live
- *   when there's no activity. Must either be a non-negative integer, or Infinity to allow the
- *   HTTP2 session to live indefinitely (use `Client#close` to manually terminate the client).
- *   Only applicable for NodeJS environment (when http2 module is used). Default is 500ms;
+ *   when there's no activity. Must be a non-negative integer, with a maximum value of 5000.
+ *   If an invalid value is passed a default of 500 ms is applied. If a value 
+ *   exceeding 5000 ms is passed (e.g. Infinity) the maximum of 5000 ms is applied.
+ *   Only applicable for NodeJS environment (when http2 module is used).
  *   can also be configured via the FAUNADB_HTTP2_SESSION_IDLE_TIME environment variable
  *   which has the highest priority and overrides the option passed into the Client constructor.
  * @param {?boolean} options.checkNewVersion
@@ -169,7 +170,11 @@ var values = require('./values')
  *   Disabled by default. Controls whether or not query metrics are returned.
  */
 function Client(options) {
-  var http2SessionIdleTime = getHttp2SessionIdleTime()
+  const http2SessionIdleTime = getHttp2SessionIdleTime(
+    options ? options.http2SessionIdleTime : undefined
+  )
+
+  if (options) options.http2SessionIdleTime = http2SessionIdleTime
 
   options = util.applyDefaults(options, {
     domain: 'db.fauna.com',
@@ -182,14 +187,10 @@ function Client(options) {
     headers: {},
     fetch: undefined,
     queryTimeout: null,
-    http2SessionIdleTime: http2SessionIdleTime.value,
+    http2SessionIdleTime,
     checkNewVersion: false,
   })
 
-  if (http2SessionIdleTime.shouldOverride) {
-    options.http2SessionIdleTime = http2SessionIdleTime.value
-  }
-
   this._observer = options.observer
   this._http = new http.HttpClient(options)
   this.stream = stream.StreamAPI(this)
@@ -384,17 +385,29 @@ Client.prototype._handleRequestResult = function(response, result, options) {
   errors.FaunaHTTPError.raiseForStatusCode(result)
 }
 
-function getHttp2SessionIdleTime() {
-  var fromEnv = util.getEnvVariable('FAUNADB_HTTP2_SESSION_IDLE_TIME')
-  var parsed =
-    // Allow either "Infinity" or parsable integer string.
-    fromEnv === 'Infinity' ? Infinity : parseInt(fromEnv, 10)
-  var useEnvVar = !isNaN(parsed)
+function getHttp2SessionIdleTime(configuredIdleTime) {
+  const maxIdleTime = 5000
+  const defaultIdleTime = 500
+  const envIdleTime = util.getEnvVariable('FAUNADB_HTTP2_SESSION_IDLE_TIME')
 
-  return {
-    shouldOverride: useEnvVar,
-    value: useEnvVar ? parsed : 500,
+  var value = defaultIdleTime
+  // attemp to set the idle time to the env value and then the configured value
+  const values = [envIdleTime, configuredIdleTime]
+  for (const rawValue of values) {
+    const parsedValue = rawValue === 'Infinity'
+      ? Number.MAX_SAFE_INTEGER
+      : parseInt(rawValue, 10)
+    const isNegative = parsedValue < 0
+    const isGreaterThanMax = parsedValue > maxIdleTime
+    // if we didn't get infinity or a positive integer move to the next value
+    if (isNegative || !parsedValue) continue
+    // if we did get something valid constrain it to the ceiling
+    value = parsedValue
+    if (isGreaterThanMax) value = maxIdleTime
+    break
   }
+
+  return value
 }
 
 module.exports = Client

test/client.test.js

@@ -9,14 +9,20 @@ var json = require('../src/_json')
 var client
 
 describe('Client', () => {
+  const env = process.env
+
   beforeAll(() => {
     // Hideous way to ensure that the client is initialized.
     client = util.client()
-
     return client.query(query.CreateCollection({ name: 'my_collection' }))
   })
 
+  beforeEach(() => {
+    process.env = { ...env }
+  })
+
   afterEach(() => {
+    process.env = env
     util.clearBrowserSimulation()
   })
 
@@ -65,7 +71,6 @@ describe('Client', () => {
       toEqual(['metrics', 'value'])
   })
 
-
   test('paginates', () => {
     return createDocument().then(function(document) {
       return client.paginate(document.ref).each(function(page) {
@@ -176,7 +181,7 @@ describe('Client', () => {
 
   test('Client#close call on Http2Adapter-based Client', async () => {
     const client = util.getClient({
-      http2SessionIdleTime: Infinity,
+      http2SessionIdleTime: 5000,
     })
 
     await client.ping()
@@ -412,6 +417,115 @@ describe('Client', () => {
       requiredKeys.every(key => driverEnvHeader.includes(key))
     ).toBeDefined()
   })
+
+  test('http2SessionIdleTime env overrides client config', async () => {
+    var client
+    var internalIdleTime
+    const maxIdleTime = 5000
+    const defaultIdleTime = 500
+
+    process.env.FAUNADB_HTTP2_SESSION_IDLE_TIME = '999'
+    client = util.getClient({
+      http2SessionIdleTime: 2500,
+    })
+    internalIdleTime = client._http._adapter._http2SessionIdleTime
+    expect(internalIdleTime).toBe(999)
+
+    process.env.FAUNADB_HTTP2_SESSION_IDLE_TIME = maxIdleTime + 1
+    client = util.getClient({
+      http2SessionIdleTime: 2500,
+    })
+    internalIdleTime = client._http._adapter._http2SessionIdleTime
+    expect(internalIdleTime).toBe(maxIdleTime)
+
+    process.env.FAUNADB_HTTP2_SESSION_IDLE_TIME = 'Infinity'
+    client = util.getClient({
+      http2SessionIdleTime: 2500,
+    })
+    internalIdleTime = client._http._adapter._http2SessionIdleTime
+    expect(internalIdleTime).toBe(maxIdleTime)
+
+    process.env.FAUNADB_HTTP2_SESSION_IDLE_TIME = 'Cat'
+    client = util.getClient({
+      http2SessionIdleTime: 2500,
+    })
+    internalIdleTime = client._http._adapter._http2SessionIdleTime
+    expect(internalIdleTime).toBe(2500)
+
+    process.env.FAUNADB_HTTP2_SESSION_IDLE_TIME = 'Cat'
+    client = util.getClient({
+      http2SessionIdleTime: "Cat",
+    })
+    internalIdleTime = client._http._adapter._http2SessionIdleTime
+    expect(internalIdleTime).toBe(defaultIdleTime)
+
+    process.env.FAUNADB_HTTP2_SESSION_IDLE_TIME = '-999'
+    client = util.getClient({
+      http2SessionIdleTime: 2500,
+    })
+    internalIdleTime = client._http._adapter._http2SessionIdleTime
+    expect(internalIdleTime).toBe(2500)
+
+    process.env.FAUNADB_HTTP2_SESSION_IDLE_TIME = '-999'
+    client = util.getClient({
+      http2SessionIdleTime: -999,
+    })
+    internalIdleTime = client._http._adapter._http2SessionIdleTime
+    expect(internalIdleTime).toBe(defaultIdleTime)
+
+    process.env.FAUNADB_HTTP2_SESSION_IDLE_TIME = '-999'
+    client = util.getClient({
+      http2SessionIdleTime: "Infinity",
+    })
+    internalIdleTime = client._http._adapter._http2SessionIdleTime
+    expect(internalIdleTime).toBe(maxIdleTime)
+  })
+
+  test('http2SessionIdleTime respects the max and default', async () => {
+    var client
+    var internalIdleTime
+    const maxIdleTime = 5000
+    const defaultIdleTime = 500
+    process.env.FAUNADB_HTTP2_SESSION_IDLE_TIME = undefined
+
+    client = util.getClient({
+      http2SessionIdleTime: 'Infinity',
+    })
+    internalIdleTime = client._http._adapter._http2SessionIdleTime
+    expect(internalIdleTime).toBe(maxIdleTime)
+
+    client = util.getClient({
+      http2SessionIdleTime: maxIdleTime + 1,
+    })
+    internalIdleTime = client._http._adapter._http2SessionIdleTime
+    expect(internalIdleTime).toBe(maxIdleTime)
+
+    client = util.getClient({})
+    internalIdleTime = client._http._adapter._http2SessionIdleTime
+    expect(internalIdleTime).toBe(defaultIdleTime)
+
+    client = util.getClient({ http2SessionIdleTime: null })
+    internalIdleTime = client._http._adapter._http2SessionIdleTime
+    expect(internalIdleTime).toBe(defaultIdleTime)
+
+    client = util.getClient({
+      http2SessionIdleTime: 'Cat',
+    })
+    internalIdleTime = client._http._adapter._http2SessionIdleTime
+    expect(internalIdleTime).toBe(defaultIdleTime)
+
+    client = util.getClient({
+      http2SessionIdleTime: 2500,
+    })
+    internalIdleTime = client._http._adapter._http2SessionIdleTime
+    expect(internalIdleTime).toBe(2500)
+
+    client = util.getClient({
+      http2SessionIdleTime: -2500,
+    })
+    internalIdleTime = client._http._adapter._http2SessionIdleTime
+    expect(internalIdleTime).toBe(defaultIdleTime)
+  })
 })
 
 function assertObserverStats(metrics, name) {

test/stream.test.js

@@ -273,6 +273,48 @@ describe('StreamAPI', () => {
         })
         .start()
     })
+
+    test.each([50, 500, 5000])('stays open beyond the value set for http2SessionIdleTime', async (idleTime) => {
+      const padding = 100
+      let seen_event = false
+      const client = util.getClient({
+        secret: key.secret,
+        http2SessionIdleTime: idleTime,
+      })
+      const oldTimestamp = doc.ts
+
+      const getNumberOfSessions = () => Object.keys(client._http._adapter._sessionMap).length
+
+      stream = client.stream
+      .document(doc.ref)
+      .on('version', (event) => {
+        seen_event = true
+        expect(event.action).toEqual("update")
+        expect(event.document.ts).toBeGreaterThan(oldTimestamp)
+      })
+
+      stream.start()
+
+      await util.delay(idleTime + padding)
+      expect(getNumberOfSessions()).toBe(1)
+
+      // this will create a new session as it is not a stream
+      const { ts: newTimestamp } = await client.query(q.Update(doc.ref, {}))
+      expect(newTimestamp).toBeGreaterThan(oldTimestamp)
+
+      await util.delay(idleTime + padding)
+      expect(getNumberOfSessions()).toBeGreaterThanOrEqual(1)
+      expect(seen_event).toBe(true)
+
+      seen_event = false
+      await util.delay(idleTime + padding)
+      expect(getNumberOfSessions()).toBeGreaterThanOrEqual(1)
+      expect(seen_event).toBe(false)
+
+      stream.close()
+      await util.delay(idleTime + padding)
+      expect(getNumberOfSessions()).toBe(0)
+    }, 30000);
   })
 
   describe('document', () => {