Catch more gateway errors (#355)

* Catch places where gateway calls may fail.

* changelog

* Fix a crash caused by users attempting to leave a room.

* Fix test

* Use a named type.

changelog.d/355.bugfix

@@ -0,0 +1 @@
+Fix several cases where Bifrost may crash if a gateway room was inaccessible.

src/MatrixEventHandler.ts

@@ -626,7 +626,11 @@ Say \`help\` for more commands.
         const name: string = context.remote.get("room_name");
         if (isGateway) {
             const msg = MessageFormatter.matrixEventToBody(event as MatrixMessageEvent, this.config.bridge);
-            this.gatewayHandler.sendMatrixMessage(name, event.sender, msg, context);
+            try {
+                await this.gatewayHandler.sendMatrixMessage(name, event.sender, msg, context);
+            } catch (ex) {
+                log.warn(`Failed to handle message for gateway:`, ex);
+            }
             return;
         }
         try {
@@ -689,9 +693,13 @@ Say \`help\` for more commands.
         const name: string = context.remote.get("room_name");
         const roomProtocol: string = context.remote.get("protocol_id");
         if (isGateway) {
-            await this.gatewayHandler.sendMatrixMembership(
-                name, context, event,
-            );
+            try {
+                await this.gatewayHandler.sendMatrixMembership(
+                    name, context, event,
+                );
+            } catch (ex) {
+                log.warn(`Failed to handle membership for gateway:`, ex);
+            }
             return;
         }
 
@@ -728,9 +736,13 @@ Say \`help\` for more commands.
         const name = context.remote.get<string>("room_name");
         log.info(`Handling group state event for ${name}`);
         if (isGateway) {
-            await this.gatewayHandler.sendStateEvent(
-                name, event.sender, event, context,
-            );
+            try {
+                await this.gatewayHandler.sendStateEvent(
+                    name, event.sender, event, context,
+                );
+            } catch (ex) {
+                log.warn(`Failed to handle state event for gateway:`, ex);
+            }
             return;
         }
         // XXX: Support state changes for non-gateways

src/RoomSync.ts

@@ -96,7 +96,11 @@ export class RoomSync {
             const isGateway = room.remote.get<boolean>("gateway");
             if (isGateway) {
                 // The gateway handler syncs via roomState.
-                this.gateway.initialMembershipSync(room);
+                try {
+                    await this.gateway.initialMembershipSync(room);
+                } catch (ex) {
+                    log.warn(`Failed to sync gateway membership for room ${roomId}`);
+                }
                 return;
             }
             let members: {[userId: string]: {display_name?: string}};

src/bifrost/Events.ts

@@ -4,7 +4,6 @@
 
 import { IBifrostAccount } from "./Account";
 import { IBasicProtocolMessage } from "../MessageFormatter";
-import { IGatewayRoom } from "./Gateway";
 import { IPublicRoomsResponse } from "../MatrixTypes";
 
 export interface IChatJoinProperties {[key: string]: string; }

src/xmppjs/XJSAccount.ts

@@ -19,6 +19,8 @@ const LASTSTANZA_CHECK_MS = 2 * 60000;
 const LASTSTANZA_MAXDURATION = 10 * 60000;
 const log = new Logger("XmppJsAccount");
 
+type XmppChatJoinComponents = { handle: string }&({room: string, server: string}|{fullRoomName: string})
+
 export class XmppJsAccount implements IBifrostAccount {
 
     get name(): string {
@@ -54,10 +56,17 @@ export class XmppJsAccount implements IBifrostAccount {
                             this.lastStanzaTs.set(roomName, Date.now());
                             return;
                         }
+                        const handle = this.roomHandles.get(roomName);
+                        if (!handle) {
+                            // Shouldn't happen, but we must be careful.
+                            return;
+                        }
                         this.joinChat({
                             fullRoomName: roomName,
-                            handle: this.roomHandles.get(roomName)!,
-                        });
+                            handle,
+                        }).catch(ex => {
+                            log.warn(`Attempted to join ${roomName} due to a failed self ping, but failed`, ex);
+                        })
                     });
                 }
             });
@@ -186,18 +195,22 @@ export class XmppJsAccount implements IBifrostAccount {
     }
 
     public async joinChat(
-        components: IChatJoinProperties,
+        components: XmppChatJoinComponents,
         instance?: IBifrostInstance,
         timeout: number = 5000,
         setWaiting: boolean = true)
         : Promise<IConversationEvent|void> {
-        if (!components.fullRoomName && (!components.room || !components.server)) {
-            throw Error("Missing fullRoomName OR room|server");
-        }
+        let roomName: string;
         if (!components.handle) {
             throw Error("Missing handle");
         }
-        const roomName = components.fullRoomName || `${components.room}@${components.server}`;
+        if ('fullRoomName' in components) {
+            roomName = components.fullRoomName;
+        } else if ('room' in components && 'server' in components) {
+            roomName = `${components.room}@${components.server}`;
+        } else {
+            throw Error("Missing fullRoomName OR room|server");
+        }
         const to = `${roomName}/${components.handle}`;
         log.debug(`joinChat:`, this.remoteId, components);
         if (this.isInRoom(roomName)) {
@@ -288,6 +301,7 @@ export class XmppJsAccount implements IBifrostAccount {
             `${components.room}@${components.server}/${components.handle}`,
         ));
         this.roomHandles.delete(room);
+        this.lastStanzaTs.delete(room);
         Metrics.remoteCall("xmpp.presence.left");
     }
 

test/xmppjs/test_XJSAccount.ts

@@ -67,7 +67,8 @@ describe("XJSAccount", () => {
                 await acct.joinChat({
                     room: "den",
                     server: "remote.server",
-                }, instance as any, 50, true);
+                // Explicit any - we want to deliberately send wrong params
+                } as any, instance as any, 50, true);
             } catch (ex) {
                 expect(ex.message).to.equal("Missing handle");
                 return;