Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Commit d092051

Browse files
author
Kerry
authored
Live location sharing - Stop publishing location to beacons with consecutive errors (#8194)
* add error state after consecutive errors Signed-off-by: Kerry Archibald <[email protected]> * polish Signed-off-by: Kerry Archibald <[email protected]> * comment Signed-off-by: Kerry Archibald <[email protected]> * remove debug Signed-off-by: Kerry Archibald <[email protected]>
1 parent 31cd7ed commit d092051

File tree

2 files changed

+194
-7
lines changed

2 files changed

+194
-7
lines changed

src/stores/OwnBeaconStore.ts

Lines changed: 62 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,14 @@ const isOwnBeacon = (beacon: Beacon, userId: string): boolean => beacon.beaconIn
4848
export enum OwnBeaconStoreEvent {
4949
LivenessChange = 'OwnBeaconStore.LivenessChange',
5050
MonitoringLivePosition = 'OwnBeaconStore.MonitoringLivePosition',
51+
WireError = 'WireError',
5152
}
5253

5354
const MOVING_UPDATE_INTERVAL = 2000;
5455
const STATIC_UPDATE_INTERVAL = 30000;
5556

57+
const BAIL_AFTER_CONSECUTIVE_ERROR_COUNT = 2;
58+
5659
type OwnBeaconStoreState = {
5760
beacons: Map<string, Beacon>;
5861
beaconWireErrors: Map<string, Beacon>;
@@ -65,9 +68,11 @@ export class OwnBeaconStore extends AsyncStoreWithClient<OwnBeaconStoreState> {
6568
public readonly beacons = new Map<string, Beacon>();
6669
public readonly beaconsByRoomId = new Map<Room['roomId'], Set<string>>();
6770
/**
68-
* Track over the wire errors for beacons
71+
* Track over the wire errors for published positions
72+
* Counts consecutive wire errors per beacon
73+
* Reset on successful publish of location
6974
*/
70-
public readonly beaconWireErrors = new Map<string, Error>();
75+
public readonly beaconWireErrorCounts = new Map<string, number>();
7176
private liveBeaconIds = [];
7277
private locationInterval: number;
7378
private geolocationError: GeolocationError | undefined;
@@ -106,7 +111,7 @@ export class OwnBeaconStore extends AsyncStoreWithClient<OwnBeaconStoreState> {
106111
this.beacons.clear();
107112
this.beaconsByRoomId.clear();
108113
this.liveBeaconIds = [];
109-
this.beaconWireErrors.clear();
114+
this.beaconWireErrorCounts.clear();
110115
}
111116

112117
protected async onReady(): Promise<void> {
@@ -125,6 +130,25 @@ export class OwnBeaconStore extends AsyncStoreWithClient<OwnBeaconStoreState> {
125130
return !!this.getLiveBeaconIds(roomId).length;
126131
}
127132

133+
/**
134+
* If a beacon has failed to publish position
135+
* past the allowed consecutive failure count (BAIL_AFTER_CONSECUTIVE_ERROR_COUNT)
136+
* Then consider it to have an error
137+
*/
138+
public hasWireError(beaconId: string): boolean {
139+
return this.beaconWireErrorCounts.get(beaconId) >= BAIL_AFTER_CONSECUTIVE_ERROR_COUNT;
140+
}
141+
142+
public resetWireError(beaconId: string): void {
143+
this.incrementBeaconWireErrorCount(beaconId, false);
144+
145+
// always publish to all live beacons together
146+
// instead of just one that was changed
147+
// to keep lastPublishedTimestamp simple
148+
// and extra published locations don't hurt
149+
this.publishCurrentLocationToBeacons();
150+
}
151+
128152
public getLiveBeaconIds(roomId?: string): string[] {
129153
if (!roomId) {
130154
return this.liveBeaconIds;
@@ -202,6 +226,13 @@ export class OwnBeaconStore extends AsyncStoreWithClient<OwnBeaconStoreState> {
202226
* State management
203227
*/
204228

229+
/**
230+
* Live beacon ids that do not have wire errors
231+
*/
232+
private get healthyLiveBeaconIds() {
233+
return this.liveBeaconIds.filter(beaconId => !this.hasWireError(beaconId));
234+
}
235+
205236
private initialiseBeaconState = () => {
206237
const userId = this.matrixClient.getUserId();
207238
const visibleRooms = this.matrixClient.getVisibleRooms();
@@ -399,7 +430,7 @@ export class OwnBeaconStore extends AsyncStoreWithClient<OwnBeaconStoreState> {
399430
*/
400431
private publishLocationToBeacons = async (position: TimedGeoUri) => {
401432
this.lastPublishedPositionTimestamp = Date.now();
402-
await Promise.all(this.liveBeaconIds.map(beaconId =>
433+
await Promise.all(this.healthyLiveBeaconIds.map(beaconId =>
403434
this.sendLocationToBeacon(this.beacons.get(beaconId), position)),
404435
);
405436
};
@@ -413,9 +444,35 @@ export class OwnBeaconStore extends AsyncStoreWithClient<OwnBeaconStoreState> {
413444
const content = makeBeaconContent(geoUri, timestamp, beacon.beaconInfoId);
414445
try {
415446
await this.matrixClient.sendEvent(beacon.roomId, M_BEACON.name, content);
447+
this.incrementBeaconWireErrorCount(beacon.identifier, false);
416448
} catch (error) {
417449
logger.error(error);
418-
this.beaconWireErrors.set(beacon.identifier, error);
450+
this.incrementBeaconWireErrorCount(beacon.identifier, true);
451+
}
452+
};
453+
454+
/**
455+
* Manage beacon wire error count
456+
* - clear count for beacon when not error
457+
* - increment count for beacon when is error
458+
* - emit if beacon error count crossed threshold
459+
*/
460+
private incrementBeaconWireErrorCount = (beaconId: string, isError: boolean): void => {
461+
const hadError = this.hasWireError(beaconId);
462+
463+
if (isError) {
464+
// increment error count
465+
this.beaconWireErrorCounts.set(
466+
beaconId,
467+
(this.beaconWireErrorCounts.get(beaconId) ?? 0) + 1,
468+
);
469+
} else {
470+
// clear any error count
471+
this.beaconWireErrorCounts.delete(beaconId);
472+
}
473+
474+
if (this.hasWireError(beaconId) !== hadError) {
475+
this.emit(OwnBeaconStoreEvent.WireError, beaconId);
419476
}
420477
};
421478
}

test/stores/OwnBeaconStore-test.ts

Lines changed: 132 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ describe('OwnBeaconStore', () => {
166166
geolocation = mockGeolocation();
167167
mockClient.getVisibleRooms.mockReturnValue([]);
168168
mockClient.unstable_setLiveBeacon.mockClear().mockResolvedValue({ event_id: '1' });
169-
mockClient.sendEvent.mockClear().mockResolvedValue({ event_id: '1' });
169+
mockClient.sendEvent.mockReset().mockResolvedValue({ event_id: '1' });
170170
jest.spyOn(global.Date, 'now').mockReturnValue(now);
171171
jest.spyOn(OwnBeaconStore.instance, 'emit').mockRestore();
172172
jest.spyOn(logger, 'error').mockRestore();
@@ -696,7 +696,7 @@ describe('OwnBeaconStore', () => {
696696
});
697697
});
698698

699-
describe('sending positions', () => {
699+
describe('publishing positions', () => {
700700
it('stops watching position when user has no more live beacons', async () => {
701701
// geolocation is only going to emit 1 position
702702
geolocation.watchPosition.mockImplementation(
@@ -825,6 +825,136 @@ describe('OwnBeaconStore', () => {
825825
});
826826
});
827827

828+
describe('when publishing position fails', () => {
829+
beforeEach(() => {
830+
geolocation.watchPosition.mockImplementation(
831+
watchPositionMockImplementation([0, 1000, 3000, 3000, 3000]),
832+
);
833+
834+
// eat expected console error logs
835+
jest.spyOn(logger, 'error').mockImplementation(() => { });
836+
});
837+
838+
// we need to advance time and then flush promises
839+
// individually for each call to sendEvent
840+
// otherwise the sendEvent doesn't reject/resolve and update state
841+
// before the next call
842+
// advance and flush every 1000ms
843+
// until given ms is 'elapsed'
844+
const advanceAndFlushPromises = async (timeMs: number) => {
845+
while (timeMs > 0) {
846+
jest.advanceTimersByTime(1000);
847+
await flushPromisesWithFakeTimers();
848+
timeMs -= 1000;
849+
}
850+
};
851+
852+
it('continues publishing positions after one publish error', async () => {
853+
// fail to send first event, then succeed
854+
mockClient.sendEvent.mockRejectedValueOnce(new Error('oups')).mockResolvedValue({ event_id: '1' });
855+
makeRoomsWithStateEvents([
856+
alicesRoom1BeaconInfo,
857+
]);
858+
const store = await makeOwnBeaconStore();
859+
// wait for store to settle
860+
await flushPromisesWithFakeTimers();
861+
862+
await advanceAndFlushPromises(50000);
863+
864+
// called for each position from watchPosition
865+
expect(mockClient.sendEvent).toHaveBeenCalledTimes(5);
866+
expect(store.hasWireError(alicesRoom1BeaconInfo.getType())).toBe(false);
867+
});
868+
869+
it('continues publishing positions when a beacon fails intermittently', async () => {
870+
// every second event rejects
871+
// meaning this beacon has more errors than the threshold
872+
// but they are not consecutive
873+
mockClient.sendEvent
874+
.mockRejectedValueOnce(new Error('oups'))
875+
.mockResolvedValueOnce({ event_id: '1' })
876+
.mockRejectedValueOnce(new Error('oups'))
877+
.mockResolvedValueOnce({ event_id: '1' })
878+
.mockRejectedValueOnce(new Error('oups'));
879+
880+
makeRoomsWithStateEvents([
881+
alicesRoom1BeaconInfo,
882+
]);
883+
const store = await makeOwnBeaconStore();
884+
const emitSpy = jest.spyOn(store, 'emit');
885+
// wait for store to settle
886+
await flushPromisesWithFakeTimers();
887+
888+
await advanceAndFlushPromises(50000);
889+
890+
// called for each position from watchPosition
891+
expect(mockClient.sendEvent).toHaveBeenCalledTimes(5);
892+
expect(store.hasWireError(alicesRoom1BeaconInfo.getType())).toBe(false);
893+
expect(emitSpy).not.toHaveBeenCalledWith(
894+
OwnBeaconStoreEvent.WireError, alicesRoom1BeaconInfo.getType(),
895+
);
896+
});
897+
898+
it('stops publishing positions when a beacon fails consistently', async () => {
899+
// always fails to send events
900+
mockClient.sendEvent.mockRejectedValue(new Error('oups'));
901+
makeRoomsWithStateEvents([
902+
alicesRoom1BeaconInfo,
903+
]);
904+
const store = await makeOwnBeaconStore();
905+
const emitSpy = jest.spyOn(store, 'emit');
906+
// wait for store to settle
907+
await flushPromisesWithFakeTimers();
908+
909+
// 5 positions from watchPosition in this period
910+
await advanceAndFlushPromises(50000);
911+
912+
// only two allowed failures
913+
expect(mockClient.sendEvent).toHaveBeenCalledTimes(2);
914+
expect(store.hasWireError(alicesRoom1BeaconInfo.getType())).toBe(true);
915+
expect(emitSpy).toHaveBeenCalledWith(
916+
OwnBeaconStoreEvent.WireError, alicesRoom1BeaconInfo.getType(),
917+
);
918+
});
919+
920+
it('restarts publishing a beacon after resetting wire error', async () => {
921+
// always fails to send events
922+
mockClient.sendEvent.mockRejectedValue(new Error('oups'));
923+
makeRoomsWithStateEvents([
924+
alicesRoom1BeaconInfo,
925+
]);
926+
const store = await makeOwnBeaconStore();
927+
const emitSpy = jest.spyOn(store, 'emit');
928+
// wait for store to settle
929+
await flushPromisesWithFakeTimers();
930+
931+
// 3 positions from watchPosition in this period
932+
await advanceAndFlushPromises(4000);
933+
934+
// only two allowed failures
935+
expect(mockClient.sendEvent).toHaveBeenCalledTimes(2);
936+
expect(store.hasWireError(alicesRoom1BeaconInfo.getType())).toBe(true);
937+
expect(emitSpy).toHaveBeenCalledWith(
938+
OwnBeaconStoreEvent.WireError, alicesRoom1BeaconInfo.getType(),
939+
);
940+
941+
// reset emitSpy mock counts to asser on wireError again
942+
emitSpy.mockClear();
943+
store.resetWireError(alicesRoom1BeaconInfo.getType());
944+
945+
expect(store.hasWireError(alicesRoom1BeaconInfo.getType())).toBe(false);
946+
947+
// 2 more positions from watchPosition in this period
948+
await advanceAndFlushPromises(10000);
949+
950+
// 2 from before, 2 new ones
951+
expect(mockClient.sendEvent).toHaveBeenCalledTimes(4);
952+
expect(emitSpy).toHaveBeenCalledWith(
953+
OwnBeaconStoreEvent.WireError, alicesRoom1BeaconInfo.getType(),
954+
);
955+
});
956+
});
957+
828958
it('publishes subsequent positions', async () => {
829959
// modern fake timers + debounce + promises are not friends
830960
// just testing that positions are published

0 commit comments

Comments
 (0)