fix: fixes cache leak

Fixes some bugs in the new request cache:

1. When stopping the client we were supposed to remove the response
    cache, instead we were deleting an item from the cache
2. When stopping we were `void`ing the cache deletion - this can
    cause unhandeled promise rejections so switches to `await`ing
    the promise instead
3. When starting the cache was being created asynchronously but without
    waiting for the promise to resolve, so you could stop the client
    before the cache was intialized which would mean it is never
    cleaned up

Author: achingbrain

packages/client/src/client.ts

@@ -9,6 +9,7 @@ import { ipnsValidator } from 'ipns/validator'
 import { parse as ndjson } from 'it-ndjson'
 import defer from 'p-defer'
 import PQueue from 'p-queue'
+import { CACHE_NAME } from './constants.js'
 import { BadResponseError, InvalidRequestError } from './errors.js'
 import { DelegatedRoutingV1HttpApiClientContentRouting, DelegatedRoutingV1HttpApiClientPeerRouting } from './routings.js'
 import type { DelegatedRoutingV1HttpApiClient, DelegatedRoutingV1HttpApiClientInit, GetProvidersOptions, GetPeersOptions, GetIPNSOptions, PeerRecord } from './index.js'
@@ -35,6 +36,7 @@ export class DefaultDelegatedRoutingV1HttpApiClient implements DelegatedRoutingV
   private readonly filterAddrs?: string[]
   private readonly filterProtocols?: string[]
   private readonly inFlightRequests: Map<string, Promise<Response>>
+  private readonly cacheName: string
   private cache?: Cache
   private readonly cacheTTL: number
   /**
@@ -55,17 +57,8 @@ export class DefaultDelegatedRoutingV1HttpApiClient implements DelegatedRoutingV
     this.contentRouting = new DelegatedRoutingV1HttpApiClientContentRouting(this)
     this.peerRouting = new DelegatedRoutingV1HttpApiClientPeerRouting(this)
 
+    this.cacheName = init.cacheName ?? CACHE_NAME
     this.cacheTTL = init.cacheTTL ?? defaultValues.cacheTTL
-    const cacheEnabled = (typeof globalThis.caches !== 'undefined') && (this.cacheTTL > 0)
-
-    if (cacheEnabled) {
-      log('cache enabled with ttl %d', this.cacheTTL)
-      globalThis.caches.open('delegated-routing-v1-cache').then(cache => {
-        this.cache = cache
-      }).catch(() => {
-        this.cache = undefined
-      })
-    }
   }
 
   get [contentRoutingSymbol] (): ContentRouting {
@@ -80,19 +73,30 @@ export class DefaultDelegatedRoutingV1HttpApiClient implements DelegatedRoutingV
     return this.started
   }
 
-  start (): void {
+  async start (): Promise<void> {
+    if (this.started) {
+      return
+    }
+
     this.started = true
+
+    if (this.cacheTTL > 0) {
+      this.cache = await globalThis.caches?.open(this.cacheName)
+
+      if (this.cache != null) {
+        log('cache enabled with ttl %d', this.cacheTTL)
+      }
+    }
   }
 
-  stop (): void {
+  async stop (): Promise<void> {
     this.httpQueue.clear()
     this.shutDownController.abort()
-    this.started = false
 
     // Clear the cache when stopping
-    if (this.cache != null) {
-      void this.cache.delete('delegated-routing-v1-cache')
-    }
+    await globalThis.caches?.delete(this.cacheName)
+
+    this.started = false
   }
 
   async * getProviders (cid: CID, options: GetProvidersOptions = {}): AsyncGenerator<PeerRecord> {

packages/client/src/constants.ts

@@ -0,0 +1 @@
+export const CACHE_NAME = 'delegated-routing-v1-cache'

packages/client/src/index.ts

@@ -144,6 +144,15 @@ export interface DelegatedRoutingV1HttpApiClientInit extends FilterOptions {
    * If 0, caching is disabled
    */
   cacheTTL?: number
+
+  /**
+   * Where a [Cache](https://developer.mozilla.org/en-US/docs/Web/API/Cache) is
+   * available in the global scope, we will store request/responses to avoid
+   * making duplicate requests.
+   *
+   * @default 'delegated-routing-v1-cache'
+   */
+  cacheName?: string
 }
 
 export interface GetIPNSOptions extends AbortOptions {

packages/client/test/client.spec.ts

@@ -0,0 +1,17 @@
+import { expect } from 'aegir/chai'
+import { DefaultDelegatedRoutingV1HttpApiClient } from '../src/client.js'
+import { itBrowser } from './fixtures/it.js'
+
+describe('client', () => {
+  itBrowser('should remove cache on stop', async function () {
+    const cacheName = 'test-cache'
+
+    const client = new DefaultDelegatedRoutingV1HttpApiClient('http://example.com', {
+      cacheName
+    })
+    await client.start()
+    await client.stop()
+
+    await expect(globalThis.caches.has(cacheName)).to.eventually.be.false('did not remove cache on stop')
+  })
+})

packages/client/test/fixtures/it.ts

@@ -0,0 +1,5 @@
+/* eslint-env mocha */
+
+import { isBrowser } from 'wherearewe'
+
+export const itBrowser = (isBrowser ? it : it.skip)

packages/client/test/index.spec.ts

@@ -1,21 +1,21 @@
 /* eslint-env mocha */
 
 import { generateKeyPair } from '@libp2p/crypto/keys'
+import { start } from '@libp2p/interface'
 import { peerIdFromPrivateKey, peerIdFromString } from '@libp2p/peer-id'
 import { multiaddr } from '@multiformats/multiaddr'
 import { expect } from 'aegir/chai'
 import { createIPNSRecord, marshalIPNSRecord } from 'ipns'
 import all from 'it-all'
 import { CID } from 'multiformats/cid'
-import { isBrowser } from 'wherearewe'
 import { createDelegatedRoutingV1HttpApiClient, type DelegatedRoutingV1HttpApiClient } from '../src/index.js'
+import { itBrowser } from './fixtures/it.js'
 
 if (process.env.ECHO_SERVER == null) {
   throw new Error('Echo server not configured correctly')
 }
 
 const serverUrl = process.env.ECHO_SERVER
-const itBrowser = (isBrowser ? it : it.skip)
 
 describe('delegated-routing-v1-http-api-client', () => {
   let client: DelegatedRoutingV1HttpApiClient
@@ -357,6 +357,7 @@ describe('delegated-routing-v1-http-api-client', () => {
     const clientWithShortTTL = createDelegatedRoutingV1HttpApiClient(new URL(serverUrl), {
       cacheTTL: shortTTL
     })
+    await start(clientWithShortTTL)
 
     const cid = CID.parse('QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn')
     const providers = [{