diff --git a/packages/loader/container-definitions/src/loader.ts b/packages/loader/container-definitions/src/loader.ts index 8b15b89ba6b4..03280613b0db 100644 --- a/packages/loader/container-definitions/src/loader.ts +++ b/packages/loader/container-definitions/src/loader.ts @@ -261,6 +261,11 @@ export type ILoaderOptions = { * Set min op frequency with which noops would be sent in case of active connection which is not sending any op. */ noopCountFrequency?: number; + + /** + * Max time(in ms) container will wait for a leave message of a disconnected client. + */ + maxClientLeaveWaitTime?: number, }; /** diff --git a/packages/loader/container-loader/src/connectionStateHandler.ts b/packages/loader/container-loader/src/connectionStateHandler.ts index 1e28d49c4f2e..2e725e34facb 100644 --- a/packages/loader/container-loader/src/connectionStateHandler.ts +++ b/packages/loader/container-loader/src/connectionStateHandler.ts @@ -3,12 +3,12 @@ * Licensed under the MIT License. */ -import { EventEmitter } from "events"; import { IEvent, ITelemetryLogger } from "@fluidframework/common-definitions"; import { IConnectionDetails } from "@fluidframework/container-definitions"; -import { ProtocolOpHandler, Quorum } from "@fluidframework/protocol-base"; +import { ProtocolOpHandler } from "@fluidframework/protocol-base"; import { ConnectionMode, ISequencedClient } from "@fluidframework/protocol-definitions"; -import { EventEmitterWithErrorHandling } from "@fluidframework/telemetry-utils"; +import { EventEmitterWithErrorHandling, PerformanceEvent } from "@fluidframework/telemetry-utils"; +import { assert, Timer } from "@fluidframework/common-utils"; import { connectEventName, ConnectionState } from "./container"; export interface IConnectionStateHandler { @@ -17,6 +17,8 @@ export interface IConnectionStateHandler { (value: ConnectionState, oldState: ConnectionState, reason?: string | undefined) => void, propagateConnectionState: () => void, isContainerLoaded: () => boolean, + shouldClientJoinWrite: () => boolean, + maxClientLeaveWaitTime: number | undefined, } export interface ILocalSequencedClient extends ISequencedClient { @@ -37,6 +39,13 @@ export class ConnectionStateHandler extends EventEmitterWithErrorHandling { + this.leaveReceivedResult = false; + this.applyForConnectedState("timeout"); + }, + ); } - public receivedAddMemberEvent(clientId: string, quorum: Quorum) { + // This is true when this client submitted any ops. + public clientSentOps(connectionMode: ConnectionMode) { + assert(this._connectionState === ConnectionState.Connected, "Ops could only be sent when connected"); + this._clientSentOps = true; + this.clientConnectionMode = connectionMode; + } + + public receivedAddMemberEvent(clientId: string) { // This is the only one that requires the pending client ID if (clientId === this.pendingClientId) { + // Start the event in case we are waiting for leave or timeout. + if (this.prevClientLeftTimer.hasTimer) { + this.waitEvent = PerformanceEvent.start(this.logger, { + eventName: "WaitBeforeClientLeave", + waitOnClientId: this._clientId, + hadOutstandingOps: this.handler.shouldClientJoinWrite(), + }); + } + this.applyForConnectedState("addMemberEvent"); + } + } + + private applyForConnectedState(source: "removeMemberEvent" | "addMemberEvent" | "timeout") { + const protocolHandler = this.handler.protocolHandler(); + // Move to connected state only if we are in Connecting state, we have seen our join op + // and there is no timer running which means we are not waiting for previous client to leave + // or timeout has occured while doing so. + if (this.pendingClientId !== this.clientId + && this.pendingClientId !== undefined + && protocolHandler !== undefined && protocolHandler.quorum.getMember(this.pendingClientId) !== undefined + && !this.prevClientLeftTimer.hasTimer + ) { + this.waitEvent?.end({ leaveReceived: this.leaveReceivedResult, source }); this.setConnectionState(ConnectionState.Connected); + } else { + // Adding this event temporarily so that we can get help debugging if something goes wrong. + this.logger.sendTelemetryEvent({ + eventName: "connectedStateRejected", + source, + pendingClientId: this.pendingClientId, + clientId: this.clientId, + hasTimer: this.prevClientLeftTimer.hasTimer, + inQuorum: protocolHandler !== undefined && this.pendingClientId !== undefined + && protocolHandler.quorum.getMember(this.pendingClientId) !== undefined, + }); + } + } + + public receivedRemoveMemberEvent(clientId: string) { + // If the client which has left was us, then finish the timer. + if (this.clientId === clientId) { + this.prevClientLeftTimer.clear(); + this.leaveReceivedResult = true; + this.applyForConnectedState("removeMemberEvent"); } } @@ -73,7 +140,6 @@ export class ConnectionStateHandler extends EventEmitterWithErrorHandling i public get options(): ILoaderOptions { return this.loader.services.options; } private get scope() { return this.loader.services.scope;} private get codeLoader() { return this.loader.services.codeLoader;} + constructor( private readonly loader: Loader, config: IContainerConfig, @@ -599,6 +600,8 @@ export class Container extends EventEmitterWithErrorHandling i this.logConnectionStateChangeTelemetry(value, oldState, reason), propagateConnectionState: () => this.propagateConnectionState(), isContainerLoaded: () => this.loaded, + shouldClientJoinWrite: () => this._deltaManager.shouldJoinWrite(), + maxClientLeaveWaitTime: this.loader.services.options.maxClientLeaveWaitTime, }, this.logger, ); @@ -1396,10 +1399,14 @@ export class Container extends EventEmitterWithErrorHandling i // Track membership changes and update connection state accordingly protocol.quorum.on("addMember", (clientId, details) => { - this.connectionStateHandler.receivedAddMemberEvent(clientId, this.protocolHandler.quorum); + this.connectionStateHandler.receivedAddMemberEvent(clientId); + }); + + protocol.quorum.on("removeMember", (clientId) => { + this.connectionStateHandler.receivedRemoveMemberEvent(clientId); }); - protocol.quorum.on("addProposal",(proposal: IPendingProposal) => { + protocol.quorum.on("addProposal", (proposal: IPendingProposal) => { if (proposal.key === "code" || proposal.key === "code2") { this.emit("codeDetailsProposed", proposal.value, proposal); } @@ -1515,7 +1522,6 @@ export class Container extends EventEmitterWithErrorHandling i deltaManager.on(connectEventName, (details: IConnectionDetails, opsBehind?: number) => { this.connectionStateHandler.receivedConnectEvent( - this, this._deltaManager.connectionMode, details, opsBehind, @@ -1529,6 +1535,10 @@ export class Container extends EventEmitterWithErrorHandling i } }); + deltaManager.once("submitOp", (message: IDocumentMessage) => { + this.connectionStateHandler.clientSentOps(this._deltaManager.connectionMode); + }); + deltaManager.on("disconnect", (reason: string) => { this.manualReconnectInProgress = false; this.connectionStateHandler.receivedDisconnectEvent(reason); diff --git a/packages/loader/container-loader/src/deltaManager.ts b/packages/loader/container-loader/src/deltaManager.ts index 20dc100fdec1..8203323aaad9 100644 --- a/packages/loader/container-loader/src/deltaManager.ts +++ b/packages/loader/container-loader/src/deltaManager.ts @@ -349,6 +349,10 @@ export class DeltaManager return this._reconnectMode; } + public shouldJoinWrite(): boolean { + return this.clientSequenceNumber !== this.clientSequenceNumberObserved; + } + public async connectToStorage(): Promise { if (this.storageService !== undefined) { return this.storageService; @@ -580,7 +584,7 @@ export class DeltaManager // firing of "connected" event from Container and switch of current clientId (as tracked // by all DDSes). This will make it impossible to figure out if ops actually made it through, // so DDSes will immediately resubmit all pending ops, and some of them will be duplicates, corrupting document - if (this.clientSequenceNumberObserved !== this.clientSequenceNumber) { + if (this.shouldJoinWrite()) { requestedMode = "write"; } diff --git a/packages/loader/container-loader/src/test/connectionStateHandler.spec.ts b/packages/loader/container-loader/src/test/connectionStateHandler.spec.ts new file mode 100644 index 000000000000..df80e43e367a --- /dev/null +++ b/packages/loader/container-loader/src/test/connectionStateHandler.spec.ts @@ -0,0 +1,444 @@ +/*! + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +import { strict as assert } from "assert"; +import { TelemetryNullLogger } from "@fluidframework/common-utils"; +import { ProtocolOpHandler } from "@fluidframework/protocol-base"; +import { IClient, IClientConfiguration, ITokenClaims } from "@fluidframework/protocol-definitions"; +import { IConnectionDetails } from "@fluidframework/container-definitions"; +import { SinonFakeTimers, useFakeTimers } from "sinon"; +import { ConnectionState } from "../container"; +import { ConnectionStateHandler } from "../connectionStateHandler"; + +describe("ConnectionStateHandler Tests", () => { + let clock: SinonFakeTimers; + let connectionStateHandler: ConnectionStateHandler; + let protocolHandler: ProtocolOpHandler; + let shouldClientJoinWrite: boolean; + let connectionDetails: IConnectionDetails; + let client: IClient; + const expectedTimeout = 90000; + const pendingClientId = "pendingClientId"; + + // Stash the real setTimeout because sinon fake timers will hijack it. + const realSetTimeout = setTimeout; + + // function to yield control in the Javascript event loop. + async function yieldEventLoop(): Promise { + await new Promise((resolve) => { + realSetTimeout(resolve, 0); + }); + } + + async function tickClock(tickValue: number) { + clock.tick(tickValue); + + // Yield the event loop because the outbound op will be processed asynchronously. + await yieldEventLoop(); + } + + before(() => { + clock = useFakeTimers(); + }); + + beforeEach(() => { + protocolHandler = new ProtocolOpHandler(0, 0, 1, [], [], [], (key, value) => 0, (seqNum) => undefined); + connectionDetails = { + clientId: pendingClientId, + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + claims: {} as ITokenClaims, + existing: true, + mode: "read", + version: "0.1", + initialClients: [], + maxMessageSize: 1000, + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + serviceConfiguration: {} as IClientConfiguration, + checkpointSequenceNumber: undefined, + }; + client = { + mode: "read", + user: { + id: "userId", + }, + permission: [], + details: { + capabilities: { interactive: true }, + }, + scopes: [], + }; + connectionStateHandler = new ConnectionStateHandler( + { + logConnectionStateChangeTelemetry: () => undefined, + propagateConnectionState:() => undefined, + isContainerLoaded: () => true, + maxClientLeaveWaitTime: expectedTimeout, + protocolHandler: () => protocolHandler, + shouldClientJoinWrite: () => shouldClientJoinWrite, + }, + new TelemetryNullLogger(), + ); + }); + + it("Should move to connected state on normal flow for read client", async () => { + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Disconnected, + "Client should be in disconnected state"); + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Read Client should be in connected state"); + }); + + it("Should move to connected state on normal flow for write client", async () => { + client.mode = "write"; + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Disconnected, + "Client should be in disconnected state"); + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Client should be in connecting state"); + protocolHandler.quorum.addMember("anotherClientId", { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent("anotherClientId"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Some other client joined."); + protocolHandler.quorum.addMember(pendingClientId, { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent(pendingClientId); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client should be in connected state"); + }); + + it("Should wait for previous client to leave before moving to conencted state", async () => { + client.mode = "write"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + protocolHandler.quorum.addMember(pendingClientId, { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent(pendingClientId); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client should be in connected state"); + + // Mock as though the client sent some ops. + shouldClientJoinWrite = true; + client.mode = "write"; + connectionStateHandler.clientSentOps(client.mode); + // Disconnect the client + connectionStateHandler.receivedDisconnectEvent("Test"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Disconnected, + "Client should be in disconnected state"); + + // Make new client join so that it waits for previous client to leave + connectionDetails.clientId = "pendingClientId2"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + protocolHandler.quorum.addMember("pendingClientId2", { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent(connectionDetails.clientId); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Client 2 should be in connecting state as we are waiting for leave"); + + // Send leave + connectionStateHandler.receivedRemoveMemberEvent(pendingClientId); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client 2 should be in connected state"); + }); + + it("Should wait for timeout before moving to conencted state if no leave received", async () => { + client.mode = "write"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + protocolHandler.quorum.addMember(pendingClientId, { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent(pendingClientId); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client should be in connected state"); + + // Mock as though the client sent some ops. + shouldClientJoinWrite = true; + client.mode = "write"; + connectionStateHandler.clientSentOps(client.mode); + // Disconnect the client + connectionStateHandler.receivedDisconnectEvent("Test"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Disconnected, + "Client should be in disconnected state"); + + // Make new client join so that it waits for previous client to leave + connectionDetails.clientId = "pendingClientId2"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + protocolHandler.quorum.addMember("pendingClientId2", { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent(connectionDetails.clientId); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Client 2 should be in connecting state as we are waiting for timeout"); + + // Check state before timeout + await tickClock(expectedTimeout - 1); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Client 2 should still be in connecting state as we are waiting for timeout"); + + await tickClock(1); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client 2 should now be in connected state"); + }); + + it("Should wait for client 1 to leave before moving to conencted state(Client 3) when client 2 " + + "got disconnected from connecting state", async () => + { + client.mode = "write"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + protocolHandler.quorum.addMember(pendingClientId, { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent(pendingClientId); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client 1 should be in connected state"); + + // Mock as though the client sent some ops. + shouldClientJoinWrite = true; + client.mode = "write"; + connectionStateHandler.clientSentOps(client.mode); + // Disconnect the client + connectionStateHandler.receivedDisconnectEvent("Test"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Disconnected, + "Client 1 should be in disconnected state"); + + // Make new client join but disconnect it from connecting state + connectionDetails.clientId = "pendingClientId2"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Client 2 should be in connecting state"); + connectionStateHandler.receivedDisconnectEvent("Test"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Disconnected, + "Client 2 should be in disconnected state"); + + // Make new client 3 join so that it waits for client 1 to leave + connectionDetails.clientId = "pendingClientId3"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + protocolHandler.quorum.addMember("pendingClientId3", { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent(connectionDetails.clientId); + + // Send leave for client 2 and check that client 3 should not move to conencted state as we were waiting + // on client 1 leave + connectionStateHandler.receivedRemoveMemberEvent("pendingClientId2"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Client 3 should still be in connecting state"); + + // Client 1 leaves. + connectionStateHandler.receivedRemoveMemberEvent(pendingClientId); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client 3 should move to connected state"); + // Timeout should not raise any error as timer should be cleared + await tickClock(expectedTimeout); + }); + + it("Should wait for client 1 timeout before moving to conencted state(Client 3) when client 2 " + + "got disconnected from connecting state", async () => + { + client.mode = "write"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + protocolHandler.quorum.addMember(pendingClientId, { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent(pendingClientId); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client 1 should be in connected state"); + + // Mock as though the client sent some ops. + shouldClientJoinWrite = true; + client.mode = "write"; + connectionStateHandler.clientSentOps(client.mode); + // Disconnect the client + connectionStateHandler.receivedDisconnectEvent("Test"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Disconnected, + "Client 1 should be in disconnected state"); + + // Make new client join but disconnect it from connecting state + connectionDetails.clientId = "pendingClientId2"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Client 2 should be in connecting state"); + connectionStateHandler.receivedDisconnectEvent("Test"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Disconnected, + "Client 2 should be in disconnected state"); + + // Make new client 3 join so that it waits for client 1 to leave + connectionDetails.clientId = "pendingClientId3"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + protocolHandler.quorum.addMember("pendingClientId3", { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent(connectionDetails.clientId); + + // Send leave for client 2 and check that client 3 should not move to conencted state as we were waiting + // on client 1 leave. + connectionStateHandler.receivedRemoveMemberEvent("pendingClientId2"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Client 3 should still be in connecting state"); + + // Pass some time. + await tickClock(expectedTimeout - 1); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Client 3 should still be in connecting state as timeout has not occured"); + + await tickClock(1); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client 3 should move to connected state"); + // Sending client 1 leave now should not cause any error + connectionStateHandler.receivedRemoveMemberEvent(pendingClientId); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client 3 should move to connected state"); + }); + + it("Should wait for client 1 to leave before moving to conencted state(Client 3) when client 2 " + + "got disconnected from connected state", async () => + { + client.mode = "write"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + protocolHandler.quorum.addMember(pendingClientId, { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent(pendingClientId); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client 1 should be in connected state"); + + // Mock as though the client sent some ops. + shouldClientJoinWrite = true; + client.mode = "write"; + connectionStateHandler.clientSentOps(client.mode); + // Disconnect the client + connectionStateHandler.receivedDisconnectEvent("Test"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Disconnected, + "Client 1 should be in disconnected state"); + + // Make new client join but disconnect it from connected state + connectionDetails.clientId = "pendingClientId2"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + protocolHandler.quorum.addMember("pendingClientId2", { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent("pendingClientId2"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Client 2 should still be in connecting state"); + connectionStateHandler.receivedDisconnectEvent("Test"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Disconnected, + "Client 2 should be in disconnected state"); + + // Make new client 3 join so that it waits for client 1 to leave + connectionDetails.clientId = "pendingClientId3"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + protocolHandler.quorum.addMember("pendingClientId3", { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent(connectionDetails.clientId); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Client 3 should still be in connecting state"); + + // Send leave for client 2 and check that client 3 should not move to conencted state as we were waiting + // on client 1 leave + connectionStateHandler.receivedRemoveMemberEvent("pendingClientId2"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Client 3 should still be in connecting state"); + + // Client 1 leaves. + connectionStateHandler.receivedRemoveMemberEvent(pendingClientId); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client 3 should move to connected state"); + // Timeout should not raise any error as timer should be cleared + await tickClock(expectedTimeout); + }); + + it("Should wait for client 1 timeout before moving to conencted state(Client 3) when client 2 " + + "got disconnected from connected state", async () => + { + client.mode = "write"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + protocolHandler.quorum.addMember(pendingClientId, { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent(pendingClientId); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client 1 should be in connected state"); + + // Mock as though the client sent some ops. + shouldClientJoinWrite = true; + client.mode = "write"; + connectionStateHandler.clientSentOps(client.mode); + // Disconnect the client + connectionStateHandler.receivedDisconnectEvent("Test"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Disconnected, + "Client 1 should be in disconnected state"); + + // Make new client join but disconnect it from connecting state + connectionDetails.clientId = "pendingClientId2"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + protocolHandler.quorum.addMember("pendingClientId2", { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent("pendingClientId2"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Client 2 should still be in connecting state"); + connectionStateHandler.receivedDisconnectEvent("Test"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Disconnected, + "Client 2 should be in disconnected state"); + + // Make new client 3 join so that it waits for client 1 to leave + connectionDetails.clientId = "pendingClientId3"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + protocolHandler.quorum.addMember("pendingClientId3", { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent(connectionDetails.clientId); + + // Send leave for client 2 and check that client 3 should not move to conencted state as we were waiting + // on client 1 leave. + connectionStateHandler.receivedRemoveMemberEvent("pendingClientId2"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Client 3 should still be in connecting state"); + + // Pass some time. + await tickClock(expectedTimeout - 1); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Client 3 should still be in connecting state as timeout has not occured"); + + await tickClock(1); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client 3 should move to connected state"); + + // Sending client 1 leave now should not cause any error + connectionStateHandler.receivedRemoveMemberEvent(pendingClientId); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client 3 should move to connected state"); + }); + + it("Client 3 should not wait for client 2(which got disconnected without sending any ops) to leave " + + "when client 2 already waited on client 1", async () => + { + client.mode = "write"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + protocolHandler.quorum.addMember(pendingClientId, { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent(pendingClientId); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client 1 should be in connected state"); + + // Mock as though the client sent some ops. + shouldClientJoinWrite = true; + client.mode = "write"; + connectionStateHandler.clientSentOps(client.mode); + // Disconnect the client + connectionStateHandler.receivedDisconnectEvent("Test"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Disconnected, + "Client 1 should be in disconnected state"); + + // Make new client join but disconnect it from connected state + connectionDetails.clientId = "pendingClientId2"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + protocolHandler.quorum.addMember("pendingClientId2", { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent("pendingClientId2"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Client 2 should still be in connecting state"); + // Client 1 leaves. + connectionStateHandler.receivedRemoveMemberEvent(pendingClientId); + + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client 2 should move to connected state"); + + // Client 2 leaves without sending any ops. So dirty state should be false + connectionStateHandler.receivedDisconnectEvent("Test"); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Disconnected, + "Client 2 should be in disconnected state"); + + // Make new client 3 join. Now it should not wait for previous client as client 2 already waited. + connectionDetails.clientId = "pendingClientId3"; + connectionStateHandler.receivedConnectEvent(client.mode, connectionDetails, 0); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connecting, + "Client 3 should still be in connecting state"); + protocolHandler.quorum.addMember("pendingClientId3", { client, sequenceNumber: 0 }); + connectionStateHandler.receivedAddMemberEvent(connectionDetails.clientId); + assert.strictEqual(connectionStateHandler.connectionState, ConnectionState.Connected, + "Client 3 should move to connected state"); + // Timeout should not raise any error as timer should be cleared + await tickClock(expectedTimeout); + }); + + afterEach(() => { + clock.reset(); + }); + + after(() => { + clock.restore(); + }); +});