revert: "revert: "feat(container-loader): add optional connection retry timeout" (#25814)"

This reverts commit 7aae2a7.

Restores the original
  "feat(container-loader): add optional connection retry timeout"
commit that was inaccessible to customers.

Author: jason-ha

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

@@ -186,6 +186,12 @@ export class ConnectionManager implements IConnectionManager {
 
 	private connectFirstConnection = true;
 
+	/**
+	 * Tracks the initial connection start time for the entire lifetime of the class.
+	 * This is used only for retryConnectionTimeoutMs checks and is never reset.
+	 */
+	private initialConnectionStartTime: number | undefined;
+
 	private _connectionVerboseProps: Record<string, string | number> = {};
 
 	private _connectionProps: ITelemetryBaseProperties = {};
@@ -344,6 +350,7 @@ export class ConnectionManager implements IConnectionManager {
 		reconnectAllowed: boolean,
 		private readonly logger: ITelemetryLoggerExt,
 		private readonly props: IConnectionManagerFactoryArgs,
+		private readonly retryConnectionTimeoutMs?: number,
 	) {
 		this.clientDetails = this.client.details;
 		this.defaultReconnectionMode = this.client.mode;
@@ -522,6 +529,12 @@ export class ConnectionManager implements IConnectionManager {
 		let delayMs = InitialReconnectDelayInMs;
 		let connectRepeatCount = 0;
 		const connectStartTime = performanceNow();
+
+		// Set the initial connection start time only once for the entire lifetime of the class
+		if (this.initialConnectionStartTime === undefined) {
+			this.initialConnectionStartTime = connectStartTime;
+		}
+
 		let lastError: unknown;
 
 		const abortController = new AbortController();
@@ -642,6 +655,31 @@ export class ConnectionManager implements IConnectionManager {
 					this.props.reconnectionDelayHandler(delayMs, origError);
 				}
 
+				// Check if the calculated delay would exceed the remaining timeout with a conservative buffer
+				if (this.retryConnectionTimeoutMs !== undefined) {
+					const elapsedTime = performanceNow() - this.initialConnectionStartTime;
+					const remainingTime = this.retryConnectionTimeoutMs - elapsedTime;
+					const connectionAttemptDuration = performanceNow() - connectStartTime;
+
+					// Use a 75% buffer to be conservative about timeout usage
+					const timeoutBufferThreshold = remainingTime * 0.75;
+					// Estimate the next attempt time as the delay plus the time it took to connect
+					const estimatedNextAttemptTime = delayMs + connectionAttemptDuration;
+
+					if (estimatedNextAttemptTime >= timeoutBufferThreshold) {
+						this.logger.sendTelemetryEvent({
+							eventName: "RetryDelayExceedsConnectionTimeout",
+							attempts: connectRepeatCount,
+							duration: formatTick(elapsedTime),
+							calculatedDelayMs: delayMs,
+							remainingTimeMs: remainingTime,
+							timeoutMs: this.retryConnectionTimeoutMs,
+						});
+						// Throw the error immediately since the estimated time for delay + next connection attempt would exceed our conservative timeout buffer
+						throw normalizeError(origError, { props: fatalConnectErrorProp });
+					}
+				}
+
 				await new Promise<void>((resolve) => {
 					setTimeout(resolve, delayMs);
 				});

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

@@ -248,6 +248,16 @@ export interface IContainerCreateProps {
 	 * protocol implementation for handling the quorum and/or the audience.
 	 */
 	readonly protocolHandlerBuilder?: ProtocolHandlerBuilder;
+
+	/**
+	 * Optional property for specifying a timeout for retry connection loop.
+	 *
+	 * If provided, container will use this value as the maximum time to wait
+	 * for a successful connection before giving up throwing the most recent error.
+	 *
+	 * If not provided, default behavior will be to retry until non-retryable error occurs.
+	 */
+	readonly retryConnectionTimeoutMs?: number;
 }
 
 /**
@@ -617,6 +627,7 @@ export class Container
 	private attachmentData: AttachmentData = { state: AttachState.Detached };
 	private readonly serializedStateManager: SerializedStateManager;
 	private readonly _containerId: string;
+	private readonly _retryConnectionTimeoutMs: number | undefined;
 
 	private lastVisible: number | undefined;
 	private readonly visibilityEventHandler: (() => void) | undefined;
@@ -799,6 +810,7 @@ export class Container
 			scope,
 			subLogger,
 			protocolHandlerBuilder,
+			retryConnectionTimeoutMs,
 		} = createProps;
 
 		// Validate that the Driver is compatible with this Loader.
@@ -814,6 +826,7 @@ export class Container
 		const pendingLocalState = loadProps?.pendingLocalState;
 
 		this._canReconnect = canReconnect ?? true;
+		this._retryConnectionTimeoutMs = retryConnectionTimeoutMs;
 		this.clientDetailsOverride = clientDetailsOverride;
 		this.urlResolver = urlResolver;
 		this.serviceFactory = documentServiceFactory;
@@ -2054,6 +2067,7 @@ export class Container
 					this._canReconnect,
 					createChildLogger({ logger: this.subLogger, namespace: "ConnectionManager" }),
 					props,
+					this._retryConnectionTimeoutMs,
 				),
 		);
 

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

@@ -386,4 +386,136 @@ describe("connectionManager", () => {
 			});
 		});
 	});
+
+	it("Connection retry timeout - throws error when delay exceeds remaining timeout", async () => {
+		// Arrange
+		const timeoutMs = 5000; // 5 second timeout
+		const connectionManager = new ConnectionManager(
+			() => mockDocumentService,
+			() => false,
+			client as IClient,
+			true /* reconnectAllowed */,
+			mockLogger.toTelemetryLogger(),
+			props,
+			timeoutMs, // Set the retry connection timeout
+		);
+
+		// Mock connectToDeltaStream to throw retryable errors that cause delays
+		const stubbedConnectToDeltaStream = stub(mockDocumentService, "connectToDeltaStream");
+		stubbedConnectToDeltaStream.throws(
+			new RetryableError("Throttling error", DriverErrorTypes.throttlingError, {
+				retryAfterSeconds: 3, // 3 second delay from error
+				driverVersion: pkgVersion,
+			}),
+		);
+
+		// Act
+		connectionManager.connect({ text: "test:retry timeout" }, "write");
+
+		// Advance time to trigger multiple retries and eventually hit the timeout
+		await clock.tickAsync(6000); // This should trigger the timeout logic
+
+		// Verify the telemetry event was logged
+		mockLogger.assertMatchAny([
+			{
+				eventName: "RetryDelayExceedsConnectionTimeout",
+				attempts: 2,
+				calculatedDelayMs: 6000, // The calculated delay that exceeded timeout
+				remainingTimeMs: 2000, // Should be positive but less than calculatedDelayMs
+				timeoutMs,
+			},
+		]);
+
+		// Verify that the container was closed due to the timeout
+		assert(closed, "Connection manager should close when retry delay exceeds timeout");
+
+		stubbedConnectToDeltaStream.restore();
+	});
+
+	it("Connection retry timeout - continues retrying when delay is within timeout", async () => {
+		// Arrange
+		const timeoutMs = 20000; // 20 second timeout (longer than test duration)
+		const connectionManager = new ConnectionManager(
+			() => mockDocumentService,
+			() => false,
+			client as IClient,
+			true /* reconnectAllowed */,
+			mockLogger.toTelemetryLogger(),
+			props,
+			timeoutMs, // Set the retry connection timeout
+		);
+
+		// Mock connectToDeltaStream to throw retryable errors with short delays
+		const stubbedConnectToDeltaStream = stub(mockDocumentService, "connectToDeltaStream");
+		stubbedConnectToDeltaStream.throws(
+			new RetryableError("Temporary error", DriverErrorTypes.genericError, {
+				retryAfterSeconds: 0.1, // Very small delay to keep within timeout
+				driverVersion: pkgVersion,
+			}),
+		);
+
+		// Act
+		connectionManager.connect({ text: "test:retry within timeout" }, "write");
+
+		// Advance time for a few retries but not long enough to exceed timeout
+		await clock.tickAsync(2000); // 2 seconds - should allow for multiple retries
+
+		// Assert - the connection should still be retrying and not have closed
+		assert(!closed, "Connection manager should not close when retries are within timeout");
+		assert(
+			stubbedConnectToDeltaStream.callCount > 1,
+			"Should have made multiple connection attempts",
+		);
+
+		// Verify no timeout telemetry was logged
+		assert(
+			!mockLogger.matchEvents([{ eventName: "RetryDelayExceedsConnectionTimeout" }]),
+			"Should not log timeout event when retries are within limit",
+		);
+
+		stubbedConnectToDeltaStream.restore();
+	});
+
+	it("Connection retry timeout - no timeout when retryConnectionTimeoutMs is undefined", async () => {
+		// Arrange - Create connection manager without timeout
+		const connectionManager = new ConnectionManager(
+			() => mockDocumentService,
+			() => false,
+			client as IClient,
+			true /* reconnectAllowed */,
+			mockLogger.toTelemetryLogger(),
+			props,
+			undefined, // No retry timeout
+		);
+
+		// Mock connectToDeltaStream to throw retryable errors
+		const stubbedConnectToDeltaStream = stub(mockDocumentService, "connectToDeltaStream");
+		stubbedConnectToDeltaStream.throws(
+			new RetryableError("Delay error", DriverErrorTypes.genericError, {
+				retryAfterSeconds: 1, // 1 second delay
+				driverVersion: pkgVersion,
+			}),
+		);
+
+		// Act
+		connectionManager.connect({ text: "test:no timeout limit" }, "write");
+
+		// Advance time for several retries
+		await clock.tickAsync(10000); // 10 seconds - would exceed most reasonable timeouts
+
+		// Assert - the connection should still be retrying and not have closed
+		assert(!closed, "Connection manager should not close when no timeout is set");
+		assert(
+			stubbedConnectToDeltaStream.callCount > 1,
+			"Should have made multiple connection attempts",
+		);
+
+		// Verify no timeout telemetry was logged
+		assert(
+			!mockLogger.matchEvents([{ eventName: "RetryDelayExceedsConnectionTimeout" }]),
+			"Should not log timeout event when no timeout is configured",
+		);
+
+		stubbedConnectToDeltaStream.restore();
+	});
 });