From 7ff6a755752725bd855d5ccdfcff5e8076a2f04a Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 3 May 2021 18:50:07 +0200 Subject: [PATCH] Add an onError config to enable plugin devs to catch and handle underlying client errors Signed-off-by: Eric Peterson --- .../src/cache/CacheClient.test.ts | 129 +++++++++++++++--- .../backend-common/src/cache/CacheClient.ts | 22 ++- .../backend-common/src/cache/CacheManager.ts | 3 +- packages/backend-common/src/cache/types.ts | 1 + 4 files changed, 129 insertions(+), 26 deletions(-) diff --git a/packages/backend-common/src/cache/CacheClient.test.ts b/packages/backend-common/src/cache/CacheClient.test.ts index 2ae7cee494..a08bb245f8 100644 --- a/packages/backend-common/src/cache/CacheClient.test.ts +++ b/packages/backend-common/src/cache/CacheClient.test.ts @@ -20,6 +20,7 @@ import cacheManager from 'cache-manager'; describe('CacheClient', () => { const pluginId = 'test'; const defaultTtl = 60; + const onError = 'returnEmpty'; let client: cacheManager.Cache; const b64 = (k: string) => Buffer.from(k).toString('base64'); @@ -34,7 +35,12 @@ describe('CacheClient', () => { describe('CacheClient.get', () => { it('calls client with normalized key', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); const keyPartial = 'somekey'; await sut.get(keyPartial); @@ -43,7 +49,12 @@ describe('CacheClient', () => { }); it('calls client with normalized key (very long key)', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); const keyPartial = 'x'.repeat(251); await sut.get(keyPartial); @@ -56,7 +67,12 @@ describe('CacheClient', () => { it('performs deserialization on returned data', async () => { const expectedValue = { some: 'value' }; - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); client.get = jest.fn().mockResolvedValue(JSON.stringify(expectedValue)); const actualValue = await sut.get('someKey'); @@ -65,18 +81,41 @@ describe('CacheClient', () => { }); it('returns undefined on any underlying error', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); client.get = jest.fn().mockRejectedValue(undefined); const actualValue = await sut.get('someKey'); expect(actualValue).toStrictEqual(undefined); }); + + it('rejects on underlying error if configured', async () => { + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError: 'reject', + }); + const expectedError = new Error('Some runtime error'); + client.get = jest.fn().mockRejectedValue(expectedError); + + return expect(sut.get('someKey')).rejects.toEqual(expectedError); + }); }); describe('CacheClient.set', () => { it('calls client with normalized key', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); const keyPartial = 'somekey'; await sut.set(keyPartial, {}); @@ -87,7 +126,12 @@ describe('CacheClient', () => { }); it('performs serialization on given data', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); const expectedData = { some: 'value' }; await sut.set('someKey', expectedData); @@ -98,7 +142,12 @@ describe('CacheClient', () => { }); it('passes ttl to client when given', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); const expectedTtl = 3600; await sut.set('someKey', {}, { ttl: expectedTtl }); @@ -109,7 +158,12 @@ describe('CacheClient', () => { }); it('passes defaultTtl to client when not', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); await sut.set('someKey', {}); @@ -118,19 +172,40 @@ describe('CacheClient', () => { expect(actualOptions.ttl).toEqual(defaultTtl); }); - it('does not throw errors on any client error', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + it('does not reject on any client error by default', async () => { + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); client.set = jest.fn().mockRejectedValue(undefined); - expect(async () => { - await sut.set('someKey', {}); - }).not.toThrow(); + return expect(sut.set('someKey', {})).resolves.toBeUndefined(); + }); + + it('rejects on underlying error if configured', async () => { + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError: 'reject', + }); + const expectedError = new Error('Some runtime error'); + client.set = jest.fn().mockRejectedValue(expectedError); + + return expect(sut.set('someKey', {})).rejects.toEqual(expectedError); }); }); describe('CacheClient.delete', () => { it('calls client with normalized key', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); const keyPartial = 'somekey'; await sut.delete(keyPartial); @@ -140,13 +215,29 @@ describe('CacheClient', () => { expect(actualKey).toEqual(b64(`${pluginId}:${keyPartial}`)); }); - it('does not throw errors on any client error', async () => { - const sut = new DefaultCacheClient({ client, defaultTtl, pluginId }); + it('does not reject on any client error by default', async () => { + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError, + }); client.del = jest.fn().mockRejectedValue(undefined); - expect(async () => { - await sut.delete('someKey'); - }).not.toThrow(); + return expect(sut.delete('someKey')).resolves.toBeUndefined(); + }); + + it('rejects on underlying error if configured', async () => { + const sut = new DefaultCacheClient({ + client, + defaultTtl, + pluginId, + onError: 'reject', + }); + const expectedError = new Error('Some runtime error'); + client.del = jest.fn().mockRejectedValue(expectedError); + + return expect(sut.delete('someKey')).rejects.toEqual(expectedError); }); }); }); diff --git a/packages/backend-common/src/cache/CacheClient.ts b/packages/backend-common/src/cache/CacheClient.ts index 523dae6fed..e1f849ccb7 100644 --- a/packages/backend-common/src/cache/CacheClient.ts +++ b/packages/backend-common/src/cache/CacheClient.ts @@ -22,6 +22,7 @@ type CacheClientArgs = { client: cacheManager.Cache; pluginId: string; defaultTtl: number; + onError: 'reject' | 'returnEmpty'; }; type CacheSetOptions = { @@ -60,11 +61,13 @@ export class DefaultCacheClient implements CacheClient { private readonly client: cacheManager.Cache; private readonly defaultTtl: number; private readonly pluginId: string; + private readonly onError: 'reject' | 'returnEmpty'; - constructor({ client, defaultTtl, pluginId }: CacheClientArgs) { + constructor({ client, defaultTtl, pluginId, onError }: CacheClientArgs) { this.client = client; this.defaultTtl = defaultTtl; this.pluginId = pluginId; + this.onError = onError; } async get(key: string): Promise { @@ -72,7 +75,10 @@ export class DefaultCacheClient implements CacheClient { try { const data = (await this.client.get(k)) as string | undefined; return this.deserializeData(data); - } catch { + } catch (e) { + if (this.onError === 'reject') { + throw e; + } return undefined; } } @@ -88,8 +94,10 @@ export class DefaultCacheClient implements CacheClient { await this.client.set(k, data, { ttl: opts.ttl || this.defaultTtl, }); - } catch { - return; + } catch (e) { + if (this.onError === 'reject') { + throw e; + } } } @@ -97,8 +105,10 @@ export class DefaultCacheClient implements CacheClient { const k = this.getNormalizedKey(key); try { await this.client.del(k); - } catch { - return; + } catch (e) { + if (this.onError === 'reject') { + throw e; + } } } diff --git a/packages/backend-common/src/cache/CacheManager.ts b/packages/backend-common/src/cache/CacheManager.ts index 1c2c003b39..13fb75e314 100644 --- a/packages/backend-common/src/cache/CacheManager.ts +++ b/packages/backend-common/src/cache/CacheManager.ts @@ -75,12 +75,13 @@ export class CacheManager { */ forPlugin(pluginId: string): PluginCacheManager { return { - getClient: ({ defaultTtl }): CacheClient => { + getClient: ({ defaultTtl, onError }): CacheClient => { const concreteClient = this.getClientWithTtl(defaultTtl); return new DefaultCacheClient({ client: concreteClient, defaultTtl, pluginId: pluginId, + onError: onError || 'returnEmpty', }); }, }; diff --git a/packages/backend-common/src/cache/types.ts b/packages/backend-common/src/cache/types.ts index c93455e6b2..5cc5782b18 100644 --- a/packages/backend-common/src/cache/types.ts +++ b/packages/backend-common/src/cache/types.ts @@ -18,6 +18,7 @@ import { CacheClient } from './CacheClient'; type ClientOptions = { defaultTtl: number; + onError?: 'reject' | 'returnEmpty'; }; /**