Revert "Fix race condition while establishing connection due to multiplexing in ODSP Driver" (microsoft#23078)

Reverts microsoft#23060

We're seeing a huge increase in errors in our odsp tests against both
prod and dogfood starting with the first run after this PR was merged,
so reverting as the probable culprit until we can investigate further.

Author: alexvy86

packages/drivers/odsp-driver/src/odspDocumentDeltaConnection.ts

@@ -634,33 +634,15 @@ export class OdspDocumentDeltaConnection extends DocumentDeltaConnection {
 			}
 		});
 
-		const p = new Deferred<void>();
-		// Due to socket reuse(multiplexing), we can get "disconnect" event from other clients in the socket reference.
-		// So, a race condition could happen, where this client is establishing connection and listening for "connect_document_success"
-		// on the socket among other events, but we get "disconnect" event on the socket reference from other clients, in which case,
-		// we dispose connection object and stop listening to further events on the socket. Due to this we get stuck as the connection
-		// is not yet established and so we don't return any connection object to the client(connection manager). So, we remain stuck.
-		// In order to handle this, listen for the "disconnect" event and reject the promise with the error so that the caller can
-		// know and handle the error.
-		this.on("disconnect", (reason: IAnyDriverError) => {
-			if (!p.isCompleted) {
-				p.reject(reason);
-			}
-		});
-
-		super
-			.initialize(connectMessage, timeout)
-			.then(() => p.resolve())
-			.catch((error) => p.reject(error));
-
-		await p.promise.finally(() => {
+		await super.initialize(connectMessage, timeout).finally(() => {
 			this.logger.sendTelemetryEvent({
 				eventName: "ConnectionAttemptInfo",
 				...this.getConnectionDetailsProps(),
 			});
 		});
 	}
 
+	// eslint-disable-next-line @typescript-eslint/no-explicit-any
 	protected addTrackedListener(event: string, listener: (...args: any[]) => void): void {
 		// override some event listeners in order to support multiple documents/clients over the same websocket
 		switch (event) {

packages/drivers/odsp-driver/src/test/socketTests/socketMock.ts

@@ -42,9 +42,6 @@ export interface IMockSocketConnectResponse {
 		  }
 		| {
 				eventToEmit: "connect_timeout";
-		  }
-		| {
-				eventToEmit: undefined;
 		  };
 }
 
@@ -55,7 +52,7 @@ export interface IMockSocketConnectResponse {
 export class ClientSocketMock extends TypedEventEmitter<SocketMockEvents> {
 	public disconnected = false;
 	constructor(
-		private mockSocketConnectResponse: IMockSocketConnectResponse = {
+		private readonly mockSocketConnectResponse: IMockSocketConnectResponse = {
 			connect_document: { eventToEmit: "connect_document_success" },
 		},
 	) {
@@ -107,16 +104,6 @@ export class ClientSocketMock extends TypedEventEmitter<SocketMockEvents> {
 		this.emit("server_disconnect", socketError, clientId);
 	}
 
-	/**
-	 * Use this to set connect response when the socket is reused.
-	 * @param connectResponse - response to be send on connect event.
-	 */
-	public setMockSocketConnectResponseForReuse(
-		connectResponse: IMockSocketConnectResponse,
-	): void {
-		this.mockSocketConnectResponse = connectResponse;
-	}
-
 	public emit(eventName: string, ...args: unknown[]): boolean {
 		switch (eventName) {
 			case "connect_document": {

packages/drivers/odsp-driver/src/test/socketTests/socketTests.spec.ts

@@ -490,61 +490,4 @@ describe("OdspDocumentDeltaConnection tests", () => {
 			"1 get_ops_response listener should exiist",
 		);
 	});
-
-	it("Multiple connection objects should handle server_disconnect event when the second client is still waiting for connection to complete", async () => {
-		socket = new ClientSocketMock();
-		const connection1 = await mockSocket(socket as unknown as Socket, async () =>
-			OdspDocumentDeltaConnection.create(
-				tenantId,
-				documentId,
-				token,
-				client,
-				webSocketUrl,
-				logger,
-				60000,
-				epochTracker,
-				socketReferenceKeyPrefix,
-			),
-		);
-
-		socket.setMockSocketConnectResponseForReuse({
-			connect_document: { eventToEmit: undefined },
-		});
-		let connection2Fails = false;
-		let errorReceived: IAnyDriverError | undefined;
-		const connection2 = mockSocket(socket as unknown as Socket, async () =>
-			OdspDocumentDeltaConnection.create(
-				tenantId,
-				documentId,
-				token,
-				client,
-				webSocketUrl,
-				logger,
-				60000,
-				epochTracker,
-				socketReferenceKeyPrefix,
-			),
-		).catch((error: IAnyDriverError) => {
-			connection2Fails = true;
-			errorReceived = error;
-		});
-		let disconnectedEvent1 = false;
-
-		const errorToThrow = { message: "OdspSocketError", code: 400 };
-		connection1.on("disconnect", (reason: IAnyDriverError) => {
-			disconnectedEvent1 = true;
-		});
-
-		socket.sendServerDisconnectEvent(errorToThrow);
-
-		assert(disconnectedEvent1, "disconnect event should happen on first object");
-
-		assert(socket !== undefined && !socket.connected, "socket should be disconnected");
-		checkListenerCount(socket);
-		await connection2;
-		assert(connection2Fails, "connection2 should fail");
-		assert(errorReceived?.message.includes("server_disconnect"), "message should be correct");
-		// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any
-		assert((errorReceived as any).statusCode === 400, "status code should be correct");
-	});
 });