diff --git a/.changeset/improbable-word-combination.md b/.changeset/improbable-word-combination.md index 7b2e78ba32..1b1abe2cf1 100644 --- a/.changeset/improbable-word-combination.md +++ b/.changeset/improbable-word-combination.md @@ -2,15 +2,16 @@ '@backstage/backend-common': patch --- -Plugin developers may now provide handlers for connection errors emitted by cache stores. +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 cacheClient = somePluginCache.getClient({ - defaultTtl: 3600000, +const cacheManager = CacheManager.fromConfig(config, { onError: e => { - logger.error(`There was a cache connection problem: ${e.message}`); - execOtherErrorHandlingLogic(); + if (isSomehowUnrecoverable(e)) { + gracefullyShutThingsDown(); + process.exit(1); + } }, }); ``` diff --git a/packages/backend-common/src/cache/CacheManager.test.ts b/packages/backend-common/src/cache/CacheManager.test.ts index 05f94d51ec..97c1714c53 100644 --- a/packages/backend-common/src/cache/CacheManager.test.ts +++ b/packages/backend-common/src/cache/CacheManager.test.ts @@ -88,13 +88,12 @@ describe('CacheManager', () => { it('attaches error handler to client', () => { const pluginId = 'error-test'; - const handler = jest.fn(); - manager.forPlugin(pluginId).getClient({ onError: handler }); + 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', handler); + expect(realClient.on).toHaveBeenCalledWith('error', expect.any(Function)); }); it('provides different plugins different cache clients', async () => { @@ -174,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); + }); + }); }); diff --git a/packages/backend-common/src/cache/CacheManager.ts b/packages/backend-common/src/cache/CacheManager.ts index ba736d6121..e6699e628e 100644 --- a/packages/backend-common/src/cache/CacheManager.ts +++ b/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,21 +55,34 @@ 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; } /** @@ -74,10 +95,16 @@ export class CacheManager { getClient: (opts = {}): CacheClient => { const concreteClient = this.getClientWithTtl(pluginId, opts.defaultTtl); - // Attach error handler if provided. - if (typeof opts?.onError === 'function') { - concreteClient.on('error', opts.onError); - } + // 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, diff --git a/packages/backend-common/src/cache/types.ts b/packages/backend-common/src/cache/types.ts index 87b1580dbb..30db53d420 100644 --- a/packages/backend-common/src/cache/types.ts +++ b/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 = { @@ -23,12 +24,21 @@ type ClientOptions = { * can be configured per entry at set-time). */ 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?: (err: Error) => void; + onError?: OptionalOnError; }; /**