Merge branch 'main' into ignore-images-in-meta-test

Author: sarahs

lib/excluded-links.js

@@ -13,5 +13,6 @@ module.exports = [
   'https://haveibeenpwned.com/',
   'https://www.ilo.org/dyn/normlex/en/f\\?p=NORMLEXPUB:12100:0::NO::P12100_ILO_CODE:P029',
   'https://www.linkedin.com/company/github',
-  'https://www.facebook.com/'
+  'https://www.facebook.com/',
+  'https://ko-fi.com/'
 ]

lib/redis-accessor.js

@@ -8,7 +8,13 @@ const { CI, NODE_ENV, REDIS_URL } = process.env
 const useRealRedis = !CI && NODE_ENV !== 'test' && !!REDIS_URL
 
 class RedisAccessor {
-  constructor ({ databaseNumber = 0, prefix = null, allowSetFailures = false, name = null } = {}) {
+  constructor ({
+    databaseNumber = 0,
+    prefix = null,
+    allowSetFailures = false,
+    allowGetFailures = false,
+    name = null
+  } = {}) {
     const redisClient = useRealRedis
       ? createRedisClient({
           url: REDIS_URL,
@@ -23,6 +29,9 @@ class RedisAccessor {
 
     // Allow for graceful failures if a Redis SET operation fails?
     this._allowSetFailures = allowSetFailures === true
+
+    // Allow for graceful failures if a Redis GET operation fails?
+    this._allowGetFailures = allowGetFailures === true
   }
 
   /** @private */
@@ -105,8 +114,24 @@ Error: ${err.message}`
 
   async get (key) {
     const getAsync = promisify(this._client.get).bind(this._client)
-    const value = await getAsync(this.prefix(key))
-    return value
+    const fullKey = this.prefix(key)
+
+    try {
+      const value = await getAsync(fullKey)
+      return value
+    } catch (err) {
+      const errorText = `Failed to get value from Redis.
+Key: ${fullKey}
+Error: ${err.message}`
+
+      if (this._allowGetFailures === true) {
+        // Allow for graceful failure
+        console.error(errorText)
+        return null
+      }
+
+      throw new Error(errorText)
+    }
   }
 }
 

middleware/render-page.js

@@ -17,6 +17,8 @@ const pageCache = new RedisAccessor({
   prefix: (HEROKU_RELEASE_VERSION ? HEROKU_RELEASE_VERSION + ':' : '') + 'rp',
   // Allow for graceful failures if a Redis SET operation fails
   allowSetFailures: true,
+  // Allow for graceful failures if a Redis GET operation fails
+  allowGetFailures: true,
   name: 'page-cache'
 })
 

tests/unit/redis-accessor.js

@@ -11,13 +11,25 @@ describe('RedisAccessor', () => {
 
   test('has expected instance properties', async () => {
     const instance = new RedisAccessor()
-    expect(Object.keys(instance).sort()).toEqual(['_allowSetFailures', '_client', '_prefix'])
+    expect(Object.keys(instance).sort()).toEqual(['_allowGetFailures', '_allowSetFailures', '_client', '_prefix'])
   })
 
   test('has expected static methods', async () => {
     expect(typeof RedisAccessor.translateSetArguments).toBe('function')
   })
 
+  describe('#_allowGetFailures property', () => {
+    test('defaults to false', async () => {
+      const instance = new RedisAccessor()
+      expect(instance._allowGetFailures).toBe(false)
+    })
+
+    test('is expected value', async () => {
+      const instance = new RedisAccessor({ allowGetFailures: true })
+      expect(instance._allowGetFailures).toBe(true)
+    })
+  })
+
   describe('#_allowSetFailures property', () => {
     test('defaults to false', async () => {
       const instance = new RedisAccessor()
@@ -299,5 +311,45 @@ Error: Redis ReplyError`
 
       expect(callbackSpy).toHaveBeenCalledWith(null, 'myValue')
     })
+
+    test('resolves to null if Redis replies with an error and `allowGetFailures` option is set to true', async () => {
+      // Temporarily override `console.error`
+      const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation()
+
+      const instance = new RedisAccessor({ prefix: 'myPrefix', allowGetFailures: true })
+      instance._client.get = jest.fn((...args) => args.pop()(new Error('Redis ReplyError')))
+
+      const result = await instance.get('myKey', 'myValue')
+
+      expect(result).toBe(null)
+      expect(consoleErrorSpy).toBeCalledWith(
+        `Failed to get value from Redis.
+Key: myPrefix:myKey
+Error: Redis ReplyError`
+      )
+
+      // Restore `console.error`
+      consoleErrorSpy.mockRestore()
+    })
+
+    test('rejects if Redis replies with an error and `allowGetFailures` option is not set to true', async () => {
+      // Temporarily override `console.error`
+      const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation()
+
+      const instance = new RedisAccessor({ prefix: 'myPrefix' })
+      instance._client.get = jest.fn((...args) => args.pop()(new Error('Redis ReplyError')))
+
+      await expect(instance.get('myKey')).rejects.toThrowError(
+        new Error(`Failed to get value from Redis.
+Key: myPrefix:myKey
+Error: Redis ReplyError`
+        )
+      )
+
+      expect(consoleErrorSpy).not.toBeCalled()
+
+      // Restore `console.error`
+      consoleErrorSpy.mockRestore()
+    })
   })
 })