Cacheable - wrap should not cache errors by default when function thr…

…ows error (#898)

* cacheable - handling wrap and not caching errors by default

* fixing lint issues

* updating to fix with emit on errors

* adding in cacheErrors property

* Update README.md

packages/cacheable/README.md

@@ -347,11 +347,24 @@ const cache = new CacheableMemory();
 const wrappedFunction = cache.wrap(syncFunction, { ttl: '1h', key: 'syncFunction' });
 console.log(wrappedFunction(2)); // 4
 console.log(wrappedFunction(2)); // 4 from cache
-console.log(cache.get('syncFunction')); // 4
 ```
 
 In this example we are wrapping a `sync` function in a cache with a `ttl` of `1 hour`. This will cache the result of the function for `1 hour` and then expire the value. You can also set the `key` property in the `wrap()` options to set a custom key for the cache.
 
+When an error occurs in the function it will not cache the value and will return the error. This is useful if you want to cache the results of a function but not cache the error. If you want it to cache the error you can set the `cacheError` property to `true` in the `wrap()` options. This is disabled by default.
+
+```javascript
+import { CacheableMemory } from 'cacheable';
+const syncFunction = (value: number) => {
+  throw new Error('error');
+};
+
+const cache = new CacheableMemory();
+const wrappedFunction = cache.wrap(syncFunction, { ttl: '1h', key: 'syncFunction', cacheError: true });
+console.log(wrappedFunction()); // error
+console.log(wrappedFunction()); // error from cache
+```
+
 # Keyv Storage Adapter - KeyvCacheableMemory
 
 `cacheable` comes with a built-in storage adapter for Keyv called `KeyvCacheableMemory`. This takes `CacheableMemory` and creates a storage adapter for Keyv. This is useful if you want to use `CacheableMemory` as a storage adapter for Keyv. Here is an example of how to use `KeyvCacheableMemory`:

packages/cacheable/src/coalesce-async.ts

@@ -81,6 +81,7 @@ export async function coalesceAsync<T>(
 			coalesce({key, result});
 			return result;
 		} catch (error: any) {
+			/* c8 ignore next 5 */
 			// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
 			coalesce({key, error});
 			// eslint-disable-next-line @typescript-eslint/only-throw-error

packages/cacheable/src/index.ts

@@ -211,6 +211,10 @@ export class Cacheable extends Hookified {
 	 */
 	public setPrimary(primary: Keyv | KeyvStoreAdapter): void {
 		this._primary = primary instanceof Keyv ? primary : new Keyv(primary);
+		/* c8 ignore next 3 */
+		this._primary.on('error', (error: unknown) => {
+			this.emit(CacheableEvents.ERROR, error);
+		});
 	}
 
 	/**
@@ -220,6 +224,10 @@ export class Cacheable extends Hookified {
 	 */
 	public setSecondary(secondary: Keyv | KeyvStoreAdapter): void {
 		this._secondary = secondary instanceof Keyv ? secondary : new Keyv(secondary);
+		/* c8 ignore next 3 */
+		this._secondary.on('error', (error: unknown) => {
+			this.emit(CacheableEvents.ERROR, error);
+		});
 	}
 
 	public getNameSpace(): string | undefined {

packages/cacheable/src/keyv-memory.ts

@@ -83,6 +83,7 @@ export class KeyvCacheableMemory implements KeyvStoreAdapter {
 	}
 
 	on(event: string, listener: (...arguments_: any[]) => void): this {
+		this.getStore(this._namespace).on(event, listener);
 		return this;
 	}
 

packages/cacheable/src/memory.ts

@@ -1,3 +1,4 @@
+import {Hookified} from 'hookified';
 import {wrapSync, type WrapFunctionOptions} from './wrap.js';
 import {DoublyLinkedList} from './memory-lru.js';
 import {shorthandToTime} from './shorthand-time.js';
@@ -20,7 +21,7 @@ export type CacheableMemoryOptions = {
 	checkInterval?: number;
 };
 
-export class CacheableMemory {
+export class CacheableMemory extends Hookified {
 	private _lru = new DoublyLinkedList<string>();
 	private readonly _hashCache = new Map<string, number>();
 	private readonly _hash0 = new Map<string, CacheableStoreItem>();
@@ -45,6 +46,8 @@ export class CacheableMemory {
 	 * @param {CacheableMemoryOptions} [options] - The options for the CacheableMemory
 	 */
 	constructor(options?: CacheableMemoryOptions) {
+		super();
+
 		if (options?.ttl) {
 			this.setTtl(options.ttl);
 		}

packages/cacheable/src/wrap.ts

@@ -5,6 +5,7 @@ import {type Cacheable, type CacheableMemory} from './index.js';
 export type WrapFunctionOptions = {
 	ttl?: number | string;
 	keyPrefix?: string;
+	cacheErrors?: boolean;
 };
 
 export type WrapOptions = WrapFunctionOptions & {
@@ -22,17 +23,22 @@ export function wrapSync<T>(function_: AnyFunction, options: WrapSyncOptions): A
 
 	return function (...arguments_: any[]) {
 		const cacheKey = createWrapKey(function_, arguments_, keyPrefix);
-
 		let value = cache.get(cacheKey);
 
 		if (value === undefined) {
-			// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
-			value = function_(...arguments_) as T;
-
-			cache.set(cacheKey, value, ttl);
+			try {
+				// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
+				value = function_(...arguments_);
+				cache.set(cacheKey, value, ttl);
+			} catch (error) {
+				cache.emit('error', error);
+				if (options.cacheErrors) {
+					cache.set(cacheKey, error, ttl);
+				}
+			}
 		}
 
-		return value;
+		return value as T;
 	};
 }
 
@@ -41,21 +47,25 @@ export function wrap<T>(function_: AnyFunction, options: WrapOptions): AnyFuncti
 
 	return async function (...arguments_: any[]) {
 		let value;
-		try {
-			const cacheKey = createWrapKey(function_, arguments_, keyPrefix);
 
-			value = await cache.get(cacheKey) as T | undefined;
+		const cacheKey = createWrapKey(function_, arguments_, keyPrefix);
+
+		value = await cache.get(cacheKey) as T | undefined;
 
-			if (value === undefined) {
-				value = await coalesceAsync(cacheKey, async () => {
+		if (value === undefined) {
+			value = await coalesceAsync(cacheKey, async () => {
+				try {
 					// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
 					const result = await function_(...arguments_) as T;
 					await cache.set(cacheKey, result, ttl);
 					return result;
-				});
-			}
-		} catch {
-			// ignore
+				} catch (error) {
+					cache.emit('error', error);
+					if (options.cacheErrors) {
+						await cache.set(cacheKey, error, ttl);
+					}
+				}
+			});
 		}
 
 		return value;

packages/cacheable/test/wrap.test.ts

@@ -143,7 +143,7 @@ describe('wrap function', () => {
 		const cache = new CacheableMemory();
 		const options: WrapSyncOptions = {
 			cache,
-			ttl: '1s',
+			ttl: '100ms',
 			keyPrefix: 'cacheKey',
 		};
 
@@ -156,7 +156,7 @@ describe('wrap function', () => {
 
 		// Expectations
 		expect(result).toBe(result2);
-		await sleep(1500);
+		await sleep(200);
 		const cacheKey = createWrapKey(wrapSync, [1, {first: 'John', last: 'Doe', meta: {age: 30}}], options.keyPrefix);
 		const cacheResult = cache.get(cacheKey);
 		expect(cacheResult).toBe(undefined);
@@ -197,3 +197,112 @@ describe('wrap function with stampede protection', () => {
 		expect(mockFunction).toHaveBeenCalledTimes(1);
 	});
 });
+
+describe('wrap functions handling thrown errors', () => {
+	it('wrapSync should emit an error by default and return undefined but not cache errors', () => {
+		const cache = new CacheableMemory();
+		const options: WrapSyncOptions = {
+			cache,
+			ttl: '1s',
+			keyPrefix: 'cacheKey',
+		};
+
+		const wrapped = wrapSync(() => {
+			throw new Error('Test error');
+		}, options);
+
+		let errorCallCount = 0;
+
+		cache.on('error', error => {
+			expect(error.message).toBe('Test error');
+			errorCallCount++;
+		});
+
+		const result = wrapped();
+
+		expect(result).toBe(undefined);
+		expect(errorCallCount).toBe(1);
+		const values = Array.from(cache.items);
+		expect(values.length).toBe(0);
+	});
+
+	it('wrapSync should cache the error when the property is set', () => {
+		const cache = new CacheableMemory();
+		const options: WrapSyncOptions = {
+			cache,
+			ttl: '1s',
+			keyPrefix: 'cacheKey',
+			cacheErrors: true,
+		};
+
+		const wrapped = wrapSync(() => {
+			throw new Error('Test error');
+		}, options);
+
+		let errorCallCount = 0;
+
+		cache.on('error', error => {
+			expect(error.message).toBe('Test error');
+			errorCallCount++;
+		});
+
+		wrapped();
+		wrapped(); // Should be cached
+
+		expect(errorCallCount).toBe(1);
+	});
+
+	it('wrap should throw an error if the wrapped function throws an error', async () => {
+		const cache = new Cacheable();
+		const error = new Error('Test error');
+		const options: WrapOptions = {
+			cache,
+			ttl: '1s',
+			keyPrefix: 'cacheKey',
+		};
+		const wrapped = wrap(() => {
+			throw error;
+		}, options);
+
+		let errorCallCount = 0;
+
+		cache.on('error', error_ => {
+			expect(error_).toBe(error);
+			errorCallCount++;
+		});
+
+		expect(await wrapped()).toBe(undefined);
+		const cacheKey = createWrapKey(() => {
+			throw error;
+		}, [], options.keyPrefix);
+		const result = await cache.get(cacheKey);
+		expect(result).toBe(undefined);
+		expect(errorCallCount).toBe(1);
+	});
+
+	it('wrap should cache the error when the property is set', async () => {
+		const cache = new Cacheable();
+		const error = new Error('Test error');
+		const options: WrapOptions = {
+			cache,
+			ttl: '1s',
+			keyPrefix: 'cacheKey',
+			cacheErrors: true,
+		};
+		const wrapped = wrap(() => {
+			throw error;
+		}, options);
+
+		let errorCallCount = 0;
+
+		cache.on('error', error_ => {
+			expect(error_).toBe(error);
+			errorCallCount++;
+		});
+
+		await wrapped();
+		await wrapped(); // Should be cached
+
+		expect(errorCallCount).toBe(1);
+	});
+});