LLS: error handling on stopping beacon (#8406)

* shared stopping error state for timeline, maxi and room warnign Signed-off-by: Kerry Archibald <[email protected]> * check for stopping errors in roomlist share warning Signed-off-by: Kerry Archibald <[email protected]> * lint Signed-off-by: Kerry Archibald <[email protected]> * test stopping errors in OwnBeaconStore Signed-off-by: Kerry Archibald <[email protected]> * update LeftPanelLiveShareWarning tests for stopping errors Signed-off-by: Kerry Archibald <[email protected]> * reinstate try/catch for stopping beacons in create Signed-off-by: Kerry Archibald <[email protected]> * remove unnecessary and buggy beacon stopping on creation Signed-off-by: Kerry Archibald <[email protected]>
matrix-org · Apr 28, 2022 · 472222c · 472222c
1 parent 1bceeb2
commit 472222c
Show file tree

Hide file tree

Showing 8 changed files with 213 additions and 49 deletions.
diff --git a/src/components/views/beacon/LeftPanelLiveShareWarning.tsx b/src/components/views/beacon/LeftPanelLiveShareWarning.tsx
@@ -16,6 +16,7 @@ limitations under the License.
 
 import classNames from 'classnames';
 import React from 'react';
+import { BeaconIdentifier, Room } from 'matrix-js-sdk/src/matrix';
 
 import { useEventEmitterState } from '../../../hooks/useEventEmitter';
 import { _t } from '../../../languageHandler';
@@ -34,10 +35,14 @@ interface Props {
  * Choose the most relevant beacon
  * and get its roomId
  */
-const chooseBestBeaconRoomId = (liveBeaconIds, errorBeaconIds): string | undefined => {
+const chooseBestBeaconRoomId = (
+    liveBeaconIds: BeaconIdentifier[],
+    updateErrorBeaconIds: BeaconIdentifier[],
+    locationErrorBeaconIds: BeaconIdentifier[],
+): Room['roomId'] | undefined => {
     // both lists are ordered by creation timestamp in store
     // so select latest beacon
-    const beaconId = errorBeaconIds?.[0] ?? liveBeaconIds?.[0];
+    const beaconId = updateErrorBeaconIds?.[0] ?? locationErrorBeaconIds?.[0] ?? liveBeaconIds?.[0];
     if (!beaconId) {
         return undefined;
     }
@@ -46,6 +51,16 @@ const chooseBestBeaconRoomId = (liveBeaconIds, errorBeaconIds): string | undefin
     return beacon?.roomId;
 };
 
+const getLabel = (hasStoppingErrors: boolean, hasLocationErrors: boolean): string => {
+    if (hasStoppingErrors) {
+        return _t('An error occurred while stopping your live location');
+    }
+    if (hasLocationErrors) {
+        return _t('An error occured whilst sharing your live location');
+    }
+    return _t('You are sharing your live location');
+};
+
 const LeftPanelLiveShareWarning: React.FC<Props> = ({ isMinimized }) => {
     const isMonitoringLiveLocation = useEventEmitterState(
         OwnBeaconStore.instance,
@@ -59,19 +74,30 @@ const LeftPanelLiveShareWarning: React.FC<Props> = ({ isMinimized }) => {
         () => OwnBeaconStore.instance.getLiveBeaconIdsWithLocationPublishError(),
     );
 
+    const beaconIdsWithStoppingError = useEventEmitterState(
+        OwnBeaconStore.instance,
+        OwnBeaconStoreEvent.BeaconUpdateError,
+        () => OwnBeaconStore.instance.getLiveBeaconIds().filter(
+            beaconId => OwnBeaconStore.instance.beaconUpdateErrors.has(beaconId),
+        ),
+    );
+
     const liveBeaconIds = useEventEmitterState(
         OwnBeaconStore.instance,
         OwnBeaconStoreEvent.LivenessChange,
         () => OwnBeaconStore.instance.getLiveBeaconIds(),
     );
 
     const hasLocationPublishErrors = !!beaconIdsWithLocationPublishError.length;
+    const hasStoppingErrors = !!beaconIdsWithStoppingError.length;
 
     if (!isMonitoringLiveLocation) {
         return null;
     }
 
-    const relevantBeaconRoomId = chooseBestBeaconRoomId(liveBeaconIds, beaconIdsWithLocationPublishError);
+    const relevantBeaconRoomId = chooseBestBeaconRoomId(
+        liveBeaconIds, beaconIdsWithStoppingError, beaconIdsWithLocationPublishError,
+    );
 
     const onWarningClick = relevantBeaconRoomId ? () => {
         dispatcher.dispatch<ViewRoomPayload>({
@@ -81,14 +107,12 @@ const LeftPanelLiveShareWarning: React.FC<Props> = ({ isMinimized }) => {
         });
     } : undefined;
 
-    const label = hasLocationPublishErrors ?
-        _t('An error occured whilst sharing your live location') :
-        _t('You are sharing your live location');
+    const label = getLabel(hasStoppingErrors, hasLocationPublishErrors);
 
     return <AccessibleButton
         className={classNames('mx_LeftPanelLiveShareWarning', {
             'mx_LeftPanelLiveShareWarning__minimized': isMinimized,
-            'mx_LeftPanelLiveShareWarning__error': hasLocationPublishErrors,
+            'mx_LeftPanelLiveShareWarning__error': hasLocationPublishErrors || hasStoppingErrors,
         })}
         title={isMinimized ? label : undefined}
         onClick={onWarningClick}

diff --git a/src/components/views/beacon/RoomLiveShareWarning.tsx b/src/components/views/beacon/RoomLiveShareWarning.tsx
@@ -15,7 +15,6 @@ limitations under the License.
 */
 
 import React from 'react';
-import classNames from 'classnames';
 import { Room } from 'matrix-js-sdk/src/matrix';
 
 import { _t } from '../../../languageHandler';
@@ -67,7 +66,7 @@ const RoomLiveShareWarningInner: React.FC<RoomLiveShareWarningInnerProps> = ({ l
     };
 
     return <div
-        className={classNames('mx_RoomLiveShareWarning')}
+        className='mx_RoomLiveShareWarning'
     >
         <StyledLiveBeaconIcon className="mx_RoomLiveShareWarning_icon" withError={hasError} />
 

diff --git a/src/components/views/messages/MBeaconBody.tsx b/src/components/views/messages/MBeaconBody.tsx
@@ -146,6 +146,7 @@ const MBeaconBody: React.FC<IBodyProps> = React.forwardRef(({ mxEvent }, ref) =>
                     className='mx_MBeaconBody_chin'
                     beacon={beacon}
                     displayStatus={displayStatus}
+                    withIcon
                 /> :
                 <BeaconStatus
                     className='mx_MBeaconBody_chin'

diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json
@@ -2948,6 +2948,7 @@
     "View list": "View list",
     "View List": "View List",
     "Close sidebar": "Close sidebar",
+    "An error occurred while stopping your live location": "An error occurred while stopping your live location",
     "An error occured whilst sharing your live location": "An error occured whilst sharing your live location",
     "You are sharing your live location": "You are sharing your live location",
     "%(timeRemaining)s left": "%(timeRemaining)s left",

diff --git a/src/stores/OwnBeaconStore.ts b/src/stores/OwnBeaconStore.ts
@@ -51,6 +51,7 @@ export enum OwnBeaconStoreEvent {
     LivenessChange = 'OwnBeaconStore.LivenessChange',
     MonitoringLivePosition = 'OwnBeaconStore.MonitoringLivePosition',
     LocationPublishError = 'LocationPublishError',
+    BeaconUpdateError = 'BeaconUpdateError',
 }
 
 const MOVING_UPDATE_INTERVAL = 2000;
@@ -61,6 +62,7 @@ const BAIL_AFTER_CONSECUTIVE_ERROR_COUNT = 2;
 type OwnBeaconStoreState = {
     beacons: Map<BeaconIdentifier, Beacon>;
     beaconLocationPublishErrorCounts: Map<BeaconIdentifier, number>;
+    beaconUpdateErrors: Map<BeaconIdentifier, Error>;
     beaconsByRoomId: Map<Room['roomId'], Set<BeaconIdentifier>>;
     liveBeaconIds: BeaconIdentifier[];
 };
@@ -99,6 +101,7 @@ export class OwnBeaconStore extends AsyncStoreWithClient<OwnBeaconStoreState> {
      * Reset on successful publish of location
      */
     public readonly beaconLocationPublishErrorCounts = new Map<BeaconIdentifier, number>();
+    public readonly beaconUpdateErrors = new Map<BeaconIdentifier, Error>();
     /**
      * ids of live beacons
      * ordered by creation time descending
@@ -144,6 +147,7 @@ export class OwnBeaconStore extends AsyncStoreWithClient<OwnBeaconStoreState> {
         this.beaconsByRoomId.clear();
         this.liveBeaconIds = [];
         this.beaconLocationPublishErrorCounts.clear();
+        this.beaconUpdateErrors.clear();
     }
 
     protected async onReady(): Promise<void> {
@@ -215,7 +219,6 @@ export class OwnBeaconStore extends AsyncStoreWithClient<OwnBeaconStoreState> {
         }
 
         await this.updateBeaconEvent(beacon, { live: false });
-
         // prune from local store
         removeLocallyCreateBeaconEventId(beacon.beaconInfoId);
     };
@@ -300,7 +303,10 @@ export class OwnBeaconStore extends AsyncStoreWithClient<OwnBeaconStoreState> {
      * Live beacon ids that do not have wire errors
      */
     private get healthyLiveBeaconIds() {
-        return this.liveBeaconIds.filter(beaconId => !this.beaconHasLocationPublishError(beaconId));
+        return this.liveBeaconIds.filter(beaconId =>
+            !this.beaconHasLocationPublishError(beaconId) &&
+            !this.beaconUpdateErrors.has(beaconId),
+        );
     }
 
     private initialiseBeaconState = () => {
@@ -393,19 +399,6 @@ export class OwnBeaconStore extends AsyncStoreWithClient<OwnBeaconStoreState> {
         );
 
         storeLocallyCreateBeaconEventId(event_id);
-
-        // try to stop any other live beacons
-        // in this room
-        this.beaconsByRoomId.get(roomId)?.forEach(beaconId => {
-            if (this.getBeaconById(beaconId)?.isLive) {
-                try {
-                    // don't await, this is best effort
-                    this.stopBeacon(beaconId);
-                } catch (error) {
-                    logger.error('Failed to stop live beacons', error);
-                }
-            }
-        });
     };
 
     /**
@@ -510,6 +503,11 @@ export class OwnBeaconStore extends AsyncStoreWithClient<OwnBeaconStoreState> {
      * MatrixClient api
      */
 
+    /**
+     * Updates beacon with provided content update
+     * Records error in beaconUpdateErrors
+     * rethrows
+     */
     private updateBeaconEvent = async (beacon: Beacon, update: Partial<BeaconInfoState>): Promise<void> => {
         const { description, timeout, timestamp, live, assetType } = {
             ...beacon.beaconInfo,
@@ -523,7 +521,21 @@ export class OwnBeaconStore extends AsyncStoreWithClient<OwnBeaconStoreState> {
             timestamp,
         );
 
-        await this.matrixClient.unstable_setLiveBeacon(beacon.roomId, updateContent);
+        try {
+            await this.matrixClient.unstable_setLiveBeacon(beacon.roomId, updateContent);
+            // cleanup any errors
+            const hadError = this.beaconUpdateErrors.has(beacon.identifier);
+            if (hadError) {
+                this.beaconUpdateErrors.delete(beacon.identifier);
+                this.emit(OwnBeaconStoreEvent.BeaconUpdateError, beacon.identifier, false);
+            }
+        } catch (error) {
+            logger.error('Failed to update beacon', error);
+            this.beaconUpdateErrors.set(beacon.identifier, error);
+            this.emit(OwnBeaconStoreEvent.BeaconUpdateError, beacon.identifier, true);
+
+            throw error;
+        }
     };
 
     /**

diff --git a/src/utils/beacon/useOwnLiveBeacons.ts b/src/utils/beacon/useOwnLiveBeacons.ts
@@ -39,7 +39,6 @@ type LiveBeaconsState = {
  */
 export const useOwnLiveBeacons = (liveBeaconIds: BeaconIdentifier[]): LiveBeaconsState => {
     const [stoppingInProgress, setStoppingInProgress] = useState(false);
-    const [error, setError] = useState<Error>();
 
     const hasLocationPublishError = useEventEmitterState(
         OwnBeaconStore.instance,
@@ -48,10 +47,22 @@ export const useOwnLiveBeacons = (liveBeaconIds: BeaconIdentifier[]): LiveBeacon
             liveBeaconIds.some(OwnBeaconStore.instance.beaconHasLocationPublishError),
     );
 
+    const hasStopSharingError = useEventEmitterState(
+        OwnBeaconStore.instance,
+        OwnBeaconStoreEvent.BeaconUpdateError,
+        () =>
+            liveBeaconIds.some(id => OwnBeaconStore.instance.beaconUpdateErrors.has(id)),
+    );
+
+    useEffect(() => {
+        if (hasStopSharingError) {
+            setStoppingInProgress(false);
+        }
+    }, [hasStopSharingError]);
+
     // reset stopping in progress on change in live ids
     useEffect(() => {
         setStoppingInProgress(false);
-        setError(undefined);
     }, [liveBeaconIds]);
 
     // select the beacon with latest expiry to display expiry time
@@ -64,10 +75,6 @@ export const useOwnLiveBeacons = (liveBeaconIds: BeaconIdentifier[]): LiveBeacon
         try {
             await Promise.all(liveBeaconIds.map(beaconId => OwnBeaconStore.instance.stopBeacon(beaconId)));
         } catch (error) {
-            // only clear loading in case of error
-            // to avoid flash of not-loading state
-            // after beacons have been stopped but we wait for sync
-            setError(error);
             setStoppingInProgress(false);
         }
     };
@@ -82,6 +89,6 @@ export const useOwnLiveBeacons = (liveBeaconIds: BeaconIdentifier[]): LiveBeacon
         beacon,
         stoppingInProgress,
         hasLocationPublishError,
-        hasStopSharingError: !!error,
+        hasStopSharingError,
     };
 };
diff --git a/test/components/views/beacon/LeftPanelLiveShareWarning-test.tsx b/test/components/views/beacon/LeftPanelLiveShareWarning-test.tsx
@@ -18,7 +18,7 @@ import React from 'react';
 import { mocked } from 'jest-mock';
 import { mount } from 'enzyme';
 import { act } from 'react-dom/test-utils';
-import { Beacon } from 'matrix-js-sdk/src/matrix';
+import { Beacon, BeaconIdentifier } from 'matrix-js-sdk/src/matrix';
 
 import LeftPanelLiveShareWarning from '../../../../src/components/views/beacon/LeftPanelLiveShareWarning';
 import { OwnBeaconStore, OwnBeaconStoreEvent } from '../../../../src/stores/OwnBeaconStore';
@@ -33,6 +33,7 @@ jest.mock('../../../../src/stores/OwnBeaconStore', () => {
         public getLiveBeaconIdsWithLocationPublishError = jest.fn().mockReturnValue([]);
         public getBeaconById = jest.fn();
         public getLiveBeaconIds = jest.fn().mockReturnValue([]);
+        public readonly beaconUpdateErrors = new Map<BeaconIdentifier, Error>();
     }
     return {
         // @ts-ignore
@@ -59,6 +60,8 @@ describe('<LeftPanelLiveShareWarning />', () => {
     beforeEach(() => {
         jest.spyOn(global.Date, 'now').mockReturnValue(now);
         jest.spyOn(dispatcher, 'dispatch').mockClear().mockImplementation(() => { });
+
+        OwnBeaconStore.instance.beaconUpdateErrors.clear();
     });
 
     afterAll(() => {
@@ -191,5 +194,55 @@ describe('<LeftPanelLiveShareWarning />', () => {
 
             expect(component.html()).toBe(null);
         });
+
+        describe('stopping errors', () => {
+            it('renders stopping error', () => {
+                OwnBeaconStore.instance.beaconUpdateErrors.set(beacon2.identifier, new Error('error'));
+                const component = getComponent();
+                expect(component.text()).toEqual('An error occurred while stopping your live location');
+            });
+
+            it('starts rendering stopping error on beaconUpdateError emit', () => {
+                const component = getComponent();
+                // no error
+                expect(component.text()).toEqual('You are sharing your live location');
+
+                act(() => {
+                    OwnBeaconStore.instance.beaconUpdateErrors.set(beacon2.identifier, new Error('error'));
+                    OwnBeaconStore.instance.emit(OwnBeaconStoreEvent.BeaconUpdateError, beacon2.identifier, true);
+                });
+
+                expect(component.text()).toEqual('An error occurred while stopping your live location');
+            });
+
+            it('renders stopping error when beacons have stopping and location errors', () => {
+                mocked(OwnBeaconStore.instance).getLiveBeaconIdsWithLocationPublishError.mockReturnValue(
+                    [beacon1.identifier],
+                );
+                OwnBeaconStore.instance.beaconUpdateErrors.set(beacon2.identifier, new Error('error'));
+                const component = getComponent();
+                expect(component.text()).toEqual('An error occurred while stopping your live location');
+            });
+
+            it('goes to room of latest beacon with stopping error when clicked', () => {
+                mocked(OwnBeaconStore.instance).getLiveBeaconIdsWithLocationPublishError.mockReturnValue(
+                    [beacon1.identifier],
+                );
+                OwnBeaconStore.instance.beaconUpdateErrors.set(beacon2.identifier, new Error('error'));
+                const component = getComponent();
+                const dispatchSpy = jest.spyOn(dispatcher, 'dispatch');
+
+                act(() => {
+                    component.simulate('click');
+                });
+
+                expect(dispatchSpy).toHaveBeenCalledWith({
+                    action: Action.ViewRoom,
+                    metricsTrigger: undefined,
+                    // stopping error beacon's room
+                    room_id: beacon2.roomId,
+                });
+            });
+        });
     });
 });