diff --git a/lib/errors.js b/lib/errors.js index 6073fc5..1fa2bf8 100644 --- a/lib/errors.js +++ b/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 + ` }; diff --git a/lib/queue.js b/lib/queue.js index a34b5c5..ba02a8d 100755 --- a/lib/queue.js +++ b/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. diff --git a/test/test_connection.js b/test/test_connection.js index f0c9d9d..f680371 100644 --- a/test/test_connection.js +++ b/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) { diff --git a/test/test_pause.js b/test/test_pause.js index 69e2063..a208e2f 100644 --- a/test/test_pause.js +++ b/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(); }); diff --git a/test/test_queue.js b/test/test_queue.js index 1e49a6a..f211b0f 100644 --- a/test/test_queue.js +++ b/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) {