No error shown upon sails lift when connect-redis is unable to connect to Redis

[sgress454]

See #3589

When session adapter is set to redis in config/sessions.js and connect-redis is properly installed, but cannot connect to a Redis instance using the configured settings (perhaps because no Redis instance is running!) no error is displayed in the console, making it appear as if the server lifted without incident when in fact the app will be unable to handle sessions.

[crobinson42]

+1

[mikermcneil]

For anyone who wants to take a pass at this, here's the spot in core where we get access to the session store instance: https://github.com/balderdashy/sails/blob/master/lib/hooks/session/index.js#L172

If I was able to dive into this right now, what I would do is probably console.log(sessionConfig.store) and see what I could see. Then I would check out the relevant file in the source code for connect-redis and see if I could figure out where the raw redis client instance was lurking in there. Then I would double-check the README to see if direct access to the redis client was a documented part of connect-redis. If not, I would post back here with what I found, then I would create an issue or PR in connect-redis (depending on what their contribution guidelines suggest) that describes our use case, links to this issue, and either implements or asks for official support for handling errors via some kind of notifier interface (preferably a function we pass in with the rest of the redis config, but agreeing to document the home for the redis client instance is also good enough).

On the other hand, if I found that the redis client was already an officially documented part of the store instance, or that there was another way to bind an error handler, awesome! I would implement connect-redis-specific error handling in that file I linked to above in the session hook within Sails core.

If dealing with error handling ends up meaning using access to the raw redis client instance, check out https://github.com/mikermcneil/machinepack-redis/blob/master/machines/get-connection.js#L81 for an example. At the point in the code where we'd be getting access to the client instance, it has likely already had an error handler bound by the code in connect-redis (which is why it is silently hanging rather than crashing the server). We would probably want to bind another error handler on the client instance (e.g. sessionConfig.store._redisClientOrWhatever.once('error', function(err){ })), then log a human-readable warning message explaining that connect-redis ran into a problem + the stack trace; i.e. something like sails.log.warn('The connect-redis session store encountered an error. Details:\n',err)

If anyone reading this has time to help with that, I'd appreciate it. No need to send a proposal since this isn't a change to usage-- a patch PR that links back to this issue would be perfect. If no one else in the community can get to this in the next couple of weeks, I'll look at it asap. Thanks!