node-redis icon indicating copy to clipboard operation
node-redis copied to clipboard

4.1.0 publish breaks when publishing an integer

Open anshul-kai opened this issue 2 years ago • 18 comments

This was working fine up until 4.0.6 but starting in version 4.1.0, publishing integers throws the following error.

/node_modules/@redis/client/dist/lib/client/RESP2/encoder.js:20
            throw new TypeError('Invalid argument type');
                  ^

TypeError: Invalid argument type
    at encodeCommand (/node_modules/@redis/client/dist/lib/client/RESP2/encoder.js:20:19)
    at RedisCommandsQueue.getCommandToSend (/node_modules/@redis/client/dist/lib/client/commands-queue.js:187:45)
    at Commander._RedisClient_tick (/node_modules/@redis/client/dist/lib/client/index.js:429:76)
    at Commander._RedisClient_sendCommand (/node_modules/@redis/client/dist/lib/client/index.js:413:82)
    at Commander.commandsExecutor (/node_modules/@redis/client/dist/lib/client/index.js:167:154)
    at Commander.BaseClass.<computed> [as publish] (/node_modules/@redis/client/dist/lib/commander.js:8:29)

Environment:

  • Node.js Version: 16
  • Redis Server Version: 4
  • Node Redis Version: 4.1.0
  • Platform: Ubuntu

anshul-kai avatar May 03 '22 15:05 anshul-kai

Same environment as a-koka, except for Redis Server Version: 6 on AWS Elasticache.

The setEx command fail on this line if the value being sent is an array. This previously worked without conversion of the array to a string.

client.setEx('myKey', 300, [100,200])

The encoderCommand(), is receiving the ttl of 300 as '300' (a string), so there is some conversion happening in the middle of all that, but not for the value.

I guess in reality, my usage is really incorrect. I actually recall being surprised that the array was converted to a delimited list in the previous version.

byron70 avatar May 03 '22 20:05 byron70

