Use node-redis for rate limiter (#18416)

* Use [node-]redis as a direct dependency
* Extract Redis client creation to its own module
* Attach extensive logging in the Redis client creation module
* Allow the rate limiter to pass requests when Redis is disconnected
* Update rate-limit-redis
* Default error input to empty object for formatRedisError method
* Provide a name for the rate limiter's Redis client

Co-authored-by: Kevin Heis <heiskr@users.noreply.github.com>
Co-authored-by: Rachael Sewell <rachmari@github.com>

lib/redis/create-client.js

@@ -0,0 +1,79 @@
+const Redis = require('redis')
+
+const { REDIS_MIN_DB, REDIS_MAX_DB } = process.env
+
+// By default, every Redis instance supports database numbers 0 - 15
+const redisMinDb = REDIS_MIN_DB || 0
+const redisMaxDb = REDIS_MAX_DB || 15
+
+function formatRedisError (error = {}) {
+  const { code } = error
+  const preamble = error.constructor.name + (code ? ` with code "${code}"` : '')
+  return preamble + error.toString()
+}
+
+module.exports = function createClient (options = {}) {
+  const { db, name, url } = options
+
+  // If no Redis URL is provided, bail out
+  // NOTE: Could support other options like `host`, `port`, and `path` but
+  // choosing not to for the time being!
+  if (!url) return null
+
+  // Verify database number is within range
+  if (db != null) {
+    if (!Number.isInteger(db) || db < redisMinDb || db > redisMaxDb) {
+      throw new TypeError(
+        `Redis database number must be an integer between ${redisMinDb} and ${redisMaxDb} but was: ${JSON.stringify(db)}`
+      )
+    }
+  }
+
+  // Create the client
+  const client = Redis.createClient(url, {
+    // Only add this configuration for TLS-enabled Redis URL values.
+    // Otherwise, it breaks for local Redis instances without TLS enabled.
+    ...url.startsWith('rediss://') && {
+      tls: {
+        // Required for production Heroku Redis
+        rejectUnauthorized: false
+      }
+    },
+
+    // Expand whatever other options and overrides were provided
+    ...options
+  })
+
+  // If a `name` was provided, use it in the prefix for logging event messages
+  const logPrefix = '[redis' + (name ? ` (${name})` : '') + '] '
+
+  // Add event listeners for basic logging
+  client.on('connect', () => { console.log(logPrefix, 'Connection opened') })
+  client.on('ready', () => { console.log(logPrefix, 'Ready to receive commands') })
+  client.on(
+    'reconnecting',
+    ({
+      attempt,
+      delay,
+      // The rest are unofficial properties but currently supported
+      error,
+      total_retry_time: totalRetryTime,
+      times_connected: timesConnected
+    }) => {
+      console.log(
+        logPrefix,
+        'Reconnecting,',
+        `attempt ${attempt}`,
+        `with ${delay} delay`,
+        `due to ${formatRedisError(error)}.`,
+        `Elapsed time: ${totalRetryTime}.`,
+        `Successful connections: ${timesConnected}.`
+      )
+    }
+  )
+  client.on('end', () => { console.log(logPrefix, 'Connection closed') })
+  client.on('warning', (msg) => { console.warn(logPrefix, 'Warning:', msg) })
+  client.on('error', (error) => { console.error(logPrefix, formatRedisError(error)) })
+
+  return client
+}

middleware/rate-limit.js

@@ -1,6 +1,6 @@
 const rateLimit = require('express-rate-limit')
 const RedisStore = require('rate-limit-redis')
-const Redis = require('ioredis')
+const createRedisClient = require('../lib/redis/create-client')
 
 const isProduction = process.env.NODE_ENV === 'production'
 const { REDIS_URL } = process.env
@@ -15,21 +15,16 @@ module.exports = rateLimit({
   // Don't rate limit requests for 200s and redirects
   // Or anything with a status code less than 400
   skipSuccessfulRequests: true,
-  // When available, use Redis
+  // When available, use Redis; if not, defaults to an in-memory store
   store: REDIS_URL && new RedisStore({
-    client: new Redis(REDIS_URL, {
+    client: createRedisClient({
+      url: REDIS_URL,
       db: rateLimitDatabaseNumber,
-
-      // Only add this configuration for TLS-enabled REDIS_URL values.
-      // Otherwise, it breaks for local Redis instances without TLS enabled.
-      ...REDIS_URL.startsWith('rediss://') && {
-        tls: {
-          // Required for production Heroku Redis
-          rejectUnauthorized: false
-        }
-      }
+      name: 'rate-limit'
     }),
     // 1 minute (or practically unlimited outside of production)
-    expiry: isProduction ? EXPIRES_IN_AS_SECONDS : 1 // Redis configuration in `s`
+    expiry: isProduction ? EXPIRES_IN_AS_SECONDS : 1, // Redis configuration in `s`
+    // If Redis is not connected, let the request succeed as failover
+    passIfNotConnected: true
   })
 })

package-lock.json

@@ -20876,9 +20876,9 @@
       "integrity": "sha512-Hrgsx+orqoygnmhFbKaHE6c296J+HTAQXoxEF6gNupROmmGJRoyzfG3ccAveqCBrwr/2yxQ5BVd/GTl5agOwSg=="
     },
     "rate-limit-redis": {
-      "version": "2.0.0",
-      "resolved": "https://registry.npmjs.org/rate-limit-redis/-/rate-limit-redis-2.0.0.tgz",
-      "integrity": "sha512-06EwXCcJYSKhKDyNPVgnAhwGk0uKxd0mpKdLpdJhnZGRoRzrxHWmponQn8Eq3hMLgbwPVvqdkgun3ZFWKKpuXg==",
+      "version": "2.1.0",
+      "resolved": "https://registry.npmjs.org/rate-limit-redis/-/rate-limit-redis-2.1.0.tgz",
+      "integrity": "sha512-6SAsTCzY0v6UCIKLOLLYqR2XzFmgdtF7jWXlSPq2FrNIZk8tZ7xwBvyGW7GFMCe5I4S9lYNdrSJ9E84rz3/CpA==",
       "requires": {
         "defaults": "^1.0.3",
         "redis": "^3.0.2"

package.json

@@ -75,9 +75,10 @@
     "node-fetch": "^2.6.1",
     "parse5": "^6.0.1",
     "port-used": "^2.0.8",
-    "rate-limit-redis": "^2.0.0",
+    "rate-limit-redis": "^2.1.0",
     "react": "^17.0.1",
     "react-dom": "^17.0.1",
+    "redis": "^3.0.2",
     "rehype-autolink-headings": "^2.0.5",
     "rehype-highlight": "^3.1.0",
     "rehype-raw": "^4.0.2",