breaks due to node_redis 2.6.x

[cfchou]

I found that the way mosca uses Multi-Exec on node_redis 2.5.x may not work on 2.6.x.
For example, redis.js

    var subscriptions;

    var multi = this._client.multi();
    var that = this;

    multi.get(key, function(err, result) {
      subscriptions = JSON.parse(result) || {};   
    });

    multi.exec(function(err) {
      cb(err, subscriptions);
    });

The callback installed by get would only be called when an error occurs and effectively only takes 'err' as argument. subscriptions therefore remains undefined.

The following code can trigger the error:

var mqtt = require('mqtt');
var client = mqtt.connect({ host: 'localhost', port: 1883,
    clientId: 'dummy_101', clean: false});

Server output:

/Users/cfchou/Project/mosca/lib/persistence/abstract.js:109
var subs = Object.keys(subscriptions);
^

TypeError: Cannot convert undefined or null to object
at Function.keys (native)
at /Users/cfchou/Project/mosca/lib/persistence/abstract.js:109:25
at Multi.callback (/Users/cfchou/Project/mosca/lib/persistence/redis.js:343:7)
at multi_callback (/Users/cfchou/Project/mosca/node_modules/redis/lib/multi.js:88:14)
at Command.callback (/Users/cfchou/Project/mosca/node_modules/redis/lib/multi.js:115:9)
at normal_reply (/Users/cfchou/Project/mosca/node_modules/redis/index.js:714:21)
at RedisClient.return_reply (/Users/cfchou/Project/mosca/node_modules/redis/index.js:816:9)
at JavascriptRedisParser.Parser.returnReply (/Users/cfchou/Project/mosca/node_modules/redis/index.js:188:18)
at JavascriptRedisParser.execute (/Users/cfchou/Project/mosca/node_modules/redis/node_modules/redis-parser/lib/parser.js:408:12)
at Socket. (/Users/cfchou/Project/mosca/node_modules/redis/index.js:267:27)

Process finished with exit code 1

[mcollina]

Same as the error shown in #490. Would you mind sending a PR that updates this to node-redis 2.6?

[andrewwolverton]

@mcollina in reference to this (#493) and #490

Reverting the redis dependency to 2.5.3 seems to immediately resolve the issue at least from the test case perspective. There are 2 obvious options:

1. revert the dependency and wait for the ioredis upgrade that's happening soon anyway
   or
2. update all of the redis.js file to conform to redis > 2.6, btw... it's not just this function that's broken. several others are as well.

#2 is probably a bigger undertaking than it's worth given the ioredis upgrade mentioned in #1. My inclination would be to just focus on ioredis upgrade.

The initial commit in #490 should still be integrated to resolve the HAProxy/redis stop/start problem.

[mcollina]

would you mind sending a PR pinning redis on 2.5.x?
Il giorno mar 14 giu 2016 alle 19:18 Andrew Wolverton <
notifications@github.com> ha scritto:

@mcollina https://github.com/mcollina in reference to this (#493
#493) and #490
#490

Reverting the redis dependency to 2.5.3 seems to immediately resolve the
issue at least from the test case perspective. There are 2 obvious options:

1. revert the dependency and wait for the ioredis upgrade that's
   happening soon anyway or
2. update all of the redis.js file to conform to redis > 2.6, btw...
   it's not just this function that's broken. several others are as well.

#2 #2 is probably a bigger
undertaking than it's worth given the ioredis upgrade mentioned in #1
#1. My inclination would be to
just focus on ioredis upgrade.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#493 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AADL40Oo8eI6JNTQtVr1UfcsjAwoM9f2ks5qLvBvgaJpZM4I1K2B
.

[mcollina]

Fixed in #494