Background Refresh can be triggered multiple times.

[ricall]

When using background refresh in the wrap function the fn function can be called multiple times.

Expected Behaviour: When wrap is performing a background refresh the fn function should only be called once.

Actual Behaviour: The fn function is not cached correctly for the time period the fn function takes to return a new value. If multiple requests are made in this period then the fn function will end up being called for each request.

Test reproducing the bug - in caching.test.ts

  it('only calls fn once when refreshing the cache', async () => {
    const key = faker.string.alpha(20);
    let callCount = 0;
    cache = await caching('memory', {
      ttl: 5 * 1000,
      refreshThreshold: 4 * 1000,
    });
    const resolveAfter =
      (timeout: number, value: number) => (): Promise<number> =>
        new Promise((resolve) =>
          setTimeout(() => {
            callCount++;
            resolve(value);
          }, timeout),
        );
    const delay = (timeout: number) =>
      new Promise((resolve) => setTimeout(resolve, timeout));

    let value = await cache.wrap(key, resolveAfter(100, 1));
    expect(value).toEqual(1);
    expect(callCount).toEqual(1);

    await delay(1100);
    for (let i = 0; i < 6; i++) {
      // Only the first fn should be called - returning 2
      value = await cache.wrap(key, resolveAfter(2000, 2 + i));
      expect(value).toEqual(1);
      expect(callCount).toEqual(1);
    }

    await delay(2100);
    value = await cache.wrap(key, resolveAfter(2000, 8));
    expect(value).toEqual(2);
    expect(callCount).toEqual(2);
  });

Fix in caching.ts

    wrap: async <T>(key: string, fn: () => Promise<T>, ttl?: WrapTTL<T>) => {
      return coalesceAsync(key, async () => {
        const value = await store.get<T>(key);
        if (value === undefined) {
          const result = await fn();
          const cacheTTL = typeof ttl === 'function' ? ttl(result) : ttl;
          await store.set<T>(key, result, cacheTTL);
          return result;
        } else if (args?.refreshThreshold) {
          const cacheTTL = typeof ttl === 'function' ? ttl(value) : ttl;
          const remainingTtl = await store.ttl(key);
          if (remainingTtl !== -1 && remainingTtl < args.refreshThreshold) {
            coalesceAsync(`+++${key}`, fn).then((result) =>
              store.set<T>(key, result, cacheTTL),
            );
          }
        }
        return value;
      });
    },

[ricall]

The test could be rewritten to use vi.useFakeTimers to speed up the test, but it is likely to impact other tests.

[jaredwray]

@ricall - this will be fixed on next release. Pull request is now merged