uppy
uppy copied to clipboard
@uppy/companion: support redis sentinel
Initial checklist
- [X] I understand this is a feature request and questions should be posted in the Community Forum
- [X] I searched issues and couldn’t find anything (or linked relevant results below)
Problem
When I was talking to dev ops about companion deployment, they would have liked it to support redis sentinel natively, which apparently currently is not the case.
Solution
I'm no expert on this topic, but afaict ioredis has sophisticated support for this, while node-redis does not.
When I saw connect-redis
abstracts them away, I was thinking it would be super straightforward to switch from node-redis
to ioredis
- it was until it wasn't :trollface:
Porting createClient
was easy, redis-emitter
not sooo much.
I'm not sending a PR because switching libraries is usually rather controversial and I would like to discuss options first.
Have you considered switching to ioredis
or are there reasons not to do it?
Possibly a breaking change if someone is passing advanced options to the library...
Alternatives
- switch to
ioredis
completely - support
ioredis
andnode-redis
simultaneously - don't change anything (just update
redis-connect
which is slightly outdated)
Excerpt from my patch
diff --git a/package.json b/package.json
index d62b40b93..81ad5bb10 100644
--- a/package.json
+++ b/package.json
@@ -168,7 +168,6 @@
]
},
"resolutions": {
- "@types/[email protected]": "patch:@types/connect-redis@npm:0.0.18#.yarn/patches/@types-connect-redis-npm-0.0.18-4fd2b614d3",
"@types/eslint@^7.2.13": "^8.2.0",
"@types/react": "^17",
"@types/webpack-dev-server": "^4",
diff --git a/packages/@uppy/companion/package.json b/packages/@uppy/companion/package.json
index e6f6362f2..5b29ac696 100644
--- a/packages/@uppy/companion/package.json
+++ b/packages/@uppy/companion/package.json
@@ -36,7 +36,7 @@
"body-parser": "1.20.0",
"chalk": "4.1.2",
"common-tags": "1.8.2",
- "connect-redis": "6.1.3",
+ "connect-redis": "7.1.0",
"content-disposition": "^0.5.4",
"cookie-parser": "1.4.6",
"cors": "^2.8.5",
@@ -52,6 +52,7 @@
"got": "11",
"grant": "5.4.21",
"helmet": "^4.6.0",
+ "ioredis": "5.3.2",
"ipaddr.js": "^2.0.1",
"jsonwebtoken": "8.5.1",
"lodash": "^4.17.21",
@@ -62,7 +63,6 @@
"ms": "2.1.3",
"node-schedule": "2.1.0",
"prom-client": "14.0.1",
- "redis": "4.2.0",
"semver": "7.5.3",
"serialize-error": "^2.1.0",
"serialize-javascript": "^6.0.0",
@@ -73,7 +73,6 @@
},
"devDependencies": {
"@types/compression": "1.7.0",
- "@types/connect-redis": "0.0.18",
"@types/cookie-parser": "1.4.2",
"@types/cors": "2.8.6",
"@types/eslint": "^8.2.0",
diff --git a/packages/@uppy/companion/src/server/emitter/redis-emitter.js b/packages/@uppy/companion/src/server/emitter/redis-emitter.js
index df1badfef..8f7d6615b 100644
--- a/packages/@uppy/companion/src/server/emitter/redis-emitter.js
+++ b/packages/@uppy/companion/src/server/emitter/redis-emitter.js
@@ -1,4 +1,4 @@
-const redis = require('redis')
+const Redis = require('ioredis').default
const { EventEmitter } = require('node:events')
const logger = require('../logger')
@@ -11,13 +11,13 @@ const logger = require('../logger')
module.exports = (redisUrl, redisPubSubScope) => {
const prefix = redisPubSubScope ? `${redisPubSubScope}:` : ''
const getPrefixedEventName = (eventName) => `${prefix}${eventName}`
- const publisher = redis.createClient({ url: redisUrl })
- publisher.on('error', err => logger.error('publisher redis error', err))
+ const publisher = new Redis(redisUrl, { lazyConnect: true })
+ publisher.on('error', err => logger.error('publisher redis error', err.toString()))
let subscriber
const connectedPromise = publisher.connect().then(() => {
subscriber = publisher.duplicate()
- subscriber.on('error', err => logger.error('subscriber redis error', err))
+ subscriber.on('error', err => logger.error('subscriber redis error', err.toString()))
return subscriber.connect()
})
@@ -56,12 +56,17 @@ module.exports = (redisUrl, redisPubSubScope) => {
handlersByThisEventName.delete(handler)
if (handlersByThisEventName.size === 0) handlersByEvent.delete(eventName)
- return subscriber.pUnsubscribe(getPrefixedEventName(eventName), actualHandler)
+ subscriber.off('message', actualHandler)
+ return subscriber.punsubscribe(getPrefixedEventName(eventName))
})
}
function addListener (eventName, handler, _once = false) {
- function actualHandler (message) {
+ function actualHandler (pattern, channel, message) {
+ if (pattern !== getPrefixedEventName(eventName)) {
+ return
+ }
+
if (_once) removeListener(eventName, handler)
let args
try {
@@ -79,7 +84,10 @@ module.exports = (redisUrl, redisPubSubScope) => {
}
handlersByThisEventName.set(handler, actualHandler)
- runWhenConnected(() => subscriber.pSubscribe(getPrefixedEventName(eventName), actualHandler))
+ runWhenConnected(() => {
+ subscriber.on('pmessage', actualHandler)
+ return subscriber.psubscribe(getPrefixedEventName(eventName))
+ })
}
/**
@@ -112,7 +120,9 @@ module.exports = (redisUrl, redisPubSubScope) => {
* @param {string} eventName name of the event
*/
function emit (eventName, ...args) {
- runWhenConnected(() => publisher.publish(getPrefixedEventName(eventName), JSON.stringify(args)))
+ runWhenConnected(() => {
+ return publisher.publish(getPrefixedEventName(eventName), JSON.stringify(args))
+ })
}
/**
@@ -125,7 +135,7 @@ module.exports = (redisUrl, redisPubSubScope) => {
return runWhenConnected(() => {
handlersByEvent.delete(eventName)
- return subscriber.pUnsubscribe(getPrefixedEventName(eventName))
+ return subscriber.punsubscribe(getPrefixedEventName(eventName))
})
}
diff --git a/packages/@uppy/companion/src/server/redis.js b/packages/@uppy/companion/src/server/redis.js
index 10298c6f0..87e2209bc 100644
--- a/packages/@uppy/companion/src/server/redis.js
+++ b/packages/@uppy/companion/src/server/redis.js
@@ -1,4 +1,4 @@
-const redis = require('redis')
+const Redis = require('ioredis').default
const logger = require('./logger')
@@ -8,24 +8,13 @@ let redisClient
* A Singleton module that provides a single redis client through out
* the lifetime of the server
*
- * @param {Record<string, unknown>} [opts] node-redis client options
+ * @param {Record<string, any>} [opts] node-redis client options
*/
function createClient (opts) {
if (!redisClient) {
- // todo remove legacyMode when fixed: https://github.com/tj/connect-redis/issues/361
- redisClient = redis.createClient({ ...opts, legacyMode: true })
+ redisClient = new Redis(opts.url, opts)
- redisClient.on('error', err => logger.error('redis error', err))
-
- ;(async () => {
- try {
- // fire and forget.
- // any requests made on the client before connection is established will be auto-queued by node-redis
- await redisClient.connect()
- } catch (err) {
- logger.error(err.message, 'redis.error')
- }
- })()
+ redisClient.on('error', err => logger.error('redis error', err.toString()))
}
return redisClient
diff --git a/packages/@uppy/companion/src/standalone/index.js b/packages/@uppy/companion/src/standalone/index.js
index fa395782d..9a8f94610 100644
--- a/packages/@uppy/companion/src/standalone/index.js
+++ b/packages/@uppy/companion/src/standalone/index.js
@@ -5,7 +5,7 @@ const morgan = require('morgan')
const { URL } = require('node:url')
const session = require('express-session')
const addRequestId = require('express-request-id')()
-const connectRedis = require('connect-redis')
+const RedisStore = require('connect-redis').default
const logger = require('../server/logger')
const redis = require('../server/redis')
@@ -110,7 +110,6 @@ module.exports = function server (inputCompanionOptions) {
}
if (companionOptions.redisUrl) {
- const RedisStore = connectRedis(session)
const redisClient = redis.client(companionOptions)
// todo next major: change default prefix to something like "companion-session:" and possibly remove this option
sessionOptions.store = new RedisStore({ client: redisClient, prefix: process.env.COMPANION_REDIS_EXPRESS_SESSION_PREFIX || 'sess:' })