From 21288baf831586756cb7c9c6642a2254b5cc9e9a Mon Sep 17 00:00:00 2001 From: Abram Sanderson Date: Mon, 16 Sep 2024 15:37:50 -0700 Subject: [PATCH] Add diagnostics to ConsensusQueue in handle validation tests (#22531) ## Description Modifies `ConsensusQueue`'s generic "store a piece of data" implementation in `handleValidation.ts` to give more information on a couple theoretical error methods. This test timed out on a recent tinylicious run where based on telemetry, it seems like the test got past the point of loading the 2nd container. Thus it was likely a deadlock awaiting this promise. I've been unable to reproduce this behavior locally. The goal here is to get information about the failure mode the next time this race condition happens. --------- Co-authored-by: Abram Sanderson --- .../src/test/handleValidation.spec.ts | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/packages/test/test-end-to-end-tests/src/test/handleValidation.spec.ts b/packages/test/test-end-to-end-tests/src/test/handleValidation.spec.ts index 10bcf968c178..3c3d771311a2 100644 --- a/packages/test/test-end-to-end-tests/src/test/handleValidation.spec.ts +++ b/packages/test/test-end-to-end-tests/src/test/handleValidation.spec.ts @@ -39,6 +39,7 @@ import { createAndAttachContainer, ITestFluidObject, type ITestObjectProvider, + timeoutAwait, } from "@fluidframework/test-utils/internal"; import { ITree, @@ -376,14 +377,25 @@ describeCompat("handle validation", "NoCompat", (getTestObjectProvider, apis) => await queue.add(handle); }, async readHandle(): Promise { - const value = await new Promise((resolve, reject) => { - queue - .acquire(async (result: FluidObject) => { - resolve(result); - return ConsensusResult.Release; - }) - .catch((error) => reject(error)); - }); + const value = await timeoutAwait( + new Promise((resolve, reject) => { + queue + .acquire(async (result: FluidObject) => { + resolve(result); + return ConsensusResult.Release; + }) + .then((wasNonempty) => { + if (!wasNonempty) { + // This could happen if a test never calls `storeHandle` before attempting to read it. + // Other modes of failure are possible (e.g. correctness issues in ConsensusQueue causing it to have the wrong data). + // Resolving the promise with `undefined` here could be another reasonable option, but this gives slightly more information. + reject(new Error("No values found in consensus queue.")); + } + }) + .catch((error) => reject(error)); + }), + { errorMsg: "Timeout waiting for acquiring value from consensus queue." }, + ); return value; }, handle: queue.handle,