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

4.2.2 regression — cannot connect to Redis server #92

Closed
mshick opened this issue Oct 1, 2018 · 6 comments
Closed

4.2.2 regression — cannot connect to Redis server #92

mshick opened this issue Oct 1, 2018 · 6 comments

Comments

@mshick
Copy link

mshick commented Oct 1, 2018

I've encountered a regression between 4.2.1 and 4.2.2, where suddenly my servers are unable to connect to my redis instance. This is replicated in two environments, locally (in docker containers) and in my CI environment.

Is there some major change that could account for this? I currently pass a config object like this:

    cache: [
      {
        engine: 'catbox-redis',
        url: redis://:pass@redis:6379
      }
    ]
@mshick
Copy link
Author

mshick commented Oct 2, 2018

I've traced the problem to this line: https://github.com/hapijs/catbox-redis/blob/master/lib/index.js#L33

Specifically it appears that the new approach to providing settings is not allowing the url is not compatible with the way ioredis now takes config: https://github.com/luin/ioredis#connect-to-redis

You'll see that you would have to either provide a connection URL string, or a connection object. You are simply passing { url: 'redis://:pass@redis:6379' } directly to ioredis.

I was able to solve my issue by converting the URL string into a connection object before passing to catbox-redis.

@marcuspoehls
Copy link
Contributor

Hi Michael, thank you for reporting this!

Seems like a breaking change to existing setups that use the connection string.

I'll have a look at this next week and fix it. This should be fixable on catbox-redis side.

PS: I'm on vacation this week.
PPS: if you want to submit a PR, please feel free 😊

marcuspoehls added a commit that referenced this issue Oct 9, 2018
@marcuspoehls
Copy link
Contributor

@mshick Hey Michael, I’ve tagged release 4.2.3 to address this issue. Thank you for reporting 🤝👍

@mshick
Copy link
Author

mshick commented Oct 9, 2018

@marcuspoehls Thanks for addressing. Sorry I didn't do that PR — I just worked around in my own code and had to move on.

@marcuspoehls
Copy link
Contributor

@mshick No problem! I hope this issue didn’t cause much trouble for you. Have a great day!

@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants