From 20d6466561163958fe3ace21b84f9f51c2133dd9 Mon Sep 17 00:00:00 2001
From: Liran Cohen <c.liran.c@gmail.com>
Date: Thu, 25 Apr 2024 12:26:52 -0400
Subject: [PATCH] do not cache resolution requests with errors (#496)

---
 .changeset/tall-toes-remember.md              |  5 ++
 .../dids/src/resolver/universal-resolver.ts   |  6 ++-
 .../tests/resolver/universal-resolver.spec.ts | 54 +++++++++++++++++++
 3 files changed, 63 insertions(+), 2 deletions(-)
 create mode 100644 .changeset/tall-toes-remember.md

diff --git a/.changeset/tall-toes-remember.md b/.changeset/tall-toes-remember.md
new file mode 100644
index 000000000..f8d8c9650
--- /dev/null
+++ b/.changeset/tall-toes-remember.md
@@ -0,0 +1,5 @@
+---
+"@web5/dids": patch
+---
+
+Do not cache results that contain a resolution error.
diff --git a/packages/dids/src/resolver/universal-resolver.ts b/packages/dids/src/resolver/universal-resolver.ts
index 3668df00c..93ff11fb8 100644
--- a/packages/dids/src/resolver/universal-resolver.ts
+++ b/packages/dids/src/resolver/universal-resolver.ts
@@ -126,8 +126,10 @@ export class UniversalResolver implements DidResolver, DidUrlDereferencer {
       return cachedResolutionResult;
     } else {
       const resolutionResult = await resolver.resolve(parsedDid.uri, options);
-
-      await this.cache.set(parsedDid.uri, resolutionResult);
+      if (!resolutionResult.didResolutionMetadata.error) {
+        // Cache the resolution result if it was successful.
+        await this.cache.set(parsedDid.uri, resolutionResult);
+      }
 
       return resolutionResult;
     }
diff --git a/packages/dids/tests/resolver/universal-resolver.spec.ts b/packages/dids/tests/resolver/universal-resolver.spec.ts
index e85d289ea..8e6adb4ee 100644
--- a/packages/dids/tests/resolver/universal-resolver.spec.ts
+++ b/packages/dids/tests/resolver/universal-resolver.spec.ts
@@ -19,6 +19,10 @@ describe('UniversalResolver', () => {
       didResolver = new UniversalResolver({ didResolvers: didMethodApis });
     });
 
+    afterEach(() => {
+      sinon.restore();
+    });
+
     it('returns an invalidDid error if the DID cannot be parsed', async () => {
       const didResolutionResult = await didResolver.resolve('unparseable:did');
       expect(didResolutionResult).to.exist;
@@ -39,6 +43,56 @@ describe('UniversalResolver', () => {
       expect(didResolutionResult.didResolutionMetadata).to.have.property('error', 'methodNotSupported');
     });
 
+    it('should not attempt to cache a DID resolution result if the result is an error', async () => {
+      // Create a Sinon spy on the cache.set method
+      const cacheSetSpy = sinon.spy(didResolver['cache'], 'set');
+
+      // stub the underlying JWK Resolver to return an error
+      const resultWithError = {
+        didResolutionMetadata: {
+          error: 'anyError'
+        },
+        didDocument: {
+          id: 'did:jwk:123456789abcdefghi'
+        },
+        didDocumentMetadata: {}
+      };
+
+      const didMethodResolver = sinon.stub(DidJwk, 'resolve').resolves(resultWithError);
+
+      // Resolve a DID
+      const did = 'did:jwk:123456789abcdefghi';
+      await didResolver.resolve(did);
+
+      // expect that the cache.set method was not called
+      expect(cacheSetSpy.called).to.be.false;
+      expect(didMethodResolver.callCount).to.equal(1);
+    });
+
+    it('should set cache for a DID resolution result if the result is not an error', async () => {
+      // Create a Sinon spy on the cache.set method
+      const cacheSetSpy = sinon.spy(didResolver['cache'], 'set');
+
+      // stub the underlying JWK Resolver to not return an error
+      const result = {
+        didResolutionMetadata : {},
+        didDocument           : {
+          id: 'did:jwk:123456789abcdefghi'
+        },
+        didDocumentMetadata: {}
+      };
+
+      const didMethodResolver = sinon.stub(DidJwk, 'resolve').resolves(result);
+
+      // Resolve a DID
+      const did = 'did:jwk:123456789abcdefghi';
+      await didResolver.resolve(did);
+
+      // expect that the cache.set was called once
+      expect(cacheSetSpy.callCount).to.equal(1);
+      expect(didMethodResolver.callCount).to.equal(1);
+    });
+
     it('pass DID JWK resolve test vectors', async () => {
         type TestVector = {
           description: string;