Fix echo handling and show a barebones toast on error

The EchoTransaction was wrongly assuming that it knew better than the caller for when the success condition was met, so the echo marking has been left an exercise for the caller. In this case, we mark when we finally receive the sync with the updated rules.

We also have to cancel previous transactions otherwise if the user mashes buttons we could forever show the toast, and that would be bad.

src/components/views/toasts/NonUrgentEchoFailureToast.tsx

@@ -0,0 +1,30 @@
+/*
+Copyright 2020 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import React from "react";
+import { _t } from "../../../languageHandler";
+
+export default class NonUrgentEchoFailureToast extends React.PureComponent {
+    render() {
+        return (
+            <div className="mx_NonUrgentEchoFailureToast">
+                {_t("Your server isn't responding to some <a>requests</a>", {}, {
+                    'a': (sub) => <a>{sub}</a>
+                })}
+            </div>
+        )
+    }
+}

src/i18n/strings/en_EN.json

@@ -612,6 +612,7 @@
     "Headphones": "Headphones",
     "Folder": "Folder",
     "Pin": "Pin",
+    "Your server isn't responding to some <a>requests</a>": "Your server isn't responding to some <a>requests</a>",
     "From %(deviceName)s (%(deviceId)s)": "From %(deviceName)s (%(deviceId)s)",
     "Decline (%(counter)s)": "Decline (%(counter)s)",
     "Accept <policyLink /> to continue:": "Accept <policyLink /> to continue:",

src/stores/local-echo/CachedEcho.ts

@@ -15,7 +15,7 @@ limitations under the License.
 */
 
 import { EchoContext } from "./EchoContext";
-import { RunFn, TransactionStatus } from "./EchoTransaction";
+import { EchoTransaction, RunFn, TransactionStatus } from "./EchoTransaction";
 import { MatrixClient } from "matrix-js-sdk/src/client";
 import { EventEmitter } from "events";
 
