joshuah/sol-redis-pool

My bluebird Promisify broke upgrading from 0.2.4 to 0.3.1

jeff-kilbride opened this issue · 3 comments

Hi,

I'm wondering what the correct way to Promisify the underlying connections in the pool would be. I'm using cache-manager-redis. In their latest release, they moved to sol-redis-pool 0.3.1 and my bluebird promises stopped working. Has something changed with the connection being returned by acquire?

The cache-manager-redis module provides a getClient method, which returns a connection from the pool. In 0.2.4, this code was working:

cache.store.getClientAsync = Promise.promisify(cache.store.getClient, { context: cache.store });
cache.store.getClient(function (err, redis) {
    redis.client.mgetAsync = Promise.promisify(redis.client.mget, { context: redis.client });
    redis.client.msetAsync = Promise.promisify(redis.client.mset, { context: redis.client });
    redis.done();
});

...  later in code ...

let redis = yield cache.store.getClientAsync(),
    values = yield redis.client.mgetAsync(...keys);

In 0.3.1, my code is bombing out on the mgetAsync(...keys) call. I'm getting the following error:

TypeError: Function.prototype.apply was called on undefined, which is a undefined and not a function

I stepped through using my debugger and redis.client.mgetAsync doesn't exist using 0.3.1. However, as soon as I downgraded to 0.2.4, it started working again. The only difference I can see is that _sol_cid exists on the 0.3.1 client, but not on the 0.2.4 client. In 0.3.1, _sol_cid in the first getClient call is 1 and in the second getClientAsync call it's 2.

As I think about it, it seems like I may only be Promisifying one connection in the pool. I guess my question would be, how do I Promisify all the connections that the pool creates? Any insight would be appreciated!

I was able to resolve this by editing index.js and adding the following lines at the top of the file, just under the other require statements:

var bluebird = require('bluebird');

bluebird.promisifyAll(redis.RedisClient.prototype);
bluebird.promisifyAll(redis.Multi.prototype);

Now, it looks like all the underlying pool connections are Promisfied and I can do this in my code without any problems:

let redis = yield cache.store.getClientAsync(),
    values = yield redis.client.mgetAsync(...keys);

As a bonus, I don't have to Promisify the get and set methods individually. It just works. Would you consider adding Promises to your module this way? It's pretty simple, but if you'd like I can submit a PR for it.

I am open to the idea. Give me a few weeks before I put it in.

On Sep 18, 2016, at 11:23 PM, Jeff Kilbride notifications@github.com wrote:

I was able to resolve this by editing index.js and adding the following lines at the top of the file, just under the other require statements:

var bluebird = require('bluebird');

bluebird.promisifyAll(redis.RedisClient.prototype);
bluebird.promisifyAll(redis.Multi.prototype);
Now, it looks like all the underlying pool connections are Promisfied and I can do this in my code without any problems:

let redis = yield cache.store.getClientAsync(),
values = yield redis.client.mgetAsync(...keys);
As a bonus, I don't have to Promisify the get and set methods individually. It just works. Would you consider adding Promises to your module this way? It's pretty simple, but if you'd like I can submit a PR for it.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

Merged and published.