redis / node-redis

Redis Node.js client
https://redis.js.org/
MIT License
16.93k stars 1.89k forks source link

Creating redis client using options object without host and port. #1420

Open mikhailsidorov opened 5 years ago

mikhailsidorov commented 5 years ago

This code runs without error, even if there is no running local server. Is it correct behavior or a bug?

const redis = require('redis')
const redisClient = redis.createClient({ retry_strategy: () => 1000 })
knoxcard commented 5 years ago

@mikhailsidorov - Yes, this is correct, as the default options are 'localhost' and 6379 respectively.

Oh, but you are stating there is no running local Redis server? hahaha Now that would be incredibly strange.

I guarantee you still have a Redis instance running...go to your linux command line console.

View all running instances of Redis ps -aux | grep redis-server

Kill all Redis instances killall redis-server

Close ticket?

mikhailsidorov commented 5 years ago

I ran it in docker, without redis. Let me check again.

knoxcard commented 5 years ago

Redis has to be running somewhere, I tried it in my environment and ran /etc/init.d/redis-server stop. The first time it did exactly as you mentioned as I was baffled! Then I ran ps -aux | grep redis-server found out I had seven Redis instances running in the background, lol. After killing all, node redis immediately errored out as expected.

mikhailsidorov commented 5 years ago

@mikhailsidorov - Yes, this is correct, as the default options are 'localhost' and 6379 respectively.

Oh, but you are stating there is no running local Redis server? hahaha Now that would be incredibly strange.

I guarantee you still have a Redis instance running...go to your linux command line console.

View all running instances of Redis ps -aux | grep redis-server

Kill all Redis instances killall redis-server

Close ticket?

There are no redis-server processes on my machine and in docker container. Also, I have not any process listens 6379 port (netstat -an | grep 6379 return nothing).

mikhailsidorov commented 5 years ago

Redis has to be running somewhere, I tried it in my environment and ran /etc/init.d/redis-server stop. The first time it did exactly as you mentioned as I was baffled! Then I ran ps -aux | grep redis-server found out I had seven Redis instances running in the background, lol. After killing all, node redis immediately errored out as expected.

I did not find any process of the redis-server running.

mikhailsidorov commented 5 years ago

I prepared an example for reproducing the issue, please see this repo https://github.com/mikhailsidorov/redis-issue This code running without any errors, and I have not redis-server processes run on the tested machine and on the docker container.

knoxcard commented 5 years ago

Are you by any chance running redis-mock, or anything similar? (Doubt it, just checking...) https://github.com/faeldt/redis-mock

Here is the error I receive when I turn off Redis and run my app...(This is not the docker app you provided) I need to set docker up, I am still not using docker, i have to get with the times. Will try a bit later....

Error: Redis connection to 127.0.0.1:6379 failed - connect ECONNREFUSED 127.0.0.1:6379
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1054:14)
Emitted 'error' event at:
    at RedisClient.on_error (/mnt/c/indospace.io/services/node_modules/redis/index.js:406:14)
    at Socket.<anonymous> (/mnt/c/indospace.io/services/node_modules/redis/index.js:279:14)
    at Socket.emit (events.js:196:13)
    at emitErrorNT (internal/streams/destroy.js:91:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:59:3)
    at processTicksAndRejections (internal/process/task_queues.js:84:17)
mikhailsidorov commented 5 years ago

Are you by any chance running redis-mock, or anything similar? (Doubt it, just checking...) https://github.com/faeldt/redis-mock

Here is the error I receive when I turn off Redis and run my app...(This is not the docker app you provided) I need to set docker up, I am still not using docker, i have to get with the times. Will try a bit later....

Error: Redis connection to 127.0.0.1:6379 failed - connect ECONNREFUSED 127.0.0.1:6379
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1054:14)
Emitted 'error' event at:
    at RedisClient.on_error (/mnt/c/indospace.io/services/node_modules/redis/index.js:406:14)
    at Socket.<anonymous> (/mnt/c/indospace.io/services/node_modules/redis/index.js:279:14)
    at Socket.emit (events.js:196:13)
    at emitErrorNT (internal/streams/destroy.js:91:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:59:3)
    at processTicksAndRejections (internal/process/task_queues.js:84:17)

No, I don't use redis-mock. When I run const redisClient = redis.createClient() without options, I have the same error, but when I pass { retry_strategy: () => 1000 } I get this issue.

knoxcard commented 5 years ago

oh okay, think we are on the same page now...

Ya, the node_redis retry connection/reconnection code seems completely busted at the moment, there are numerous open tickets referring to these issues. I am submitted PR requests on other nodeJS modules tonight, will try to dive into this and submit a valid PR when I can.

mikhailsidorov commented 5 years ago

oh okay, think we are on the same page now...

Ya, the node_redis retry connection/reconnection code seems completely busted at the moment, there are numerous open tickets referring to these issues. I am submitted PR requests on other nodeJS modules tonight, will try to dive into this and submit a valid PR when I can.

Thank you

BridgeAR commented 5 years ago

This is somewhat intentional. Check connect_timeout: Default is to try connecting until the default system socket timeout has been exceeded and to try reconnecting until 1h has elapsed..

The intention is to have a very powerful function where all reconnect logic is in the hands of the user. It might not always be ideal though.

mikhailsidorov commented 5 years ago

This is somewhat intentional. Check connect_timeout: Default is to try connecting until the default system socket timeout has been exceeded and to try reconnecting until 1h has elapsed..

The intention is to have a very powerful function where all reconnect logic is in the hands of the user. It might not always be ideal though.

Thank you for the clarification.

knoxcard commented 5 years ago

@mikhailsidorov - close issue?

BridgeAR commented 5 years ago

@knoxcard I would like to keep this open for myself to look into this again when I find some time.

BridgeAR commented 5 years ago

@knoxcard thanks for looking into a couple of issues here by the way! Are you interested in helping out more in general?

knoxcard commented 5 years ago

@BridgeAR - absolutely! how should I help out more?

mikhailsidorov commented 5 years ago

@mikhailsidorov - close issue?

Sure, if it is needed. Thanks

knoxcard commented 5 years ago

@mikhailsidorov - nevermind closing the issue, BridgeAr wants to keep this issue open, Thanks!