feat: parse info data as numbers if possible and improve parsing

benchmarks/multi_bench.js

@@ -82,7 +82,7 @@ Test.prototype.newClient = function (id) {
       console.log([
         `clients: ${numClients}`,
         `NodeJS: ${process.versions.node}`,
-        `Redis: ${newClient.serverInfo.redis_version}`,
+        `Redis: ${newClient.serverInfo.server.redis_version}`,
         `connected by: ${clientOptions.path ? 'socket' : 'tcp'}`
       ].join(', '))
       versionsLogged = true

changelog.md

@@ -47,6 +47,8 @@ Breaking Changes
 - Changed the name of two functions
   - `restore-asking` is now `restore_asking_`
   - `host:` is now `host`
+- Changed the `serverInfo` into a nested object and to parse numbers
+- Changed the `serverInfo.versions` to `serverInfo.version`
 - Using `.end` without the flush parameter is now going to throw an TypeError
 - Only emit ready when all commands were truly send to Redis
 

index.js

@@ -1,6 +1,6 @@
 'use strict'
 
-// TODO: Replace all `Error` with `RedisError` and improve errors in general
+// TODO: Improve errors in general
 // We have to replace the error codes and make them coherent.
 // We also have to use InterruptError s instead of AbortError s.
 // The Error messages might be improved as well.

lib/individualCommands.js

@@ -116,36 +116,54 @@ Multi.prototype.quit = function quit () {
 function infoCallback (client) {
   return function (err, res) {
     if (err) {
-      client.serverInfo = {}
       return err
     }
 
-    const obj = {}
+    if (typeof res !== 'string') {
-    const lines = res.toString().split('\r\n')
+      res = res.toString()
-    var line, parts, subParts
+    }
 
-    for (var i = 0; i < lines.length; i++) {
+    const obj = {}
-      parts = lines[i].split(':')
+    const lines = res.split('\r\n')
-      if (parts[1]) {
+    var topic = ''
-        if (parts[0].indexOf('db') === 0) {
+
-          subParts = parts[1].split(',')
+    while (lines.length) {
-          obj[parts[0]] = {}
+      const parts = lines.shift().split(':')
-          for (line = subParts.pop(); line !== undefined; line = subParts.pop()) {
+      const key = parts[0]
-            line = line.split('=')
+      if (parts.length === 1) {
-            obj[parts[0]][line[0]] = +line[1]
+        if (key !== '') {
+          topic = key[2].toLowerCase() + key.substr(3)
+          obj[topic] = {}
         }
       } else {
-          obj[parts[0]] = parts[1]
+        const value = parts[1]
+        const part = obj[topic]
+        if (value === '') {
+          part[key] = ''
+        } else if (topic === 'keyspace' || topic === 'commandstats') {
+          const subParts = value.split(',')
+          part[key] = {}
+          while (subParts.length) {
+            const line = subParts.shift().split('=')
+            part[key][line[0]] = +line[1]
+          }
+        } else {
+          const num = +value
+          // A fast Number.isNaN check
+          // eslint-disable-next-line no-self-compare
+          if (num === num) {
+            part[key] = num
+          } else {
+            part[key] = value
           }
         }
       }
-    obj.versions = []
-    if (obj.redis_version) {
-      obj.redis_version.split('.').forEach((num) => {
-        obj.versions.push(+num)
-      })
     }
-    // Expose info key/values to users
+
+    if (obj.server && obj.server.redis_version) {
+      obj.server.version = obj.server.redis_version.split('.').map(Number)
+    }
+
     client.serverInfo = obj
     return res
   }

lib/readyHandler.js

@@ -126,10 +126,12 @@ function readyCheck (client) {
       return
     }
 
-    if (!client.serverInfo.loading || client.serverInfo.loading === '0') {
+    const persistence = client.serverInfo.persistence
+    if (persistence === undefined || persistence.loading === undefined || persistence.loading === 0) {
       // If the master_link_status exists but the link is not up, try again after 50 ms
-      if (client.serverInfo.master_link_status && client.serverInfo.master_link_status !== 'up') {
+      const replication = client.serverInfo.replication
-        client.serverInfo.loading_eta_seconds = 0.05
+      if (replication && typeof replication.master_link_status === 'string' && replication.master_link_status !== 'up') {
+        persistence.loading_eta_seconds = 0.05
       } else {
         // Eta loading should change
         debug('Redis server ready.')
@@ -138,7 +140,7 @@ function readyCheck (client) {
       }
     }
 
-    var retryTime = +client.serverInfo.loading_eta_seconds * 1000
+    var retryTime = +persistence.loading_eta_seconds * 1000
     if (retryTime > 1000) {
       retryTime = 1000
     }

test/auth.spec.js

@@ -80,10 +80,11 @@ if (process.platform !== 'win32') {
               // Set a key so the used database is returned in the info command
               promises.push(client.set('foo', 'bar'))
               promises.push(client.get('foo'))
-              assert.strictEqual(client.serverInfo.db2, undefined)
+              const space = client.serverInfo.keyspace
+              assert.strictEqual(space && space.db2, undefined)
               // Using the info command should update the serverInfo
               promises.push(client.info().then(() => {
-                assert(typeof client.serverInfo.db2 === 'object')
+                assert.strictEqual(typeof client.serverInfo.keyspace.db2, 'object')
               }))
               promises.push(client.flushdb())
               return Promise.all(promises).then(() => done())

test/commands/info.spec.js

@@ -22,11 +22,11 @@ describe('The \'info\' method', () => {
       it('update serverInfo after a info command', () => {
         client.set('foo', 'bar')
         return client.info().then(() => {
-          assert.strictEqual(client.serverInfo.db2, undefined)
+          assert.strictEqual(client.serverInfo.keyspace.db2, undefined)
           client.select(2)
           client.set('foo', 'bar')
           return client.info().then(() => {
-            assert.strictEqual(typeof client.serverInfo.db2, 'object')
+            assert.strictEqual(typeof client.serverInfo.keyspace.db2, 'object')
           })
         })
       })
@@ -35,14 +35,15 @@ describe('The \'info\' method', () => {
         client.set('foo', 'bar')
         client.info('keyspace')
         return client.select(2).then(() => {
-          assert.strictEqual(Object.keys(client.serverInfo).length, 2, 'Key length should be three')
+          assert.strictEqual(Object.keys(client.serverInfo).length, 1, 'Key length should be one')
-          assert.strictEqual(typeof client.serverInfo.db0, 'object', 'db0 keyspace should be an object')
+          assert.strictEqual(Object.keys(client.serverInfo.keyspace.db0).length, 3, 'Key length should be three')
+          assert.strictEqual(typeof client.serverInfo.keyspace.db0, 'object', 'db0 keyspace should be an object')
           client.info(['keyspace'])
           client.set('foo', 'bar')
           return client.info('all').then((res) => {
             assert(Object.keys(client.serverInfo).length > 3, 'Key length should be way above three')
-            assert.strictEqual(typeof client.serverInfo.redis_version, 'string')
+            assert.strictEqual(typeof client.serverInfo.server.redis_version, 'string')
-            assert.strictEqual(typeof client.serverInfo.db2, 'object')
+            assert.strictEqual(typeof client.serverInfo.keyspace.db2, 'object')
           })
         })
       })

test/commands/select.spec.js

@@ -67,7 +67,7 @@ describe('The \'select\' method', () => {
             client.set('foo', 'bar').then(() => client._stream.destroy())
             client.once('ready', () => {
               assert.strictEqual(client.selectedDb, 3)
-              assert(typeof client.serverInfo.db3 === 'object')
+              assert(typeof client.serverInfo.keyspace.db3 === 'object')
               done()
             })
           })

test/conect.slave.spec.js

@@ -66,8 +66,8 @@ if (process.platform !== 'win32') {
       const end = helper.callFuncAfter(done, 2)
 
       slave.on('ready', function () {
-        assert.strictEqual(this.serverInfo.master_link_status, 'up')
+        assert.strictEqual(this.serverInfo.replication.master_link_status, 'up')
-        assert.strictEqual(firstInfo.master_link_status, 'down')
+        assert.strictEqual(firstInfo.replication.master_link_status, 'down')
         assert(i > 1)
         this.get('foo300').then((res) => {
           assert.strictEqual(res.substr(0, 3), 'bar')

test/connection.spec.js

@@ -479,8 +479,8 @@ describe('connection tests', () => {
           client.info = function () {
             return tmp().then((res) => {
               if (!delayed) {
-                client.serverInfo.loading = 1
+                client.serverInfo.persistence.loading = 1
-                client.serverInfo.loading_eta_seconds = 0.5
+                client.serverInfo.persistence.loading_eta_seconds = 0.5
                 delayed = true
                 time = Date.now()
               }
@@ -509,8 +509,8 @@ describe('connection tests', () => {
             return tmp().then((res) => {
               if (!delayed) {
                 // Try reconnecting after one second even if redis tells us the time needed is above one second
-                client.serverInfo.loading = 1
+                client.serverInfo.persistence.loading = 1
-                client.serverInfo.loading_eta_seconds = 2.5
+                client.serverInfo.persistence.loading_eta_seconds = 2.5
                 delayed = true
                 time = Date.now()
               }

test/helper.js

@@ -139,7 +139,7 @@ module.exports = {
       throw new Error('Version check not possible as the client is not yet ready or did not expose the version')
     }
     // Return true if the server version >= desiredVersion
-    const version = connection.serverInfo.versions
+    const version = connection.serverInfo.server.version
     for (let i = 0; i < 3; i++) {
       if (version[i] > desiredVersion[i]) {
         return true

test/multi.spec.js

@@ -15,7 +15,7 @@ describe('The \'multi\' method', () => {
   })
 
   describe('regression test', () => {
-    it('saved buffers with charsets different than utf-8 (issue #913)', function (done) {
+    it('saved buffers with a charset different than utf-8 (issue #913)', function (done) {
       this.timeout(12000) // Windows tests on 0.10 are slow
       client = redis.createClient()
 
@@ -226,13 +226,13 @@ describe('The \'multi\' method', () => {
           return multi.exec().then(helper.isDeepEqual([]))
         })
 
-        it('runs normal calls in-between multis', () => {
+        it('runs normal calls in-between multi commands', () => {
           const multi1 = client.multi()
           multi1.set('m1', '123')
           return client.set('m2', '456')
         })
 
-        it('runs simultaneous multis with the same client', () => {
+        it('runs simultaneous multi commands with the same client', () => {
           const multi1 = client.multi()
           multi1.set('m1', '123')
           multi1.get('m1')
@@ -247,7 +247,7 @@ describe('The \'multi\' method', () => {
           ])
         })
 
-        it('runs simultaneous multis with the same client version 2', () => {
+        it('runs simultaneous multi commands with the same client version 2', () => {
           const multi2 = client.multi()
           const multi1 = client.multi()
 
@@ -512,7 +512,7 @@ describe('The \'multi\' method', () => {
           return multi.exec().then((res) => {
             res[2] = res[2].substr(0, 10)
             assert.strictEqual(client.selectedDb, 5)
-            assert.deepStrictEqual(client.serverInfo.db5, { avg_ttl: 0, expires: 0, keys: 1 })
+            assert.deepStrictEqual(client.serverInfo.keyspace.db5, { avg_ttl: 0, expires: 0, keys: 1 })
             assert.deepStrictEqual(res, ['OK', 'OK', '# Server\r\n', 'bar'])
             return client.flushdb()
           })

test/node_redis.spec.js

@@ -7,6 +7,7 @@ const path = require('path')
 const config = require('./lib/config')
 const helper = require('./helper')
 const fork = require('child_process').fork
+const Errors = require('redis-errors')
 const redis = config.redis
 let client
 
@@ -20,7 +21,7 @@ describe('The nodeRedis client', () => {
       const multiPrototype = data.match(/(\n| = )Multi\.prototype\.[a-z][a-zA-Z_]+/g)
       // Check that every entry RedisClient entry has a correspondent Multi entry
       assert.strictEqual(clientPrototype.filter((entry) => {
-        return !multiPrototype.includes(entry.replace('RedisClient', 'Multi'))
+        return multiPrototype.indexOf(entry.replace('RedisClient', 'Multi')) === -1
       }).length, 0)
       assert.strictEqual(clientPrototype.length, multiPrototype.length)
       // Check that all entries exist only in lowercase variants
@@ -82,10 +83,11 @@ describe('The nodeRedis client', () => {
               }
             }
             client2.on('error', (err) => {
-              assert.strictEqual(err.message, 'Connection forcefully ended and command aborted. It might have been processed.')
+              assert.strictEqual(err.message, 'Connection forcefully ended and command aborted.')
               assert.strictEqual(err.command, 'SELECT')
-              assert(err instanceof Error)
+              assert(err instanceof Errors.AbortError)
-              assert.strictEqual(err.name, 'AbortError')
+              assert(err instanceof Errors.InterruptError)
+              assert.strictEqual(err.name, 'InterruptError')
             })
             client2.on('ready', () => {
               client2.end(true)
@@ -178,7 +180,7 @@ describe('The nodeRedis client', () => {
           })
 
           it('using multi with sendCommand should work as individual command instead of using the internal multi', () => {
-            // This is necessary to keep backwards compatibility and it is the only way to handle multis as you want in nodeRedis
+            // This is necessary to keep backwards compatibility and it is the only way to handle multi as you want in nodeRedis
             client.sendCommand('multi')
             client.sendCommand('set', ['foo', 'bar']).then(helper.isString('QUEUED'))
             client.get('foo')
@@ -333,8 +335,8 @@ describe('The nodeRedis client', () => {
                 const end = helper.callFuncAfter(() => {
                   client.removeListener('connect', onConnect)
                   client.removeListener('reconnecting', onRecon)
-                  assert.strictEqual(client.serverInfo.db0.keys, 2)
+                  assert.strictEqual(client.serverInfo.keyspace.db0.keys, 2)
-                  assert.strictEqual(Object.keys(client.serverInfo.db0).length, 3)
+                  assert.strictEqual(Object.keys(client.serverInfo.keyspace.db0).length, 3)
                   done()
                 }, 4)
                 client.get('recon 1').then(helper.isString('one')).then(end)
@@ -495,9 +497,9 @@ describe('The nodeRedis client', () => {
           })
           client.once('ready', () => {
             client.set('foo', 'bar').then(helper.fail, (err) => {
-              assert.strictEqual(err.message, 'Fatal error encountered. Command aborted. It might have been processed.')
+              assert.strictEqual(err.message, 'Fatal error encountered. Command aborted.')
               assert.strictEqual(err.code, 'NR_FATAL')
-              assert(err instanceof redis.AbortError)
+              assert(err instanceof redis.InterruptError)
               error = err.origin
             })
             // Make sure we call execute out of the reply

test/prefix.spec.js

@@ -34,9 +34,9 @@ describe('prefix key names', () => {
           client.mset('key2', 'value2', 'key3', 'value3'),
           client.keys('*').then((res) => {
             assert.strictEqual(res.length, 3)
-            assert(res.includes('test:prefix:key'))
+            assert(res.indexOf('test:prefix:key') !== -1)
-            assert(res.includes('test:prefix:key2'))
+            assert(res.indexOf('test:prefix:key2') !== -1)
-            assert(res.includes('test:prefix:key3'))
+            assert(res.indexOf('test:prefix:key3') !== -1)
           })
         ])
       })
@@ -55,9 +55,9 @@ describe('prefix key names', () => {
           const prefixes = res.pop()
           assert.deepStrictEqual(res, ['OK', 'value', 'alue', 1, 0, 'OK'])
           assert.strictEqual(prefixes.length, 3)
-          assert(prefixes.includes('test:prefix:key'))
+          assert(prefixes.indexOf('test:prefix:key') !== -1)
-          assert(prefixes.includes('test:prefix:key2'))
+          assert(prefixes.indexOf('test:prefix:key2') !== -1)
-          assert(prefixes.includes('test:prefix:key3'))
+          assert(prefixes.indexOf('test:prefix:key3') !== -1)
         })
       })
 
@@ -75,9 +75,9 @@ describe('prefix key names', () => {
           const prefixes = res.pop()
           assert.deepStrictEqual(res, ['OK', 'value', 'alue', 1, 0, 'OK'])
           assert.strictEqual(prefixes.length, 3)
-          assert(prefixes.includes('test:prefix:key'))
+          assert(prefixes.indexOf('test:prefix:key') !== -1)
-          assert(prefixes.includes('test:prefix:key2'))
+          assert(prefixes.indexOf('test:prefix:key2') !== -1)
-          assert(prefixes.includes('test:prefix:key3'))
+          assert(prefixes.indexOf('test:prefix:key3') !== -1)
         })
       })
     })