Merge pull request #1098 from matrix-org/tilosp-fix-944-2

Make sure a killed BridgedClient is dead, even if connect was never called (continuation)

changelog.d/1098.bugfix

@@ -0,0 +1 @@
+Make sure a killed BridgedClient is dead, even if connect was never called

spec/util/irc-client-mock.js

@@ -124,6 +124,10 @@ function Client(addr, nick, opts) {
         return channel.toLowerCase();
     }
 
+    this.getSplitMessages = function(target, text) {
+        return [];
+    }
+
     this.modeForPrefix = {
         "@" : "o",
         "+" : "v",

src/DebugApi.ts

@@ -24,7 +24,7 @@ import { inspect } from "util";
 import { DataStore } from "./datastore/DataStore";
 import { ClientPool } from "./irc/ClientPool";
 import { getLogger } from "./logging";
-import { BridgedClient } from "./irc/BridgedClient";
+import { BridgedClient, BridgedClientStatus } from "./irc/BridgedClient";
 import { IrcBridge } from "./bridge/IrcBridge";
 import { ProvisionRequest } from "./provisioning/ProvisionRequest";
 import { getBridgeVersion } from "./util/PackageInfo";
@@ -251,8 +251,7 @@ export class DebugApi {
                 "User " + user + " does not have a client on " + server.domain + "\n"
             );
         }
