Skip to content
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

connect-redis not working with latest version of node-redis client (https://github.com/redis/node-redis) #336

Closed
rukmanidevi-bh opened this issue Nov 18, 2021 · 9 comments

Comments

@rukmanidevi-bh
Copy link

This code snippet uses latest version of conenct redis and node-redis and req.session is setting undefined when opening app in browser.
`import { createRequire } from 'module';
const require = createRequire(import.meta.url);
const express = require('express');
const session = require('express-session');
const app = express();
import { createClient } from 'redis';
const redisStore = require('connect-redis')(session);

const port = process.env.PORT || 8030;
const config= {
"host":"127.0.0.1",
"port":6379,
"username":"",
"password":"test"
};

async function setupsession(){
const client = createClient({ url: 'redis://default:test@127.0.0.1:6379'});

client.on('error', (err) => console.log('Redis Client Error', err));

await client.connect();

app.use(session({
secret: 'top_secrect_react_app_hub',
name: 'local.cookie',
proxy: true,
resave: false,
saveUninitialized: true,
cookie: {
secure: true,
httpOnly: true
},
store: new redisStore({ client: client }),
}
));

await client.set('age', '31');
await client.set('field1', 'value1');
 await client.set( 'field2', 'value2');
 await client.hSet('myhash', 'field1', "Hello");

const val=await client.hGetAll('myhash');
const value = await client.get('field1');
console.log(value);
console.log(val);
const [ping, get, quit] = await Promise.all([
client.ping(),
client.get('key1'),
//client.quit()
]); // ['PONG', null, 'OK']
console.log(ping,get,quit);
try {
await client.get('key');
} catch (err) {
// ClosedClient Error
console.log(err);
}
}

app.get('/', function (req, res) {
	 req.session.test="new";
		   req.session.save();
		   res.send('server started');
		  
	
});
app.get('/rbac', function (req, res) {
		   req.session.expiresOn = 'test stored';
	
});
setupsession();

var server = app.listen(port, function() {
console.log(Server started on PORT ${port});

});`

@basaran
Copy link

basaran commented Nov 23, 2021

Ran into the same issue, just now. If you had installed the @next version of node-redis, add this legacyMode to your client config:

const client = createClient({
    legacyMode: true
});

https://github.com/redis/node-redis/blob/master/docs/v3-to-v4.md

@rukmanidevi-bh
Copy link
Author

@basaran are able to connect to Redis 6 instance with username and password in legacy mode. Because the reason i upgraded to @next version of node-redis is that old version send only password in auth , which doesn't support ACL in Redis 6 version.

@huineng
Copy link

huineng commented Nov 25, 2021

the legacyMode: true currently allows for using the new "redis": "^4.0.0" but without that flag, it's indeed not working

The client successfully connects to redis , but loading or using session storage is not working
Difficult to debug but i have seen errors like this

TypeError [ERR_INVALID_ARG_TYPE]: The "chunk" argument must be of type string or an instance of Buffer or Uint8Array. Received an instance of Array
        at new NodeError (node:internal/errors:371:5)
        at _write (node:internal/streams/writable:312:13)
        at TLSSocket.Writable.write (node:internal/streams/writable:334:10)
        at RedisSocket.writeCommand (myapp/node_modules/@node-redis/client/dist/lib/client/socket.js:57:130)
        at Commander._RedisClient_tick (myapp/node_modules/@node-redis/client/dist/lib/client/index.js:415:64)
        at Commander._RedisClient_sendCommand (myapp/node_modules/@node-redis/client/dist/lib/client/index.js:396:82)
        at Commander.commandsExecutor (myapp/node_modules/@node-redis/client/dist/lib/client/index.js:160:154)
        at Commander.BaseClass.<computed> [as set] (myapp/node_modules/@node-redis/client/dist/lib/commander.js:8:29)
        at RedisStore.set (myapp/node_modules/connect-redis/lib/connect-redis.js:65:21)
        at Session.save (myapp/node_modules/express-session/session/session.js:72:25) {
         code: 'ERR_INVALID_ARG_TYPE'
    }

and

message: Promise {
    <rejected> ReplyError: ERR Protocol error: expected '$', got ' '
        at parseError (myapp/node_modules/redis-parser/lib/parser.js:179:12)
        at parseType (myapp/node_modules/redis-parser/lib/parser.js:302:14)
  },

I only use redis for session storage

thanks

@abhijoshi2k
Copy link
Contributor

Any estimate when #337 will be merged?

benb116 added a commit to benb116/Ball-Street that referenced this issue Dec 2, 2021
Not fully ready, other dependencies need to upgrade to match. Specifically connect-redis (tj/connect-redis#336)
@wavded
Copy link
Collaborator

wavded commented Dec 7, 2021

I cannot recommend using Redis v4 at this time as it is not interoperable with Redis v3 (its legacyMode feature is broken and has many bugs at the moment). It is also not interoperable with ioredis and redis-mock which are used by others. For now, I have updated the documentation to install redis@v3 for the time being.

redis/node-redis#1765

benb116 added a commit to benb116/Ball-Street that referenced this issue Dec 8, 2021
commit 3392d67
Author: benb116 <benb116@gmail.com>
Date:   Sun Dec 5 23:14:57 2021 -0500

    Use legacy redis client for session store

commit 96b4b88
Author: benb116 <benb116@gmail.com>
Date:   Wed Dec 1 22:15:24 2021 -0500

    Fix redis in tests

    Correct set, get, keys, and ttl functions. Also wait for client ready in global setup

commit 3d9f31b
Author: benb116 <benb116@gmail.com>
Date:   Wed Dec 1 22:14:17 2021 -0500

    Fix HSET

commit 0e23d90
Author: benb116 <benb116@gmail.com>
Date:   Wed Dec 1 22:11:51 2021 -0500

    Fix redis key ttl setting

commit 7bc261e
Author: benb116 <benb116@gmail.com>
Date:   Wed Dec 1 17:14:23 2021 -0500

    Switch to node-redis 4.0.0

    Not fully ready, other dependencies need to upgrade to match. Specifically connect-redis (tj/connect-redis#336)

commit 0af210c
Author: benb116 <benb116@gmail.com>
Date:   Wed Dec 1 17:10:48 2021 -0500

    Get current week not through redis
@salaadas
Copy link

I also noticed that the maxAge isn't working right, I set to 10 years but it is only set for several months

so010 added a commit to edumeet/edumeet that referenced this issue Feb 1, 2022
Redis:
- redis-> redis@v3 until v4 is working stable
  ( tj/connect-redis#336 )
- connect-redis -> 6.0.0
- update @types/connect-redis -> ^0.0.18 and removed
  @types/redis since this is not necessary anymore

Corrected edumeet server version

deactivated spdy since it is not working anymore with node.js>15
( spdy-http2/node-spdy#380 )

removed package-lock.json ( just support yarn, use npm at your own risk
wavded added a commit that referenced this issue Feb 4, 2022
The `legacyMode` option in `redis@v4` has matured enough such that all our tests pass now.

Also includes:

 - Remove legacy v3 -> v4 upgrade docs
 - Switch to double quotes for formatting

#336
@wavded
Copy link
Collaborator

wavded commented Feb 4, 2022

Support for Redis V4 now exists through the legacyMode option.

@wavded wavded closed this as completed Feb 4, 2022
jellis24 added a commit to jellis24/connect-redis that referenced this issue Apr 6, 2022
The `legacyMode` option in `redis@v4` has matured enough such that all our tests pass now.

Also includes:

 - Remove legacy v3 -> v4 upgrade docs
 - Switch to double quotes for formatting

tj/connect-redis#336
@wavded
Copy link
Collaborator

wavded commented Feb 22, 2023

I am removing the legacyMode requirement in the next major version of this package. If you want to try it you can npm install connect-redis@next in your projects:

Migration guide in this PR: #377

@wavded wavded reopened this Feb 22, 2023
@wavded
Copy link
Collaborator

wavded commented Feb 28, 2023

v7.0.0 has been released, closing

@wavded wavded closed this as completed Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants