From 3c22db0a2ca627eddd1955227a830a99fd901135 Mon Sep 17 00:00:00 2001 From: Michael Peters Date: Thu, 13 Oct 2022 21:11:13 -0700 Subject: [PATCH] common deduped request rejection test --- src/client/tests/webapp/auto-verifier.test.ts | 99 ++++++++++++++++--- src/client/webapp/auto-verifier.ts | 36 ++++++- 2 files changed, 115 insertions(+), 20 deletions(-) diff --git a/src/client/tests/webapp/auto-verifier.test.ts b/src/client/tests/webapp/auto-verifier.test.ts index e543d20..36a02e3 100644 --- a/src/client/tests/webapp/auto-verifier.test.ts +++ b/src/client/tests/webapp/auto-verifier.test.ts @@ -826,7 +826,7 @@ describe('fetchAndVerifyIfNeeded tests', () => { expect(verifyFunc).toHaveBeenCalledTimes(1); }); - test('primary null, unverify during first trusted fetch, second trusted with value - cache miss, unverify during fetch, return from server', async () => { + test('primary null, unverify during first trusted fetch, second trusted with value - cache miss, unverify during fetch, return from server', async () => { const { nextPrimary, nextTrusted, nextVerify, primaryFunc, trustedFunc, verifyFunc @@ -855,19 +855,19 @@ describe('fetchAndVerifyIfNeeded tests', () => { expect(av.trustedPromise).toBe(trusted1.promise); expect(av.trustedStatus).toBe('verifying'); - av.unverify(); + av.unverify(); - expect(av.primaryPromise).toBe(null); + expect(av.primaryPromise).toBe(null); expect(av.trustedPromise).toBe(null); expect(av.trustedStatus).toBe('none'); const cwSpy = jest.spyOn(console, 'warn').mockImplementationOnce(() => {}); // suppress the warning - trusted1.resolve(new BasicWE(2)); - await disjoint(); + trusted1.resolve(new BasicWE(2)); + await disjoint(); - expect(cwSpy).toHaveBeenCalled(); + expect(cwSpy).toHaveBeenCalled(); expect(trustedFunc).toHaveBeenCalledTimes(2); - expect(av.primaryPromise).toBe(null); + expect(av.primaryPromise).toBe(null); expect(av.trustedPromise).toBe(trusted2.promise); expect(av.trustedStatus).toBe('verifying'); @@ -895,8 +895,8 @@ describe('fetchAndVerifyIfNeeded tests', () => { expect(trustedFunc).toHaveBeenCalledTimes(2); expect(verifyFunc).toHaveBeenCalledTimes(1); }); - - test('primary null, unverify during verify, result is original trusted - cache miss, unverify during verify, return from server', async () => { + + test('primary null, unverify during verify, result is original trusted - cache miss, unverify during verify, return from server', async () => { const { nextPrimary, nextTrusted, nextVerify, primaryFunc, trustedFunc, verifyFunc @@ -925,17 +925,17 @@ describe('fetchAndVerifyIfNeeded tests', () => { expect(av.trustedStatus).toBe('verifying'); expect(verifyFunc).toHaveBeenCalledTimes(0); - trusted.resolve(new BasicWE(2)); - await disjoint(); + trusted.resolve(new BasicWE(2)); + await disjoint(); expect(verifyFunc).toHaveBeenCalledWith(null, new BasicWE(2)); expect(av.primaryPromise).toBe(null); expect(av.trustedPromise).toBe(trusted.promise); expect(av.trustedStatus).toBe('verifying'); - av.unverify(); // effectively during the verify function call + av.unverify(); // during the verify function call - expect(av.primaryPromise).toBe(null); + expect(av.primaryPromise).toBe(null); expect(av.trustedPromise).toBe(null); expect(av.trustedStatus).toBe('none'); @@ -943,7 +943,7 @@ describe('fetchAndVerifyIfNeeded tests', () => { verify.resolve(true); await disjoint(); - // result is the value of the trusted promise, but the trusted will be re-called on next fetch + // result is the value of the trusted promise, but the trusted will be re-called on next fetch expect(result).toEqual(new BasicWE(2)); expect(av.primaryPromise).toBe(null); expect(av.trustedPromise).toBe(null); @@ -956,9 +956,78 @@ describe('fetchAndVerifyIfNeeded tests', () => { expect(verifyFunc).toHaveBeenCalledTimes(1); }); + test('ab: a: primary with null, b: primary dedup, ab: reject trusted fetch and unverify, second trusted with value', async () => { + const { + nextPrimary, nextTrusted, nextVerify, + primaryFunc, trustedFunc, verifyFunc + } = getManualsAndMocks(); + + const av = new AutoVerifier(primaryFunc, trustedFunc, verifyFunc); + + const a = fetchAndVerifyIfNeededManually(av, nextPrimary, nextTrusted, nextVerify); + const b = fetchAndVerifyIfNeededManually(av, nextPrimary, nextTrusted, nextVerify); + + expect(av.primaryPromise).toBe(a.primary.promise); + expect(av.trustedPromise).toBe(a.trusted.promise); + expect(av.trustedStatus).toBe('fetching'); + expect(primaryFunc).toHaveBeenCalled(); + expect(trustedFunc).toHaveBeenCalled(); + + a.primary.resolve(null); + await disjoint(); + + expect(av.primaryPromise).toBe(null); + expect(av.trustedPromise).toBe(a.trusted.promise); + expect(av.trustedStatus).toBe('verifying'); + + a.trusted.reject(new Error('rejected')); + av.unverify() + + expect(av.primaryPromise).toBe(null); + expect(av.trustedPromise).toBe(null); + expect(av.trustedStatus).toBe('none'); + + await disjoint(); + + expect(av.primaryPromise).toBe(null); + expect(av.trustedPromise).toBe(b.trusted.promise); + expect(av.trustedStatus).toBe('verifying'); + + expect(verifyFunc).toHaveBeenCalledTimes(0); + b.trusted.resolve(new BasicWE(2)); + await disjoint(); + + expect(verifyFunc).toHaveBeenCalledWith(null, new BasicWE(2)); + expect(av.primaryPromise).toBe(null); + expect(av.trustedPromise).toBe(b.trusted.promise); + expect(av.trustedStatus).toBe('verifying'); + + expect(a.result).toBeUndefined(); + expect(b.result).toBeUndefined(); + a.verify.resolve(true); + await disjoint() + + expect(a.result).toEqual(new BasicWE(2)); + expect(b.result).toEqual(new BasicWE(2)); + expect(av.primaryPromise).toBe(null); + expect(av.trustedPromise).toBe(b.trusted.promise); + expect(av.trustedStatus).toBe('verified'); + + await disjoint(); + + expect(primaryFunc).toHaveBeenCalledTimes(1); + expect(trustedFunc).toHaveBeenCalledTimes(2); + expect(verifyFunc).toHaveBeenCalledTimes(1); + }); + + // TODO: Why not have fetcher and a simple dedup wrapper to remove some complexity from this function? + // This would likely make it possible to get rid of the "else" block in tryResolveTrustedPromise + // Make sure it would work properly for retry functionality + // can't because of primary promise early resolution + // Make sure to do try/catch errors/rejections, verification failures, unverifies in the middle // Multiple tryResolveTrustedPromises - multiple fetchAndVerifyIfNeeded at the same time // Trusted resolves *BEFORE* ensureTrustedFuncReady // Fetching after verified doesn't re-fetch trusted - // Unverify during verify w/ dedups + // Unverify during verify w/ dedups }); diff --git a/src/client/webapp/auto-verifier.ts b/src/client/webapp/auto-verifier.ts index 7300e81..ff6292e 100644 --- a/src/client/webapp/auto-verifier.ts +++ b/src/client/webapp/auto-verifier.ts @@ -231,7 +231,22 @@ export class AutoVerifier { // note: Promises that have already resolved will return the same value when awaited again :) const origTrustedPromise: Promise | null = this.trustedPromise; - const trustedResult = await origTrustedPromise; + let trustedResult = undefined; + try { + trustedResult = await origTrustedPromise; + } catch (e: unknown) { + if (this.trustedPromise == origTrustedPromise) { + throw e; + } else { + console.warn('trusted promise rejected after unverify', e); + if (this.trustedPromise === null) { + this.trustedStatus = 'fetching'; + this.trustedPromise = this.trustedFunc(); + } + await tryResolveTrustedPromise(); + return; + } + } if (this.trustedPromise !== origTrustedPromise) { // we've been invalidated while we were waiting for the trusted result! @@ -281,7 +296,18 @@ export class AutoVerifier { // note: Promises that have already resolved will return the same value when awaited again :) const origTrustedPromise: Promise | null = this.trustedPromise; - const trustedResult = await origTrustedPromise; + let trustedResult = undefined; + try { + trustedResult = await origTrustedPromise; + } catch (e: unknown) { + if (this.trustedPromise == origTrustedPromise) { + throw e; + } else { + console.warn('trusted promise from another path rejected after unverify', e); + await tryResolveTrustedPromise(); + return; + } + } if (this.trustedPromise !== origTrustedPromise) { // we've been invalidated while we were waiting for the trusted result! @@ -321,9 +347,9 @@ export class AutoVerifier { await tryResolveTrustedPromise(); } catch (e: unknown) { if (!resolved) { - this.trustedPromise = null; // suppress warnings - this.primaryPromise = null; // suppress warnings - this.verifyPromise = null; // suppress warnings + this.trustedPromise = null; // suppress warnings about uncaught rejections + this.primaryPromise = null; // suppress warnings about uncaught rejections + this.verifyPromise = null; // suppress warnings about uncaught rejections this.unverify(); // eslint-disable-next-line prefer-promise-reject-errors reject(e as Error);