Skip to content

Commit

Permalink
Fail gracefully when the Redis limiter isn't ready
Browse files Browse the repository at this point in the history
  • Loading branch information
Simon Grondin committed Feb 22, 2018
1 parent f863f65 commit 0117b15
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 22 deletions.
26 changes: 16 additions & 10 deletions bottleneck.js
Original file line number Diff line number Diff line change
Expand Up @@ -882,6 +882,7 @@
client: this.client,
subscriber: this.subClient
};
this.isReady = false;
this.ready = new this.Promise((resolve, reject) => {
var count, done, errorListener;
errorListener = function(e) {
Expand Down Expand Up @@ -926,6 +927,7 @@
initSettings.version = this.instance.version;
args = this.prepareObject(initSettings);
args.unshift((options.clearDatastore ? 1 : 0));
this.isReady = true;
return this.runScript("init", args);
}).then((results) => {
return this.clients;
Expand Down Expand Up @@ -988,17 +990,21 @@
}

runScript(name, args) {
return new this.Promise((resolve, reject) => {
var arr, script;
script = scripts[name];
arr = [this.shas[name], script.keys.length].concat(script.keys, args, function(err, replies) {
if (err != null) {
return reject(err);
}
return resolve(replies);
if (!this.isReady) {
return this.Promise.reject(new BottleneckError("This limiter is not done connecting to Redis yet. Wait for the 'ready' event to be triggered before submitting requests."));
} else {
return new this.Promise((resolve, reject) => {
var arr, script;
script = scripts[name];
arr = [this.shas[name], script.keys.length].concat(script.keys, args, function(err, replies) {
if (err != null) {
return reject(err);
}
return resolve(replies);
});
return this.client.evalsha.bind(this.client).apply({}, arr);
});
return this.client.evalsha.bind(this.client).apply({}, arr);
});
}
}

convertBool(b) {
Expand Down
2 changes: 1 addition & 1 deletion bottleneck.min.js

Large diffs are not rendered by default.

26 changes: 16 additions & 10 deletions lib/RedisStorage.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion src/RedisStorage.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class RedisStorage
@shas = {}

@clients = { client: @client, subscriber: @subClient }
@isReady = false
@ready = new @Promise (resolve, reject) =>
errorListener = (e) -> reject e
count = 0
Expand Down Expand Up @@ -87,6 +88,7 @@ class RedisStorage

args = @prepareObject(initSettings)
args.unshift (if options.clearDatastore then 1 else 0)
@isReady = true
@runScript "init", args
.then (results) =>
@clients
Expand Down Expand Up @@ -116,7 +118,8 @@ class RedisStorage
arr

runScript: (name, args) ->
new @Promise (resolve, reject) =>
if !@isReady then @Promise.reject new BottleneckError "This limiter is not done connecting to Redis yet. Wait for the 'ready' event to be triggered before submitting requests."
else new @Promise (resolve, reject) =>
script = scripts[name]
arr = [@shas[name], script.keys.length].concat script.keys, args, (err, replies) ->
if err? then return reject err
Expand Down
13 changes: 13 additions & 0 deletions test/redis.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,19 @@ if (process.env.DATASTORE === 'redis') {
c.limiter.disconnect(false)
})

it('Errors out if not ready', function () {
c = makeTest({ maxConcurrent: 2 })

return c.limiter.schedule({id: 1}, c.slowPromise, 100, null, 1)
.then(function (result) {
return Promise.reject('Should not have been ready')
})
.catch(function (err) {
c.mustEqual(err.message, 'This limiter is not done connecting to Redis yet. Wait for the \'ready\' event to be triggered before submitting requests.')
return Promise.resolve()
})
})

it('Publishes running decreases', function () {
c = makeTest({ maxConcurrent: 2 })
var limiter2, p1, p2, p3, p4
Expand Down

0 comments on commit 0117b15

Please sign in to comment.