-        const connection = client.unsafeClient && client.unsafeClient.conn;
-        if (!client.unsafeClient || !connection) {
+        if (client.status !== BridgedClientStatus.CONNECTED) {
             return Bluebird.resolve(
                 "There is no underlying client instance.\n"
             );
@@ -261,25 +260,23 @@ export class DebugApi {
         // store all received response strings
         const buffer: string[] = [];
         // "raw" can take many forms
-        const listener = (msg: object) => {
+        const listener = (msg: unknown) => {
             buffer.push(JSON.stringify(msg));
         }
 
-        client.unsafeClient.on("raw", listener);
+        client.addClientListener("raw", listener);
         // turn rn to n so if there are any new lines they are all n.
         body = body.replace("\r\n", "\n");
         body.split("\n").forEach((c: string) => {
             // IRC protocol require rn
-            connection.write(c + "\r\n");
+            client.writeToConnection(c + "\r\n");
             buffer.push(c);
         });
 
         // wait 3s to pool responses
         return Bluebird.delay(3000).then(function() {
             // unhook listener to avoid leaking
-            if (client.unsafeClient) {
-                client.unsafeClient.removeListener("raw", listener);
-            }
+            client.removeClientListener("raw", listener);
             return buffer.join("\n") + "\n";
         });
     }

src/bridge/AdminRoomHandler.ts

@@ -287,11 +287,7 @@ export class AdminRoomHandler {
                 server, sender
             );
 
-            if (!bridgedClient.unsafeClient) {
-                throw new Error('Possibly disconnected');
-            }
-
-            bridgedClient.unsafeClient.send(...sendArgs);
+            bridgedClient.sendCommands(...sendArgs);
         }
         catch (err) {
             const notice = new MatrixAction("notice", `${err}\n` );

src/bridge/IrcBridge.ts

@@ -6,7 +6,7 @@ import { MatrixHandler } from "./MatrixHandler";
 import { MemberListSyncer } from "./MemberListSyncer";
 import { IrcServer } from "../irc/IrcServer";
 import { ClientPool } from "../irc/ClientPool";
-import { BridgedClient } from "../irc/BridgedClient";
+import { BridgedClient, BridgedClientStatus } from "../irc/BridgedClient";
 import { IrcUser } from "../models/IrcUser";
 import { IrcRoom } from "../models/IrcRoom";
 import { BridgeRequest, BridgeRequestErr } from "../models/BridgeRequest";
@@ -984,7 +984,7 @@ export class IrcBridge {
     public sendIrcAction(ircRoom: IrcRoom, bridgedClient: BridgedClient, action: IrcAction) {
         log.info(
             "Sending IRC message in %s as %s (connected=%s)",
-            ircRoom.channel, bridgedClient.nick, Boolean(bridgedClient.unsafeClient)
+            ircRoom.channel, bridgedClient.nick, Boolean(bridgedClient.status === BridgedClientStatus.CONNECTED)
         );
         return bridgedClient.sendAction(ircRoom, action);
     }

src/bridge/MatrixHandler.ts

@@ -905,14 +905,11 @@ export class MatrixHandler {
             this.eventCache.delete(delKey);
         }
 
-        // Check for the existance of the getSplitMessages method.
-        if (!(ircClient.unsafeClient && ircClient.unsafeClient.getSplitMessages)) {
-            await this.ircBridge.sendIrcAction(ircRoom, ircClient, ircAction);
-            return;
-        }
+        // The client might still be connected, for abundance of safety let's wait.
+        await ircClient.waitForConnected();
 
         // Generate an array of individual messages that would be sent
-        const potentialMessages = ircClient.unsafeClient.getSplitMessages(ircRoom.channel, text);
+        const potentialMessages = ircClient.getSplitMessages(ircRoom.channel, text);
         const lineLimit = ircRoom.server.getLineLimit();
 
         if (potentialMessages.length <= lineLimit) {

src/bridge/PublicitySyncer.ts

@@ -46,19 +46,15 @@ export class PublicitySyncer {
     };
     constructor (private ircBridge: IrcBridge) { }
 
-    public initModeForChannel(server: IrcServer, chan: string) {
-        return this.ircBridge.getBotClient(server).then(
-            (client) => {
-                if (!client.unsafeClient) {
-                    throw Error("Can't request modes, bot client not connected")
-                }
-                log.info(`Bot requesting mode for ${chan} on ${server.domain}`);
-                client.unsafeClient.mode(chan);
-            },
-            (err) => {
-                log.error(`Could not request mode of ${chan} (${err.message})`);
-            }
-        );
+    public async initModeForChannel(server: IrcServer, chan: string) {
+        try {
+            const botClient = await this.ircBridge.getBotClient(server);
+            log.info(`Bot requesting mode for ${chan} on ${server.domain}`);
+            await botClient.mode(chan);
+        }
+        catch (err) {
+            log.error(`Could not request mode of ${chan} (${err.message})`);
+        }
     }
 
     public async initModes (server: IrcServer) {

src/bridge/RoomAccessSyncer.ts

@@ -3,6 +3,7 @@ import { IrcBridge } from "./IrcBridge";
 import { BridgeRequest } from "../models/BridgeRequest";
 import { IrcServer } from "../irc/IrcServer";
 import { MatrixRoom } from "matrix-appservice-bridge";
+import { BridgedClientStatus } from "../irc/BridgedClient";
 const log = getLogger("RoomAccessSyncer");
 
 const MODES_TO_WATCH = [
@@ -114,21 +115,20 @@ export class RoomAccessSyncer {
         let userId = null;
         if (nick !== null && bridgedClient) {
             userId = bridgedClient.userId;
-            if (!bridgedClient.unsafeClient) {
+            if (bridgedClient.status !== BridgedClientStatus.CONNECTED) {
                 req.log.info(`Bridged client for ${nick} has no IRC client.`);
                 return;
             }
-            const client = bridgedClient.unsafeClient;
-            const chanData = client.chanData(channel);
+            const chanData = bridgedClient.chanData(channel);
             if (!(chanData && chanData.users)) {
                 req.log.error(`No channel data for ${channel}`);
                 return;
             }
             const userPrefixes = chanData.users[nick] as string;
             userPrefixes.split('').forEach(
                 prefix => {
-                    const m = client.modeForPrefix[prefix];
-                    if (modeToPower[m] !== undefined) {
+                    const m = bridgedClient.modeForPrefix(prefix);
+                    if (m && modeToPower[m] !== undefined) {
                         userPowers.push(modeToPower[m]);
                     }
                 }