fix: force options to guarantee correct reconnects

BREAKING CHANGE:
If redis opts are missing:
 { maxRetriesPerRequest: null,
   enableReadyCheck: false }
then a exception will be thrown.

lib/errors.js

@@ -4,5 +4,8 @@ module.exports.Messages = {
   RETRY_JOB_NOT_EXIST: "Couldn't retry job: The job doesn't exist",
   RETRY_JOB_IS_LOCKED: "Couldn't retry job: The job is locked",
   RETRY_JOB_NOT_FAILED:
-    "Couldn't retry job: The job has been already retried or has not failed"
+    "Couldn't retry job: The job has been already retried or has not failed",
+  MISSING_REDIS_OPTS: `Using a redis instance with enableReadyCheck or maxRetriesPerRequest is not permitted.
+  see https://https://github.com/OptimalBits/bull/issues/1873
+  `
 };

lib/queue.js

@@ -93,7 +93,10 @@ const Queue = function Queue(name, url, opts) {
     const clonedOpts = _.cloneDeep(opts || {});
     opts = {
       ...clonedOpts,
-      redis: { ...redisOptsFromUrl(url), ...clonedOpts.redis }
+      redis: {
+        ...redisOptsFromUrl(url),
+        ...clonedOpts.redis
+      }
     };
   } else {
     opts = _.cloneDeep(url || {});
@@ -118,7 +121,11 @@ const Queue = function Queue(name, url, opts) {
   this.name = name;
   this.token = uuid.v4();
 
-  opts.redis = opts.redis || {};
+  opts.redis = {
+    ...opts.redis,
+    maxRetriesPerRequest: null,
+    enableReadyCheck: false
+  };
 
   _.defaults(opts.redis, {
     port: 6379,
@@ -281,6 +288,13 @@ function redisClientGetter(queue, options, initCallback) {
       clientOptions.connectionName = this.clientName();
       const client = (connections[type] = createClient(type, clientOptions));
 
+      if (
+        client.options.enableReadyCheck ||
+        client.options.maxRetriesPerRequest
+      ) {
+        throw new Error(errors.Messages.MISSING_REDIS_OPTS);
+      }
+
       // Since connections are lazily initialized, we can't check queue.client
       // without initializing a connection. So expose a boolean we can safely
       // query.

test/test_connection.js

@@ -3,6 +3,7 @@
 const expect = require('expect.js');
 const utils = require('./utils');
 const redis = require('ioredis');
+const Queue = require('../lib/queue');
 
 describe('connection', () => {
   let queue;
@@ -20,6 +21,59 @@ describe('connection', () => {
     return client.quit();
   });
 
+  it('should fail if reusing connections with invalid options', () => {
+    const errMsg = Queue.ErrorMessages.MISSING_REDIS_OPTS;
+    {
+      let testQueue;
+
+      try {
+        const client = new redis();
+
+        const opts = {
+          createClient(type) {
+            switch (type) {
+              case 'client':
+                return client;
+              default:
+                return new redis();
+            }
+          }
+        };
+        testQueue = utils.buildQueue('external connections', opts);
+        throw new Error('should fail with invalid redis options');
+      } catch (err) {
+        expect(err.message).to.be.equal(errMsg);
+        testQueue.close();
+      }
+    }
+    {
+      const subscriber = new redis();
+
+      const opts = {
+        createClient(type) {
+          switch (type) {
+            case 'subscriber':
+              return subscriber;
+            default:
+              return new redis({
+                maxRetriesPerRequest: null,
+                enableReadyCheck: false
+              });
+          }
+        }
+      };
+
+      const testQueue = utils.buildQueue('external connections', opts);
+
+      try {
+        testQueue.on('global:completed', () => {});
+      } catch (err) {
+        expect(err.message).to.be.equal(errMsg);
+        testQueue.close();
+      }
+    }
+  });
+
   it('should recover from a connection loss', done => {
     queue.on('error', () => {
       // error event has to be observed or the exception will bubble up
@@ -80,8 +134,13 @@ describe('connection', () => {
   });
 
   it('should not close external connections', () => {
-    const client = new redis();
-    const subscriber = new redis();
+    const redisOpts = {
+      maxRetriesPerRequest: null,
+      enableReadyCheck: false
+    };
+
+    const client = new redis(redisOpts);
+    const subscriber = new redis(redisOpts);
 
     const opts = {
       createClient(type) {

test/test_pause.js

@@ -11,7 +11,10 @@ const sinon = require('sinon');
 describe('.pause', () => {
   let client;
   beforeEach(() => {
-    client = new redis();
+    client = new redis({
+      maxRetriesPerRequest: null,
+      enableReadyCheck: false
+    });
     return client.flushdb();
   });
 

test/test_queue.js

@@ -280,8 +280,12 @@ describe('Queue', () => {
     });
 
     it('should allow reuse redis connections', done => {
-      const client = new redis();
-      const subscriber = new redis();
+      const redisOpts = {
+        maxRetriesPerRequest: null,
+        enableReadyCheck: false
+      };
+      const client = new redis(redisOpts);
+      const subscriber = new redis(redisOpts);
 
       const opts = {
         createClient(type, opts) {