@a-koka @byron70 in previous versions (4.0.0-4.0.6) some values were incorrectly converted into strings, which could cause unexpected behaviors (see #1898 for example). I forgot to add that to the release notes.. :man_facepalming:

leibale avatar May 04 '22 00:05 leibale

I'm not sure if I follow this thoroughly. Publishing integer values has worked for the longest time until 4.1.0. Are we saying that publishing integers is no longer supported?

redisPubClient.publish('channel', 1) used to work but now has to be written as redisPubClient.publish('channel', '1')

anshul-kai avatar May 04 '22 11:05 anshul-kai

@leibale this is definitely a regression, the addCommand, hSet are also now broken with integer/boolean values. If https://github.com/redis/node-redis/issues/1898 complains about converting undefined, null to empty strings, shouldn't that particular error be fixed instead of completely stopping the conversion of any non-buffer, non-string arguments?

tugtugtug avatar May 04 '22 16:05 tugtugtug

import * as redis from 'redis';
import session from 'express-session';
import connectRedis from 'connect-redis';

const RedisStore = connectRedis(session);
const redisClient = redis.createClient();
await redisClient.connect();

app.use(
        session({
            name: 'sessionId',
            store: new RedisStore({
                client: redisClient as any,
                disableTouch: true,
            }),
            cookie: {
                maxAge: 31536000000,
                sameSite: "lax",
                secure: false
            },
            saveUninitialized: false,
            secret: "test",
            resave: false
        })
)

I am not sure if I am doing this right, but something similar has worked for me before. So now should I be making maxAge a string? @leibale

patel-kalp avatar May 04 '22 18:05 patel-kalp

In this version 4.1.0, Date values are not working anymore too . Before, they worked fine.

moisesbites avatar May 06 '22 11:05 moisesbites

Hi, some solution for this issue?

moisesbites avatar May 19 '22 01:05 moisesbites

hi facing the same issue solution yet

musta20 avatar Jun 23 '22 08:06 musta20

I got the same error. It works for me adding createClient option(legecyMode: true) and connect() together.

import connectRedis from "connect-redis";
...
    const redisClient = createClient({
        legacyMode: true,
    });
   await redisClient.connect();

younggeun0 avatar Jun 24 '22 06:06 younggeun0

I got the same error. It works for me adding createClient option(legecyMode: true) and connect() together.

import connectRedis from "connect-redis";
...
    const redisClient = createClient({
        legacyMode: true,
    });
   await redisClient.connect();

For me, this workaround it's not working... I use many resources from v4.

For while, I'm converting every value to string.

moisesbites avatar Jun 24 '22 13:06 moisesbites

please help my production is broken because of this update... other bugs also are introduced because of the update, i am reverting back to the working version

dragonlobster avatar Aug 01 '22 14:08 dragonlobster

please help my production is broken because of this update

@dragonlobster for while, convert to string every value on or before command db.set... when you to execute command get, convert the returned string to correct datatype...

moisesbites avatar Aug 01 '22 14:08 moisesbites

@a-koka i am using redis 7 and node 16 having same problem but it work sometime and give above error What to do ?

devel0perx avatar Sep 26 '22 11:09 devel0perx

Make sure to convert the channel and message into strings or buffers

leibale avatar Sep 26 '22 19:09 leibale

I found the same issue while trying to upgrade Node ACL2 to Redis 4.3.0. sAdd stopped working with integers as well. Stack trace:

 Uncaught TypeError: Invalid argument type
      at encodeCommand (node_modules\@redis\client\dist\lib\client\RESP2\encoder.js:17:19)
      at RedisCommandsQueue.getCommandToSend (node_modules\@redis\client\dist\lib\client\commands-queue.js:187:45)
      at Commander._RedisClient_tick (node_modules\@redis\client\dist\lib\client\index.js:440:76)
      at Commander.multiExecutor (node_modules\@redis\client\dist\lib\client\index.js:246:86)
      at Commander.exec (node_modules\@redis\client\dist\lib\client\multi-command.js:84:175)
      at RedisBackend.end (lib\redis-backend.js:36:46)
      at RedisBackend.tryCatcher (node_modules\bluebird\js\release\util.js:16:23)
      at RedisBackend.ret [as endAsync] (eval at makeNodePromisifiedEval (node_modules\bluebird\js\release\promisify.js:184:12), <anonymous>:13:39)
      at Acl.allow (lib\acl.js:337:29)
      at C:\Users\jose.lucena\Downloads\node_acl-master\node_acl-master\lib\acl.js:766:59
      at tryCatcher (node_modules\bluebird\js\release\util.js:16:23)
      at Object.gotValue (node_modules\bluebird\js\release\reduce.js:168:18)
      at Object.gotAccum (node_modules\bluebird\js\release\reduce.js:155:25)
      at Object.tryCatcher (node_modules\bluebird\js\release\util.js:16:23)
      at Promise._settlePromiseFromHandler (node_modules\bluebird\js\release\promise.js:547:31)
      at Promise._settlePromise (node_modules\bluebird\js\release\promise.js:604:18)
      at Promise._settlePromise0 (node_modules\bluebird\js\release\promise.js:649:10)
      at Promise._settlePromises (node_modules\bluebird\js\release\promise.js:729:18)
      at Promise._fulfill (node_modules\bluebird\js\release\promise.js:673:18)
      at C:\Users\jose.lucena\Downloads\node_acl-master\node_acl-master\node_modules\bluebird\js\release\nodeback.js:42:21
      at RedisBackend.end (lib\redis-backend.js:37:13)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

zelucena avatar Oct 04 '22 12:10 zelucena

I found the same issue while trying to upgrade Node ACL2 to Redis 4.3.0. sAdd stopped working with integers as well. Stack trace:

 Uncaught TypeError: Invalid argument type
      at encodeCommand (node_modules\@redis\client\dist\lib\client\RESP2\encoder.js:17:19)
      at RedisCommandsQueue.getCommandToSend (node_modules\@redis\client\dist\lib\client\commands-queue.js:187:45)
      at Commander._RedisClient_tick (node_modules\@redis\client\dist\lib\client\index.js:440:76)
      at Commander.multiExecutor (node_modules\@redis\client\dist\lib\client\index.js:246:86)
      at Commander.exec (node_modules\@redis\client\dist\lib\client\multi-command.js:84:175)
      at RedisBackend.end (lib\redis-backend.js:36:46)
      at RedisBackend.tryCatcher (node_modules\bluebird\js\release\util.js:16:23)
      at RedisBackend.ret [as endAsync] (eval at makeNodePromisifiedEval (node_modules\bluebird\js\release\promisify.js:184:12), <anonymous>:13:39)
      at Acl.allow (lib\acl.js:337:29)
      at C:\Users\jose.lucena\Downloads\node_acl-master\node_acl-master\lib\acl.js:766:59
      at tryCatcher (node_modules\bluebird\js\release\util.js:16:23)
      at Object.gotValue (node_modules\bluebird\js\release\reduce.js:168:18)
      at Object.gotAccum (node_modules\bluebird\js\release\reduce.js:155:25)
      at Object.tryCatcher (node_modules\bluebird\js\release\util.js:16:23)
      at Promise._settlePromiseFromHandler (node_modules\bluebird\js\release\promise.js:547:31)
      at Promise._settlePromise (node_modules\bluebird\js\release\promise.js:604:18)
      at Promise._settlePromise0 (node_modules\bluebird\js\release\promise.js:649:10)
      at Promise._settlePromises (node_modules\bluebird\js\release\promise.js:729:18)
      at Promise._fulfill (node_modules\bluebird\js\release\promise.js:673:18)
      at C:\Users\jose.lucena\Downloads\node_acl-master\node_acl-master\node_modules\bluebird\js\release\nodeback.js:42:21
      at RedisBackend.end (lib\redis-backend.js:37:13)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

Convert to string before send to redis client, reconvert to datatype after get from redis. It's the solution, for while.

moisesbites avatar Oct 04 '22 13:10 moisesbites

I got the same error. It works for me adding createClient option(legecyMode: true) and connect() together.

import connectRedis from "connect-redis";
...
    const redisClient = createClient({
        legacyMode: true,
    });
   await redisClient.connect();

Thank you !!! and this should be the fix and connect-redis did not support redis@v4 completely.

https://github.com/tj/connect-redis

Known compatible and tested clients:

[redis](https://github.com/NodeRedis/node-redis) (v3, v4 with legacyMode: true)
[ioredis](https://github.com/luin/ioredis)
[redis-mock](https://github.com/yeahoffline/redis-mock) for testing.

It would be great if this is specified in migration guide ~

DDDDDanica avatar Nov 14 '22 13:11 DDDDDanica

This is a serious error :(

kasvith avatar Jul 17 '23 17:07 kasvith