Extend configuration for consumer handling of retry on container attach (#26698)

Currently, we continue to retry retriable errors indefinitely on
container creation. However in case of certain consumers where there is
limited time (30/60 seconds) for the create, we should not have infinite
retries even on retriable errors.

If the consumer of `attach` wants to handle the retry mechanism, they
can enable the `"Fluid.Container.DisableCloseOnAttachFailure"` flag.
This flag will cause no internal retries to occur and let any error
surface to the consumer of `attach`.


[AB#57593](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/57593)

Author: kian-thompson

packages/common/container-definitions/src/loader.ts

@@ -421,7 +421,7 @@ export interface IContainer extends IEventProvider<IContainerEvents> {
 	 *
 	 * By default, the container will close if attach fails.
 	 * However, closure can now be avoided in most cased by setting:
-	 * Fluid.Container.RetryOnAttachFailure to true
+	 * Fluid.Container.DisableCloseOnAttachFailure to true
 	 * via the config provider passed to the loader.
 	 *
 	 * If attach fails, check the closed property to discover if retry is possible.
@@ -431,7 +431,9 @@ export interface IContainer extends IEventProvider<IContainerEvents> {
 	 */
 	attach(
 		request: IRequest,
-		attachProps?: { deltaConnection?: "none" | "delayed" },
+		attachProps?: {
+			deltaConnection?: "none" | "delayed";
+		},
 	): Promise<void>;
 
 	/**

packages/loader/container-loader/src/container.ts

@@ -1301,7 +1301,9 @@ export class Container
 					});
 
 					// only enable the new behavior if the config is set
-					if (this.mc.config.getBoolean("Fluid.Container.RetryOnAttachFailure") !== true) {
+					if (
+						this.mc.config.getBoolean("Fluid.Container.DisableCloseOnAttachFailure") !== true
+					) {
 						attachP = attachP.catch((error) => {
 							throw normalizeErrorAndClose(error);
 						});
@@ -1547,6 +1549,12 @@ export class Container
 				service.on("metadataUpdate", this.metadataUpdateHandler);
 			}
 		} else {
+			// When DisableCloseOnAttachFailure is enabled, use no internal retries
+			// The consumer will own the retry policy
+			const disableCloseOnAttachFailure =
+				this.mc.config.getBoolean("Fluid.Container.DisableCloseOnAttachFailure") === true;
+			const maxRetries = disableCloseOnAttachFailure ? 0 : undefined;
+
 			service = await runWithRetry(
 				async () =>
 					this.serviceFactory.createContainer(
@@ -1560,6 +1568,7 @@ export class Container
 				{
 					cancel: this._deltaManager.closeAbortController.signal,
 				}, // progress
+				maxRetries,
 			);
 		}
 		return service;

packages/loader/container-loader/src/test/loader.spec.ts

@@ -98,7 +98,7 @@ describe("loader unit test", () => {
 			urlResolver: failProxy(),
 			configProvider: {
 				getRawConfig: (name): ConfigTypes =>
-					name === "Fluid.Container.RetryOnAttachFailure" ? true : undefined,
+					name === "Fluid.Container.DisableCloseOnAttachFailure" ? true : undefined,
 			},
 		});
 		const detached = await loader.createDetachedContainer({ package: "none" });
@@ -136,7 +136,7 @@ describe("loader unit test", () => {
 			}),
 			configProvider: {
 				getRawConfig: (name): ConfigTypes =>
-					name === "Fluid.Container.RetryOnAttachFailure" ? true : undefined,
+					name === "Fluid.Container.DisableCloseOnAttachFailure" ? true : undefined,
 			},
 		});
 		const detached = await loader.createDetachedContainer({ package: "none" });

packages/loader/driver-utils/src/runWithRetry.ts

@@ -8,6 +8,7 @@ import { delay } from "@fluidframework/core-utils/internal";
 import { DriverErrorTypes } from "@fluidframework/driver-definitions/internal";
 import {
 	isFluidError,
+	wrapError,
 	type ITelemetryLoggerExt,
 } from "@fluidframework/telemetry-utils/internal";
 
@@ -57,6 +58,7 @@ export async function runWithRetry<T>(
 	fetchCallName: string,
 	logger: ITelemetryLoggerExt,
 	progress: IProgress,
+	maxRetries?: number,
 ): Promise<T> {
 	let result: T | undefined;
 	let success = false;
@@ -122,6 +124,35 @@ export async function runWithRetry<T>(
 			}
 
 			numRetries++;
+
+			// Check if max retries limit has been reached
+			if (maxRetries !== undefined && numRetries > maxRetries) {
+				logger.sendTelemetryEvent(
+					{
+						eventName: `${fetchCallName}_maxRetriesExceeded`,
+						retry: numRetries - 1,
+						maxRetries,
+						duration: performanceNow() - startTime,
+						fetchCallName,
+					},
+					error,
+				);
+				// Wrap the original error to preserve its details while marking it non-retriable
+				throw wrapError(
+					error,
+					(message) =>
+						new NonRetryableError(
+							`runWithRetry failed after max retries: ${message}`,
+							DriverErrorTypes.genericError,
+							{
+								driverVersion: pkgVersion,
+								fetchCallName,
+								maxRetries,
+							},
+						),
+				);
+			}
+
 			lastError = error;
 			// Wait for the calculated time before retrying.
 			retryAfterMs = calculateMaxWaitTime(retryAfterMs, error);

packages/loader/driver-utils/src/test/runWithRetry.spec.ts

@@ -221,4 +221,100 @@ describe("runWithRetry Tests", () => {
 			assert.strictEqual((error as any).reason, "Sample abort reason");
 		}
 	});
+
+	it("Should stop retrying after maxRetries is exceeded", async () => {
+		let retryTimes = 0;
+		const api = async (): Promise<boolean> => {
+			retryTimes += 1;
+			const error = new Error("Throw error");
+			// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access -- TODO: use a real type
+			(error as any).canRetry = true;
+			throw error;
+		};
+
+		try {
+			await runWithFastSetTimeout(async () => runWithRetry(api, "test", logger, {}, 3));
+			assert.fail("Should not succeed");
+		} catch (error) {
+			// Verify the wrapped error includes the original error message
+			// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment -- TODO: use a real type
+			const errorMessage = (error as any).message;
+			assert.strictEqual(errorMessage, "runWithRetry failed after max retries: Throw error");
+			// Verify the original error is preserved in the cause property
+			// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment -- TODO: use a real type
+			const causeMessage = (error as any).cause?.message;
+			assert.strictEqual(causeMessage, "Throw error");
+		}
+		// Initial call + maxRetries attempts
+		assert.strictEqual(retryTimes, 4, "Should retry exactly maxRetries times");
+	});
+
+	it("Should succeed before maxRetries is exceeded", async () => {
+		let retryTimes = 0;
+		const api = async (): Promise<boolean> => {
+			retryTimes += 1;
+			// Succeed on the 3rd attempt (after 2 failures)
+			if (retryTimes < 3) {
+				const error = new Error("Throw error");
+				// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access -- TODO: use a real type
+				(error as any).canRetry = true;
+				throw error;
+			}
+			return true;
+		};
+
+		const success = await runWithFastSetTimeout(async () =>
+			runWithRetry(api, "test", logger, {}, 5),
+		);
+		assert.strictEqual(success, true, "Should succeed");
+		assert.strictEqual(retryTimes, 3, "Should take 3 attempts to succeed");
+	});
+
+	it("Should retry infinitely when maxRetries is undefined", async () => {
+		const totalRetries = 10;
+		let retryTimes = 0;
+		const api = async (): Promise<boolean> => {
+			retryTimes += 1;
+			if (retryTimes <= totalRetries) {
+				const error = new Error("Throw error");
+				// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access -- TODO: use a real type
+				(error as any).canRetry = true;
+				throw error;
+			}
+			return true;
+		};
+
+		const success = await runWithFastSetTimeout(async () =>
+			runWithRetry(api, "test", logger, {}),
+		);
+		assert.strictEqual(success, true, "Should succeed");
+		assert.strictEqual(retryTimes, totalRetries + 1, "Should retry until success");
+	});
+
+	it("Should fail immediately with maxRetries set to 0", async () => {
+		let retryTimes = 0;
+		const api = async (): Promise<boolean> => {
+			retryTimes += 1;
+			const error = new Error("Throw error");
+			// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access -- TODO: use a real type
+			(error as any).canRetry = true;
+			throw error;
+		};
+
+		try {
+			await runWithFastSetTimeout(async () => runWithRetry(api, "test", logger, {}, 0));
+			assert.fail("Should not succeed");
+		} catch (error) {
+			// Verify the wrapped error includes the original error message
+			// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment -- TODO: use a real type
+			const errorMessage = (error as any).message;
+			assert.strictEqual(errorMessage, "runWithRetry failed after max retries: Throw error");
+			// Verify the original error is preserved in the cause property
+			// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment -- TODO: use a real type
+			const causeMessage = (error as any).cause?.message;
+			assert.strictEqual(causeMessage, "Throw error");
+		}
+		// Only the initial call, no retries
+		assert.strictEqual(retryTimes, 1, "Should not retry at all");
+	});
 });

packages/test/test-end-to-end-tests/src/test/blobs.spec.ts

@@ -583,7 +583,7 @@ function serializationTests({
 					loaderProps: {
 						documentServiceFactory,
 						configProvider: createTestConfigProvider({
-							"Fluid.Container.RetryOnAttachFailure": true,
+							"Fluid.Container.DisableCloseOnAttachFailure": true,
 						}),
 					},
 				});

packages/test/test-end-to-end-tests/src/test/detachedContainerTests.spec.ts

@@ -956,7 +956,7 @@ describeCompat("Detached Container", "NoCompat", (getTestObjectProvider, apis) =
 				{
 					configProvider: {
 						getRawConfig: (name) =>
-							name === "Fluid.Container.RetryOnAttachFailure" ? true : undefined,
+							name === "Fluid.Container.DisableCloseOnAttachFailure" ? true : undefined,
 					},
 				},
 			);

packages/test/test-end-to-end-tests/src/test/serializeAfterFailedAttach.spec.ts

@@ -87,7 +87,7 @@ describeCompat(
 				logger: provider.logger,
 				configProvider: {
 					getRawConfig: (name) =>
-						name === "Fluid.Container.RetryOnAttachFailure" ? true : undefined,
+						name === "Fluid.Container.DisableCloseOnAttachFailure" ? true : undefined,
 				},
 			});
 			return testLoader;