[Bug] Redis reconnect on connection issue#442
Conversation
ocastx
force-pushed
the
fix/redis-reconnect-issue
branch
from
32e7c59 to
810d62f
Compare
|
Or perhaps provide a way to pass in or set an error handler so redis errors can be logged |
| socket: this.options.socket, | ||
| username: this.options.username, | ||
| password: this.options.password, | ||
| name: this.options.name, | ||
| database: this.options.database, | ||
| commandsQueueMaxLength: this.options.commandsQueueMaxLength, | ||
| disableOfflineQueue: this.options.disableOfflineQueue, | ||
| readonly: this.options.disableOfflineQueue, | ||
| legacyMode: this.options.legacyMode, | ||
| isolationPoolOptions: this.options.isolationPoolOptions, |
There was a problem hiding this comment.
I like the general idea (thanks!), but I'm not sure we want to explicitly add the options here, rather than spread this.options. Was there a particular reason for doing that here that I'm missing?
There was a problem hiding this comment.
Thank you! Definitely, using spread operator makes it much easier to read.
I was having troubles with making types work, because of a known Redis issue regarding RedisClientType. I added a solution towards it which was previously discussed in their repo.
| this.client.on( | ||
| 'error', | ||
| this.options.onError ? this.options.onError : () => {}, | ||
| ); |
There was a problem hiding this comment.
I think we can merge this, but there's just one little thing I missed before - it would probably make more sense to skip setting this handler at all if onError isn't defined, rather than making it an empty function.
If you could make that change, we can go ahead and merge this 🙂
paulomarg
left a comment
paulomarg
left a comment
There was a problem hiding this comment.
🎉
I think you might need to rebase so that we can merge!
3 participants
WHY are these changes introduced?
Fixes #440
If redis client library looses connection an error is thrown which is currently unhandled. This makes it especially problematic in ephemeral cloud environments like Heroku which leaves the app in a crashed state after idle time.
WHAT is this pull request doing?
4.1.0which addresses a reconnection issue.Type of change
Checklist