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

Don't crash the app if Redis is unavailable on start #84

Open
MichaelMarkieta opened this issue Mar 15, 2017 · 6 comments
Open

Don't crash the app if Redis is unavailable on start #84

MichaelMarkieta opened this issue Mar 15, 2017 · 6 comments
Labels
bug v2.0.0 Road to version 2

Comments

@MichaelMarkieta
Copy link

Just like cache.route() doesn't crash the app if the Redis instance becomes unavailable, the instantiation of the cache should not crash the app if Redis is unavailable at the time of creation.

@SassNinja
Copy link

Apart from not crashing the app on start, it shouldn't cause a crash if redis becomes unavailable on runtime.

After digging into the code I think this.emit('error', error') is the problem and should be replaced with a console.error().
https://github.com/rv-kip/express-redis-cache/blob/master/lib/ExpressRedisCache.js#L85

@rv-kip what do you think abt this adjustment?
Would be great to use the middleware without the risk of crashing my app.

@krazyjakee
Copy link
Collaborator

Let's aim to have this for v2.0.0

@kryten87
Copy link

kryten87 commented Aug 29, 2018

Maybe I'm misreading the issues that earlier posters have, but I encountered this and solved it, simply by ensuring that I included:

cache.on("error", function(error) {
  console.error("cache", error);
});

The error listener seems to be required; without it, the application fails at startup. Perhaps this just needs to be clarified in the documentation?

@SassNinja
Copy link

The error listener seems to be required; without it, the application fails at startup. Perhaps this just needs to be clarified in the documentation? – @Kryten0807

I don't get any error on startup and I don't have such an event listener defined.
But I've tried it anyway: unfortunately the error still remains.
My app crashes when I kill the redis server while it's running.

@kibernetika
Copy link

SassNinja look at Kryten0807 comment

@SassNinja
Copy link

Seems another redis package was also crashing the app when it became unavailable.
That's why I thought it's not working.

I can confirm it's working as described by @Kryten0807
Once you've defined an error listener the app doesn't get crashed anymore if redis becomes unavailable.

However I agree the docs should be clearer and mention this.

rawpixel-vincent pushed a commit to rawpixel-vincent/express-redis-cache that referenced this issue Jul 17, 2021
rawpixel-vincent pushed a commit to rawpixel-vincent/express-redis-cache that referenced this issue Jul 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug v2.0.0 Road to version 2
Projects
None yet
Development

No branches or pull requests

5 participants