Merge pull request #264 from matrix-org/tadzik/gh-236/reinforce-xmpp-…

…connection-handling Make XMPP connection handling more resilient
matrix-org · Jul 29, 2021 · 87bb575 · 87bb575
2 parents 452e4f4 + 91376a9
commit 87bb575
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 8 deletions.
diff --git a/changelog.d/264.bugfix b/changelog.d/264.bugfix
@@ -0,0 +1 @@
+Make XMPP connection handling more resilient
diff --git a/src/xmppjs/XJSInstance.ts b/src/xmppjs/XJSInstance.ts
@@ -67,7 +67,6 @@ export class XmppJsInstance extends EventEmitter implements IBifrostInstance {
     private myAddress!: JID;
     private accounts: Map<string, XmppJsAccount>;
     private seenMessages: Set<string>;
-    private canWrite: boolean;
     private defaultRes!: string;
     private connectionWasDropped: boolean;
     private bufferedMessages: {xmlMsg: Element|string, resolve: (res: Promise<void>) => void}[];
@@ -78,7 +77,6 @@ export class XmppJsInstance extends EventEmitter implements IBifrostInstance {
     private lastMessageInMUC: Map<string, {originIsMatrix: boolean, id: string}>;
     constructor(private config: Config) {
         super();
-        this.canWrite = false;
         this.accounts = new Map();
         this.bufferedMessages = [];
         this.seenMessages = new Set();
@@ -148,7 +146,16 @@ export class XmppJsInstance extends EventEmitter implements IBifrostInstance {
         const xml = typeof(xmlMsg) === "string" ? xmlMsg : xmlMsg.xml;
         let p: Promise<unknown>;
         if (this.canWrite) {
-            p = this.xmpp.write(xml);
+            this.xmpp.write(xml).catch((err: Error) => {
+                // This should only happen in case of a connection error
+                // that xmpp.js hasn't noticed yet for some reason.
+                // xmpp.js recovers from these automatically,
+                // so we can reschedule this for post-connection and hope it goes through then.
+                log.error("Error writing xmpp stanza:", err.toString(), "scheduling it for later");
+                p = new Promise((resolve) => {
+                    this.bufferedMessages.push({xmlMsg: xml, resolve});
+                });
+            });
         } else {
             p = new Promise((resolve) => {
                 this.bufferedMessages.push({xmlMsg: xml, resolve});
@@ -242,7 +249,6 @@ export class XmppJsInstance extends EventEmitter implements IBifrostInstance {
         xmpp.on("online", (address) => {
             xLog.info("gone online as " + address);
             this.myAddress = address;
-            this.canWrite = true;
             log.info(`flushing ${this.bufferedMessages.length} buffered messages`);
             if (this.connectionWasDropped) {
                 log.warn("Connection was dropped, attempting reconnect..");
@@ -262,9 +268,6 @@ export class XmppJsInstance extends EventEmitter implements IBifrostInstance {
 
         // Debug
         xmpp.on("status", (status) => {
-            if (status === "disconnecting" || status === "disconnected") {
-                this.canWrite = false;
-            }
             if (status === "disconnect") {
                 log.error("Connection to XMPP server was lost..");
                 this.connectionWasDropped = true;
@@ -280,7 +283,6 @@ export class XmppJsInstance extends EventEmitter implements IBifrostInstance {
             xLog.info("status: reconnecting");
         });
 
-
         if (opts.logRawStream) {
             xmpp.on("input", (input) => {
                 xLog.debug("RX:", input);
@@ -938,4 +940,8 @@ export class XmppJsInstance extends EventEmitter implements IBifrostInstance {
             }
         }
     }
+
+    private get canWrite(): boolean {
+        return this.xmpp?.status === 'online';
+    }
 }