client.multi() not working

[ghostdouble]

using the following, for example, does not work await client.multi().set('a','b').exec()

temporary fix:

1. in node_modules/redis-commands/commands.json, remove the multi key (or find a way to skip multi in async-redis decorate)
2. use

   const redis = require("redis")
   const asyncRedis = require("async-redis")
   const client = redis.createClient()
   const asyncRedisClient = asyncRedis.decorate(client)

3. use all commands the normal way, except for multi - wrap it in a promise

   const res = await new Promise((resolve, reject)=>{
   multistack = asyncRedisClient.multi()
   multistack.set('a','b')
   multistack.hset('h','x','y')
   multistack.exec((err,results)=>{
   resolve(results)
   })
   })

[moaxaca]

@ghostdouble I will write a wrapper for this explicitly. Nice catch ty.

[nissim-kurle]

hi can you please show how to use watch with multi. Its not working for me as well.

redisClient.watch(conv, async function(err) {
                                if (err) {
                                    console.log('There was an error while watching .', conversations[0].id);
                                }
                                try {
                                let result = await redisClient.multi().hmset(conv, lastActivity, lastActivityTimeStamp.toString(), helper.participants, JSON.stringify(data.participants)).exec();

                                    if (result === null) {
                                        console.log('The transaction was not performed since the data was accessed by someone else during the transaction and changed.');
                                    }
                                } catch (err) {
                                    console.log('An error occured while performing the update conversation transation in redis');
                                }
                            });

[jd20]

Any update on this? Seems multi() is still broken when using the async client. I'm seeing the following:

> require("async-redis").createClient().multi()
Promise {
  <rejected> TypeError: Cannot read property '0' of undefined
      at new Multi (~/testing/node_modules/redis/lib/multi.js:13:30)
      at RedisClient.multi (~/testing/node_modules/redis/lib/individualCommands.js:25:17)
      at Promise (~/testing/node_modules/async-redis/src/index.js:24:14)
      at new Promise (<anonymous>)

[moaxaca]

@jd20 Sorry for the delay I've been caught up in other projects. I was hoping someone else might take a stab at this but it appears not. I will try to make an effort at resolving this later this week.

Thanks for your patience everybody.

[gilad-solter]

@jd20 Sorry for the delay I've been caught up in other projects. I was hoping someone else might take a stab at this but it appears not. I will try to make an effort at resolving this later this week.

Thanks for your patience everybody.

Thanks @moaxaca

[andylash]

here's a PR for this

[kwalski]

Can this issue be closed now that this PR is merged?

[gilad-solter]

Can this issue be closed now that this PR is merged?

I don't think so. This PR only eliminates the error while using .multi, but the issue was about using multi as async.

[andylash]

This PR only eliminates the error while using .multi, but the issue was about using multi as async.

From what I can tell node redis's multi() isn't async and doesn't take a callback as a parameter. Thus having it return a promise doesn't seem correct to mean because it would break chaining. I'm surely open to being wrong, but I don't understand what further change would make multi async as requested without breaking multi and diverting from the node redis interface.

[gilad-solter]

Ohh I see, thanks @andylash for the pr and for looking into this!

[jd20]

@kwalski I think this issue can be closed, multi() shouldn't return a promise (exec() should) and not throw an exception, which the PR seems to fix. If exec() works properly (which I think it already does), then the example in the first comment of this issue should work. Any idea when 1.1.5 will be released? Will be great to have multi() functionality live!

[moaxaca]

@jd20 https://www.npmjs.com/package/async-redis

[moaxaca]

I am leaving this issue open until I implement a more robust solution.

[jd20]

Testing with 1.1.5, the resolved value from the promise returned by exec() doesn't seem to correspond to the actual results set, it's just a single boolean value. For example, using the workaround provided by @ghostdouble, the final resolved results is [ 'OK', 0 ], whereas the code below returns just true:

await client.multi().set('a', 'b').hset('h', 'x', 'y').exec()

[jd20]

The problem seems to be that the Multi object returned by multi() (or batch()) needs to also be decorated. I took a stab at this with #15.

[prodigga]

Noticing the same thing here - I can't get the result. It just returns 'true'.

const result = await client.multi().incr(token).expire([token,59]).exec();
console.log(result); //true

ioredis returns an array of results for this operation, so I can check the results of incr and ex operations.

[MatanYemini]

I think that people are still reaching here...

After the fixes that went in, it is possible to wrap the result of exec with Promise, and then you will be sure that the result will include the replies from the multi.

something like: return new Promise((resolve, reject) => { multi.exec((err, replies) => { if (err) { reject(err); } return resolve(replies); }); });

[moaxaca]

Fixed in 2.0.0