@@ -26,10 +26,10 @@ export async function implicitlyReverted() {
 export const PROPERTY_UPDATED = "property_updated";
 
 export abstract class CachedEcho<C extends EchoContext, K, V> extends EventEmitter {
-    private cache = new Map<K, V>();
+    private cache = new Map<K, {txn: EchoTransaction, val: V}>();
     protected matrixClient: MatrixClient;
 
-    protected constructor(protected context: C, private lookupFn: (key: K) => V) {
+    protected constructor(public readonly context: C, private lookupFn: (key: K) => V) {
         super();
     }
 
@@ -49,24 +49,39 @@ export abstract class CachedEcho<C extends EchoContext, K, V> extends EventEmitt
      * @returns The value for the key.
      */
     public getValue(key: K): V {
-        return this.cache.has(key) ? this.cache.get(key) : this.lookupFn(key);
+        return this.cache.has(key) ? this.cache.get(key).val : this.lookupFn(key);
     }
 
-    private cacheVal(key: K, val: V) {
+    private cacheVal(key: K, val: V, txn: EchoTransaction) {
-        this.cache.set(key, val);
+        this.cache.set(key, {txn, val});
         this.emit(PROPERTY_UPDATED, key);
     }
 
     private decacheKey(key: K) {
-        this.cache.delete(key);
+        if (this.cache.has(key)) {
-        this.emit(PROPERTY_UPDATED, key);
+            this.cache.get(key).txn.cancel(); // should be safe to call
+            this.cache.delete(key);
+            this.emit(PROPERTY_UPDATED, key);
+        }
+    }
+
+    protected markEchoReceived(key: K) {
+        this.decacheKey(key);
     }
 
     public setValue(auditName: string, key: K, targetVal: V, runFn: RunFn, revertFn: RunFn) {
-        this.cacheVal(key, targetVal); // set the cache now as it won't be updated by the .when() ladder below.
+        // Cancel any pending transactions for the same key
-        this.context.beginTransaction(auditName, runFn)
+        if (this.cache.has(key)) {
-            .when(TransactionStatus.Pending, () => this.cacheVal(key, targetVal))
+            this.cache.get(key).txn.cancel();
-            .whenAnyOf([TransactionStatus.DoneError, TransactionStatus.DoneSuccess], () => this.decacheKey(key))
+        }
+
+        const txn = this.context.beginTransaction(auditName, runFn);
+        this.cacheVal(key, targetVal, txn); // set the cache now as it won't be updated by the .when() ladder below.
+
+        txn.when(TransactionStatus.Pending, () => this.cacheVal(key, targetVal, txn))
+            .when(TransactionStatus.DoneError, () => this.decacheKey(key))
             .when(TransactionStatus.DoneError, () => revertFn());
+
+        txn.run();
     }
 }

src/stores/local-echo/EchoContext.ts

@@ -27,12 +27,17 @@ export enum ContextTransactionState {
 
 export abstract class EchoContext extends Whenable<ContextTransactionState> implements IDestroyable {
     private _transactions: EchoTransaction[] = [];
+    private _state = ContextTransactionState.NotStarted;
     public readonly startTime: Date = new Date();
 
     public get transactions(): EchoTransaction[] {
         return arrayFastClone(this._transactions);
     }
 
+    public get state(): ContextTransactionState {
+        return this._state;
+    }
+
     public beginTransaction(auditName: string, runFn: RunFn): EchoTransaction {
         const txn = new EchoTransaction(auditName, runFn);
         this._transactions.push(txn);
@@ -48,7 +53,7 @@ export abstract class EchoContext extends Whenable<ContextTransactionState> impl
     private checkTransactions = () => {
         let status = ContextTransactionState.AllSuccessful;
         for (const txn of this.transactions) {
-            if (txn.status === TransactionStatus.DoneError) {
+            if (txn.status === TransactionStatus.DoneError || txn.didPreviouslyFail) {
                 status = ContextTransactionState.PendingErrors;
                 break;
             } else if (txn.status === TransactionStatus.Pending) {
@@ -56,6 +61,7 @@ export abstract class EchoContext extends Whenable<ContextTransactionState> impl
                 // no break as we might hit something which broke
             }
         }
+        this._state = status;
         this.notifyCondition(status);
     };
 
@@ -63,6 +69,7 @@ export abstract class EchoContext extends Whenable<ContextTransactionState> impl
         for (const txn of this.transactions) {
             txn.destroy();
         }
+        this._transactions = [];
         super.destroy();
     }
 }

src/stores/local-echo/EchoStore.ts

@@ -22,12 +22,19 @@ import { RoomEchoContext } from "./RoomEchoContext";
 import { AsyncStoreWithClient } from "../AsyncStoreWithClient";
 import defaultDispatcher from "../../dispatcher/dispatcher";
 import { ActionPayload } from "../../dispatcher/payloads";
+import { ContextTransactionState } from "./EchoContext";
+import NonUrgentToastStore, { ToastReference } from "../NonUrgentToastStore";
+import NonUrgentEchoFailureToast from "../../components/views/toasts/NonUrgentEchoFailureToast";
+
+interface IState {
+    toastRef: ToastReference;
+}
 
 type ContextKey = string;
 
 const roomContextKey = (room: Room): ContextKey => `room-${room.roomId}`;
 
-export class EchoStore extends AsyncStoreWithClient<any> {
+export class EchoStore extends AsyncStoreWithClient<IState> {
     private static _instance: EchoStore;
 
     private caches = new Map<ContextKey, CachedEcho<any, any, any>>();
@@ -47,13 +54,35 @@ export class EchoStore extends AsyncStoreWithClient<any> {
         if (this.caches.has(roomContextKey(room))) {
             return this.caches.get(roomContextKey(room)) as RoomCachedEcho;
         }
-        const echo = new RoomCachedEcho(new RoomEchoContext(room));
+
+        const context = new RoomEchoContext(room);
+        context.whenAnything(() => this.checkContexts());
+
+        const echo = new RoomCachedEcho(context);
         echo.setClient(this.matrixClient);
         this.caches.set(roomContextKey(room), echo);
+
         return echo;
     }
 
+    private async checkContexts() {
+        let hasOrHadError = false;
+        for (const echo of this.caches.values()) {
+            hasOrHadError = echo.context.state === ContextTransactionState.PendingErrors;
+            if (hasOrHadError) break;
+        }
+
+        if (hasOrHadError && !this.state.toastRef) {
+            const ref = NonUrgentToastStore.instance.addToast(NonUrgentEchoFailureToast);
+            await this.updateState({toastRef: ref});
+        } else if (!hasOrHadError && this.state.toastRef) {
+            NonUrgentToastStore.instance.removeToast(this.state.toastRef);
+            await this.updateState({toastRef: null});
+        }
+    }
+
     protected async onReady(): Promise<any> {
+        if (!this.caches) return; // can only happen during initialization
         for (const echo of this.caches.values()) {
             echo.setClient(this.matrixClient);
         }

src/stores/local-echo/EchoTransaction.ts

@@ -53,6 +53,11 @@ export class EchoTransaction extends Whenable<TransactionStatus> {
             .catch(() => this.setStatus(TransactionStatus.DoneError));
     }
 
+    public cancel() {
+        // Success basically means "done"
+        this.setStatus(TransactionStatus.DoneSuccess);
+    }
+
     private setStatus(status: TransactionStatus) {
         this._status = status;
         if (status === TransactionStatus.DoneError) {

src/stores/local-echo/RoomCachedEcho.ts

@@ -60,6 +60,7 @@ export class RoomCachedEcho extends CachedEcho<RoomEchoContext, CachedRoomKey, C
 
     private updateNotificationVolume() {
         this.properties.set(CachedRoomKey.NotificationVolume, getRoomNotifsState(this.context.room.roomId));
+        this.markEchoReceived(CachedRoomKey.NotificationVolume);
         this.emit(PROPERTY_UPDATED, CachedRoomKey.NotificationVolume);
     }
 
@@ -71,7 +72,7 @@ export class RoomCachedEcho extends CachedEcho<RoomEchoContext, CachedRoomKey, C
 
     public set notificationVolume(v: Volume) {
         this.setValue(_t("Change notification settings"), CachedRoomKey.NotificationVolume, v, async () => {
-            setRoomNotifsState(this.context.room.roomId, v);
+            return setRoomNotifsState(this.context.room.roomId, v);
         }, implicitlyReverted);
     }
 }