Merge pull request #5668 from backstage/iameap/cache-improvements

Handle/log cache connection errors and allow integrators to pass custom handlers

.changeset/improbable-word-combination.md

@@ -0,0 +1,17 @@
+---
+'@backstage/backend-common': patch
+---
+
+All cache-related connection errors are now handled and logged by the cache manager. App Integrators may provide an optional error handler when instantiating the cache manager if custom error handling is needed.
+
+```typescript
+// Providing an error handler
+const cacheManager = CacheManager.fromConfig(config, {
+  onError: e => {
+    if (isSomehowUnrecoverable(e)) {
+      gracefullyShutThingsDown();
+      process.exit(1);
+    }
+  },
+});
+```

packages/backend-common/api-report.md

@@ -73,7 +73,7 @@ export interface CacheClient {
 // @public
 export class CacheManager {
     forPlugin(pluginId: string): PluginCacheManager;
-    static fromConfig(config: Config): CacheManager;
+    static fromConfig(config: Config, options?: CacheManagerOptions): CacheManager;
     }
 
 // @public (undocumented)

packages/backend-common/src/cache/CacheClient.ts

@@ -92,7 +92,8 @@ export class DefaultCacheClient implements CacheClient {
     const wellFormedKey = Buffer.from(candidateKey).toString('base64');
 
     // Memcache in particular doesn't do well with keys > 250 bytes.
-    if (wellFormedKey.length < 250) {
+    // Padded because a plugin ID is also prepended to the key.
+    if (wellFormedKey.length < 200) {
       return wellFormedKey;
     }
 

packages/backend-common/src/cache/CacheManager.test.ts

@@ -86,6 +86,16 @@ describe('CacheManager', () => {
       expect(client).toHaveBeenCalledTimes(1);
     });
 
+    it('attaches error handler to client', () => {
+      const pluginId = 'error-test';
+      manager.forPlugin(pluginId).getClient();
+
+      const client = DefaultCacheClient as jest.Mock;
+      const mockCalls = client.mock.calls.splice(-1);
+      const realClient = mockCalls[0][0].client as Keyv;
+      expect(realClient.on).toHaveBeenCalledWith('error', expect.any(Function));
+    });
+
     it('provides different plugins different cache clients', async () => {
       const plugin1Id = 'test1';
       const plugin2Id = 'test2';
@@ -163,4 +173,53 @@ describe('CacheManager', () => {
       expect(mockMemcacheCalls[0][0]).toEqual(expectedHost);
     });
   });
+
+  describe('connection errors', () => {
+    it('uses provided logger', () => {
+      // Set up and inject mock logger.
+      const mockLogger = { child: jest.fn(), error: jest.fn() };
+      mockLogger.child.mockImplementation(() => mockLogger as any);
+      const manager = CacheManager.fromConfig(defaultConfig(), {
+        logger: mockLogger as any,
+      });
+
+      // Set up a cache client using the configured manager.
+      manager.forPlugin('error-logger-test').getClient();
+
+      // Retrieve the error handler attached to the cache client.
+      const client = DefaultCacheClient as jest.Mock;
+      const mockCalls = client.mock.calls.splice(-1);
+      const realClient = mockCalls[0][0].client as Keyv;
+      const realOnError = realClient.on as jest.Mock;
+      const realHandler = realOnError.mock.calls.splice(-1)[0][1];
+
+      // Invoke the actual error handler.
+      const expectedError = new Error('some error');
+      realHandler(expectedError);
+      expect(mockLogger.error).toHaveBeenCalledWith(expectedError);
+    });
+
+    it('calls provided handler', () => {
+      // Set up and inject mock logger.
+      const mockHandler = jest.fn();
+      const manager = CacheManager.fromConfig(defaultConfig(), {
+        onError: mockHandler,
+      });
+
+      // Set up a cache client using the configured manager.
+      manager.forPlugin('error-handler-test').getClient();
+
+      // Retrieve the error handler attached to the cache client.
+      const client = DefaultCacheClient as jest.Mock;
+      const mockCalls = client.mock.calls.splice(-1);
+      const realClient = mockCalls[0][0].client as Keyv;
+      const realOnError = realClient.on as jest.Mock;
+      const realHandler = realOnError.mock.calls.splice(-1)[0][1];
+
+      // Invoke the actual error handler.
+      const expectedError = new Error('some error');
+      realHandler(expectedError);
+      expect(mockHandler).toHaveBeenCalledWith(expectedError);
+    });
+  });
 });

packages/backend-common/src/cache/CacheManager.ts

@@ -18,9 +18,15 @@ import { Config } from '@backstage/config';
 import Keyv from 'keyv';
 // @ts-expect-error
 import KeyvMemcache from 'keyv-memcache';
+import { Logger } from 'winston';
+import { getRootLogger } from '../logging';
 import { DefaultCacheClient, CacheClient } from './CacheClient';
 import { NoStore } from './NoStore';
-import { PluginCacheManager } from './types';
+import {
+  CacheManagerOptions,
+  OptionalOnError,
+  PluginCacheManager,
+} from './types';
 
 /**
  * Implements a Cache Manager which will automatically create new cache clients
@@ -38,8 +44,10 @@ export class CacheManager {
     none: this.getNoneClient,
   };
 
+  private readonly logger: Logger;
   private readonly store: keyof CacheManager['storeFactories'];
   private readonly connection: string;
+  private readonly errorHandler: OptionalOnError;
 
   /**
    * Creates a new CacheManager instance by reading from the `backend` config
@@ -47,25 +55,38 @@ export class CacheManager {
    *
    * @param config The loaded application configuration.
    */
-  static fromConfig(config: Config): CacheManager {
+  static fromConfig(
+    config: Config,
+    options: CacheManagerOptions = {},
+  ): CacheManager {
     // If no `backend.cache` config is provided, instantiate the CacheManager
     // with a "NoStore" cache client.
     const store = config.getOptionalString('backend.cache.store') || 'none';
     const connectionString =
       config.getOptionalString('backend.cache.connection') || '';
-    return new CacheManager(store, connectionString);
+    const logger = (options.logger || getRootLogger()).child({
+      type: 'cacheManager',
+    });
+    return new CacheManager(store, connectionString, logger, options.onError);
   }
 
-  private constructor(store: string, connectionString: string) {
+  private constructor(
+    store: string,
+    connectionString: string,
+    logger: Logger,
+    errorHandler: OptionalOnError,
+  ) {
     if (!this.storeFactories.hasOwnProperty(store)) {
       throw new Error(`Unknown cache store: ${store}`);
     }
+    this.logger = logger;
     this.store = store as keyof CacheManager['storeFactories'];
     this.connection = connectionString;
+    this.errorHandler = errorHandler;
   }
 
   /**
-   * Generates a CacheManagerInstance for consumption by plugins.
+   * Generates a PluginCacheManager for consumption by plugins.
    *
    * @param pluginId The plugin that the cache manager should be created for. Plugin names should be unique.
    */
@@ -73,6 +94,18 @@ export class CacheManager {
     return {
       getClient: (opts = {}): CacheClient => {
         const concreteClient = this.getClientWithTtl(pluginId, opts.defaultTtl);
+
+        // Always provide an error handler to avoid killing the process.
+        concreteClient.on('error', (err: Error) => {
+          // In all cases, just log the error.
+          this.logger.error(err);
+
+          // Invoke any custom error handler if provided.
+          if (typeof this.errorHandler === 'function') {
+            this.errorHandler(err);
+          }
+        });
+
         return new DefaultCacheClient({
           client: concreteClient,
         });

packages/backend-common/src/cache/types.ts

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import { Logger } from 'winston';
 import { CacheClient } from './CacheClient';
 
 type ClientOptions = {
@@ -25,6 +26,21 @@ type ClientOptions = {
   defaultTtl?: number;
 };
 
+export type OptionalOnError = ((err: Error) => void) | undefined;
+
+export type CacheManagerOptions = {
+  /**
+   * An optional logger for use by the PluginCacheManager.
+   */
+  logger?: Logger;
+
+  /**
+   * An optional handler for connection errors emitted from the underlying data
+   * store.
+   */
+  onError?: OptionalOnError;
+};
+
 /**
  * The PluginCacheManager manages access to cache stores that Plugins get.
  */