fix: onFactoryError not called with foreground factories

Author: Julien-R44

packages/bentocache/factories/cache_factory.ts

@@ -41,6 +41,7 @@ export class CacheFactory {
       emitter: this.#parameters.emitter,
       lockTimeout: this.#parameters.lockTimeout,
       serializer: this.#parameters.serializer,
+      onFactoryError: this.#parameters.onFactoryError,
     }).serializeL1Cache(this.#l1Options.serialize ?? true)
 
     const stack = new CacheStack('primary', options, {

packages/bentocache/src/bento_cache_options.ts

@@ -3,6 +3,7 @@ import lodash from '@poppinss/utils/lodash'
 import string from '@poppinss/utils/string'
 import { noopLogger } from '@julr/utils/logger'
 
+import type { FactoryError } from './errors.js'
 import { JsonSerializer } from './serializers/json.js'
 import type {
   CacheSerializer,
@@ -76,6 +77,7 @@ export class BentoCacheOptions {
    * If the L1 cache should be serialized
    */
   serializeL1: boolean = true
+  onFactoryError?: (error: FactoryError) => void
 
   constructor(options: RawBentoCacheOptions) {
     this.#options = lodash.merge({}, this, options)
@@ -93,6 +95,7 @@ export class BentoCacheOptions {
     this.serializer = this.#options.serializer ?? defaultSerializer
 
     this.logger = this.#options.logger!.child({ pkg: 'bentocache' })
+    this.onFactoryError = this.#options.onFactoryError
   }
 
   serializeL1Cache(shouldSerialize: boolean = true) {

packages/bentocache/src/cache/factory_runner.ts

@@ -41,9 +41,9 @@ export class FactoryRunner {
       await this.#stack.set(key, result, options)
       return result
     } catch (error) {
-      if (!isBackground) throw new errors.E_FACTORY_ERROR(key, error)
+      options.onFactoryError?.(new errors.E_FACTORY_ERROR(key, error, isBackground))
 
-      options.onFactoryError?.(new errors.E_FACTORY_ERROR(key, error, true))
+      if (!isBackground) throw new errors.E_FACTORY_ERROR(key, error)
     } finally {
       this.#locks.release(key, lockReleaser)
     }

packages/bentocache/tests/cache/error.spec.ts

@@ -0,0 +1,55 @@
+import { test } from '@japa/runner'
+import { sleep } from '@julr/utils/misc'
+
+import { throwingFactory } from '../helpers/index.js'
+import type { FactoryError } from '../../src/errors.js'
+import { CacheFactory } from '../../factories/cache_factory.js'
+
+test.group('Error handling', () => {
+  test('handle foreground factory error', async ({ assert }) => {
+    assert.plan(3)
+
+    const { cache } = new CacheFactory()
+      .withL1L2Config()
+      .merge({
+        onFactoryError: (error: FactoryError) => {
+          if (error.cause instanceof Error) {
+            assert.deepEqual(error.key, 'error')
+            assert.deepEqual(error.cause?.message, 'Factory error')
+            assert.deepEqual(error.isBackgroundFactory, false)
+          }
+        },
+      })
+      .create()
+
+    await cache
+      .getOrSet({ key: 'error', factory: throwingFactory('Factory error') })
+      .catch(() => {})
+  })
+
+  test('handle background factory error', async ({ assert }) => {
+    assert.plan(3)
+
+    const { cache } = new CacheFactory()
+      .withL1L2Config()
+      .merge({
+        grace: '10m',
+        onFactoryError: (error: FactoryError) => {
+          if (error.cause instanceof Error) {
+            assert.deepEqual(error.key, 'error')
+            assert.deepEqual(error.cause?.message, 'Factory error')
+            assert.deepEqual(error.isBackgroundFactory, true)
+          }
+        },
+      })
+      .create()
+
+    await cache.set({ key: 'error', value: 'value', ttl: 10 })
+
+    await sleep(50)
+
+    await cache
+      .getOrSet({ key: 'error', factory: throwingFactory('Factory error') })
+      .catch(() => {})
+  })
+})