Merge pull request #16 from CartoDB/promisify-node-12

Throw exception when .acquire() resolves with error
CartoDB · May 13, 2020 · 5551826 · 5551826
2 parents 6224247 + 9c1cdae
commit 5551826
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 28 deletions.
diff --git a/index.js b/index.js
@@ -40,7 +40,7 @@ module.exports = class RedisPool extends EventEmitter {
 
         this.pools = {};
         this.options = Object.assign({}, DEFAULTS, options);
-        this.logger = logger
+        this.logger = logger;
 
         this._addCommands();
         this._emitStatus();
@@ -62,8 +62,18 @@ module.exports = class RedisPool extends EventEmitter {
         const client = await pool.acquire();
         const elapsedTime = Date.now() - startTime;
 
-        if (elapsedTime > this.options.slowPool.elapsedThreshold) {
-            this._log({ db: database, action: 'acquire', elapsed: elapsedTime, waiting: pool.pending });
+        if (this.options.slowPool.log && elapsedTime > this.options.slowPool.elapsedThreshold) {
+            this.logger.info({ name: this.options.name, db: database, action: 'acquire', elapsed: elapsedTime, waiting: pool.pending });
+        }
+
+        if (client instanceof Error) {
+            const err = client;
+            err.name = this.options.name;
+            err.db = database;
+            err.action = 'acquire';
+            this.logger.error(err);
+
+            throw err;
         }
 
         return client;
@@ -107,18 +117,9 @@ module.exports = class RedisPool extends EventEmitter {
         }, this._getStatusDelay());
     }
 
-    _getStatusDelay() {
+    _getStatusDelay () {
         return (this.options.emitter && this.options.emitter.statusInterval) || DEFAULT_STATUS_INTERVAL;
     }
-
-    _log(info) {
-        if (!this.options.slowPool.log) {
-            return;
-        }
-
-        info = Object.assign({ name: this.options.name }, info)
-        this.logger.error(JSON.stringify(info));
-    }
 };
 
 /**
@@ -139,28 +140,31 @@ function makePool (redisPool, database) {
                     no_ready_check: redisPool.options.noReadyCheck
                 });
 
-                client.on('error', function (err) {
-                    redisPool._log({ db: database, action: 'error', err: err.message });
+                client.on('error', (err) => {
+                    if (settled) {
+                        err.name = redisPool.options.name;
+                        return redisPool.logger.error(err);
+                    }
 
-                    if (!settled) {
-                        settled = true;
-                        client.end(FLUSH_CONNECTION);
+                    settled = true;
+                    client.end(FLUSH_CONNECTION);
 
-                        if (err) {
-                            return resolve(err);
-                        }
-                        return resolve(client);
+                    if (err) {
+                        return resolve(err);
                     }
+
+                    return resolve(client);
                 });
 
-                client.on('ready', function () {
+                client.on('ready', () => {
                     client.select(database, err => {
                         if (!settled) {
                             settled = true;
 
                             if (err) {
                                 return resolve(err);
                             }
+
                             return resolve(client);
                         }
                     });

diff --git a/test/index.js b/test/index.js
@@ -143,7 +143,7 @@ describe('RedisPool', function () {
         await redisPool.release(0, client2);
     });
 
-    it('log is called if elapsed time is above configured one', async function () {
+    it('logger is called if elapsed time is above configured one', async function () {
         let logWasCalled = false;
         const elapsedThreshold = 25;
         const enabledSlowPoolConfig = {
@@ -160,10 +160,10 @@ describe('RedisPool', function () {
         };
 
         const consoleLogFunc = console.error;
-        console.error = function (what) {
-            const whatObj = JSON.parse(what);
-            logWasCalled = whatObj && whatObj.action && whatObj.action === 'acquire';
-            consoleLogFunc.apply(console, arguments);
+        console.info = function (what) {
+            logWasCalled = what && what.action && what.action === 'acquire';
+            // uncomment the following line for debugging
+            // consoleLogFunc.apply(console, arguments);
         };
 
         // test