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

Reverts #24948

Author: jason-ha

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

@@ -186,12 +186,6 @@ 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 = {};
@@ -350,7 +344,6 @@ 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;
@@ -529,12 +522,6 @@ 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();
@@ -655,31 +642,6 @@ 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,16 +248,6 @@ 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;
 }
 
 /**
@@ -627,7 +617,6 @@ 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;
@@ -810,7 +799,6 @@ export class Container
 			scope,
 			subLogger,
 			protocolHandlerBuilder,
-			retryConnectionTimeoutMs,
 		} = createProps;
 
 		// Validate that the Driver is compatible with this Loader.
@@ -826,7 +814,6 @@ export class Container
 		const pendingLocalState = loadProps?.pendingLocalState;
 
 		this._canReconnect = canReconnect ?? true;
-		this._retryConnectionTimeoutMs = retryConnectionTimeoutMs;
 		this.clientDetailsOverride = clientDetailsOverride;
 		this.urlResolver = urlResolver;
 		this.serviceFactory = documentServiceFactory;
@@ -2067,7 +2054,6 @@ export class Container
 					this._canReconnect,
 					createChildLogger({ logger: this.subLogger, namespace: "ConnectionManager" }),
 					props,
-					this._retryConnectionTimeoutMs,
 				),
 		);
 

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

@@ -386,136 +386,4 @@ 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();
-	});
 });