fix monolith not removing the correct balancer on disconnect (#1174)

* fix `onBalancerDisconnect` so it actually removes the correct balancer from the list

* add a unit test

Author: dyc3

server/balancer.ts

@@ -84,12 +84,12 @@ class BalancerManager {
 	private onBalancerDisconnect(conn: BalancerConnection, code: number, reason: string) {
 		log.debug(`Disconnected from balancer ${conn.id}: ${code} ${reason}`);
 		this.emit("disconnect", conn);
-		for (const conn of this.balancerConnections) {
-			if (conn.id === conn.id) {
-				this.balancerConnections.splice(this.balancerConnections.indexOf(conn), 1);
-				break;
-			}
+		const idx = this.balancerConnections.indexOf(conn);
+		if (idx === -1) {
+			log.error(`Balancer ${conn.id} was not found in balancerConnections`);
+			return;
 		}
+		this.balancerConnections.splice(idx, 1);
 	}
 
 	private onBalancerMessage(conn: BalancerConnection, message: MsgB2M) {

server/tests/unit/clientmanager.spec.ts

@@ -118,3 +118,26 @@ describe("ClientManager", () => {
 		expect(joins2).toHaveLength(0);
 	});
 });
+
+describe("BalancerManager", () => {
+	beforeEach(() => {
+		balancerManager.balancerConnections.splice(0, balancerManager.balancerConnections.length);
+	});
+
+	it("should remove the correct balancer from the list when it disconnects", () => {
+		const con1 = new BalancerConnectionMock();
+		const con2 = new BalancerConnectionMock();
+		const con3 = new BalancerConnectionMock();
+
+		balancerManager.addBalancerConnection(con1);
+		balancerManager.addBalancerConnection(con2);
+		balancerManager.addBalancerConnection(con3);
+
+		expect(balancerManager.balancerConnections).toHaveLength(3);
+
+		con2.emit("disconnect", 1000, "reason");
+
+		expect(balancerManager.balancerConnections).toHaveLength(2);
+		expect(balancerManager.balancerConnections).not.toContain(con2);
+	});
+});