Skip to content

4.1.0 publish breaks when publishing an integer #2116

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
anshul-kai opened this issue May 3, 2022 · 18 comments
Open

4.1.0 publish breaks when publishing an integer #2116

anshul-kai opened this issue May 3, 2022 · 18 comments
Labels

Comments

@anshul-kai
Copy link

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 anshul-kai added the Bug label May 3, 2022
@byron70
Copy link

byron70 commented May 3, 2022

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.

@leibale
Copy link
Contributor

leibale commented May 4, 2022

@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.. 🤦‍♂️

@anshul-kai
Copy link
Author

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')

@tugtugtug
Copy link

tugtugtug commented May 4, 2022

@leibale this is definitely a regression, the addCommand, hSet are also now broken with integer/boolean values. If #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?

@patel-kalp
Copy link

patel-kalp commented May 4, 2022

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

@moisesbites
Copy link

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

@moisesbites
Copy link

Hi, some solution for this issue?

@musta20
Copy link

musta20 commented Jun 23, 2022

hi facing the same issue solution yet

@younggeun0
Copy link

younggeun0 commented Jun 24, 2022

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();

@moisesbites
Copy link

moisesbites commented Jun 24, 2022

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.

@dragonlobster
Copy link

dragonlobster commented Aug 1, 2022

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

@moisesbites
Copy link

moisesbites commented Aug 1, 2022

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...

@devel0perx
Copy link

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

@leibale
Copy link
Contributor

leibale commented Sep 26, 2022

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

@zelucena
Copy link

zelucena commented Oct 4, 2022

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)

@moisesbites
Copy link

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.

LeeMoonki pushed a commit to joyfulprogrammers/jakshim-backend that referenced this issue Nov 10, 2022
@DDDDDanica
Copy link

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 ~

@kasvith
Copy link

kasvith commented Jul 17, 2023

This is a serious error :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests