Merge pull request #834 from vector-im/fix-627

Gap fill from HS should halt on network disconnection

src/domain/session/room/timeline/tiles/GapTile.js

@@ -16,6 +16,8 @@ limitations under the License.
 
 import {SimpleTile} from "./SimpleTile.js";
 import {UpdateAction} from "../UpdateAction.js";
+import {ConnectionError} from "../../../../../matrix/error.js";
+import {ConnectionStatus} from "../../../../../matrix/net/Reconnector";
 
 export class GapTile extends SimpleTile {
     constructor(entry, options) {
@@ -24,23 +26,37 @@ export class GapTile extends SimpleTile {
         this._error = null;
         this._isAtTop = true;
         this._siblingChanged = false;
+        this._showSpinner = false;
     }
 
     async fill() {
         if (!this._loading && !this._entry.edgeReached) {
             this._loading = true;
+            this._error = null;
+            this._showSpinner = true;
             this.emitChange("isLoading");
             try {
                 await this._room.fillGap(this._entry, 10);
             } catch (err) {
                 console.error(`room.fillGap(): ${err.message}:\n${err.stack}`);
                 this._error = err;
+                if (err instanceof ConnectionError) {
                     this.emitChange("error");
+                    /*
+                    We need to wait for reconnection here rather than in
+                    notifyVisible() because when we return/throw here
+                    this._loading is set to false and other queued invocations of
+                    this method will succeed and attempt further room.fillGap() calls - 
+                    resulting in multiple error entries in logs and elsewhere!
+                    */
+                    await this._waitForReconnection();
+                }
                 // rethrow so caller of this method
                 // knows not to keep calling this for now
                 throw err;
             } finally {
                 this._loading = false;
+                this._showSpinner = false;
                 this.emitChange("isLoading");
             }
                 return true;
@@ -55,7 +71,19 @@ export class GapTile extends SimpleTile {
         let canFillMore;
         this._siblingChanged = false;
         do {
+            try {
                 canFillMore = await this.fill();
+            }
+            catch (e) {
+                if (e instanceof ConnectionError) {
+                    canFillMore = true;
+                    // Don't increase depth because this gap fill was a noop
+                    continue;
+                }
+                else {
+                    canFillMore = false;
+                }
+            }
             depth = depth + 1;
         } while (depth < 10 && !this._siblingChanged && canFillMore && !this.isDisposed);
     }
@@ -90,6 +118,10 @@ export class GapTile extends SimpleTile {
         }
     }
 
+    async _waitForReconnection() {
+        await this.options.client.reconnector.connectionStatus.waitFor(status => status === ConnectionStatus.Online).promise;
+    }
+
     get shape() {
         return "gap";
     }
@@ -98,13 +130,32 @@ export class GapTile extends SimpleTile {
         return this._loading;
     }
 
+    get showSpinner() {
+        return this._showSpinner;
+    }
+
     get error() {
         if (this._error) {
+            if (this._error instanceof ConnectionError) {
+                return "Waiting for reconnection";
+            }
             const dir = this._entry.prev_batch ? "previous" : "next";
             return `Could not load ${dir} messages: ${this._error.message}`;
         }
         return null;
     }
+
+    get currentAction() {
+        if (this.error) {
+            return this.error;
+        }
+        else if (this.isLoading) {
+            return "Loading";
+        }
+        else {
+            return "Not Loading";
+        }
+    }
 }
 
 import {FragmentBoundaryEntry} from "../../../../../matrix/room/timeline/entries/FragmentBoundaryEntry.js";

src/platform/web/ui/session/room/timeline/GapView.js

@@ -30,9 +30,8 @@ export class GapView extends TemplateView {
             isAtTop: vm => vm.isAtTop,
         };
         return t.li({ className }, [
-            spinner(t),
+            t.if(vm => vm.showSpinner, (t) => spinner(t)),
-            t.div(vm => vm.isLoading ? vm.i18n`Loading more messages …` : vm.i18n`Not loading!`),
+            t.span(vm => vm.currentAction)
-            t.if(vm => vm.error, t => t.strong(vm => vm.error))
         ]);
     }