Skip to content

Commit 11084fb

Browse files
committed
store: Report non-transient polling errors.
Signed-off-by: Zixuan James Li <[email protected]>
1 parent 45462ff commit 11084fb

File tree

3 files changed

+61
-41
lines changed

3 files changed

+61
-41
lines changed

assets/l10n/app_en.arb

+7
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,13 @@
160160
"message": {"type": "String", "example": "Invalid format"}
161161
}
162162
},
163+
"errorLoadingServerData": "Error loading server data. Will retry:\n\n{error}",
164+
"@errorLoadingServerData": {
165+
"description": "Dialog message for a generic unknown error.",
166+
"placeholders": {
167+
"error": {"type": "String", "example": "Invalid format"}
168+
}
169+
},
163170
"errorSharingFailed": "Sharing failed",
164171
"@errorSharingFailed": {
165172
"description": "Error message when sharing a message failed."

lib/model/store.dart

+5-1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import '../log.dart';
2020
import '../notifications/receive.dart';
2121
import 'autocomplete.dart';
2222
import 'database.dart';
23+
import 'localizations.dart';
2324
import 'message.dart';
2425
import 'message_list.dart';
2526
import 'recent_dm_conversations.dart';
@@ -782,6 +783,7 @@ class UpdateMachine {
782783
queueId: queueId, lastEventId: lastEventId);
783784
} catch (e) {
784785
store.isLoading = true;
786+
final localizations = GlobalLocalizations.zulipLocalizations;
785787
switch (e) {
786788
case ZulipApiException(code: 'BAD_EVENT_QUEUE_ID'):
787789
assert(debugLog('Lost event queue for $store. Replacing…'));
@@ -802,7 +804,9 @@ class UpdateMachine {
802804
default:
803805
assert(debugLog('Error polling event queue for $store: $e\n'
804806
'Backing off and retrying even though may be hopeless…'));
805-
// TODO tell user on non-transient error in polling
807+
// TODO(#186): Handle unrecoverable failures
808+
reportErrorToUserInDialog(
809+
localizations.errorLoadingServerData(e.toString()));
806810
await (backoffMachine ??= BackoffMachine()).wait();
807811
assert(debugLog('… Backoff wait complete, retrying poll.'));
808812
continue;

test/model/store_test.dart

+49-40
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'package:zulip/api/model/events.dart';
88
import 'package:zulip/api/model/model.dart';
99
import 'package:zulip/api/route/events.dart';
1010
import 'package:zulip/api/route/messages.dart';
11+
import 'package:zulip/log.dart';
1112
import 'package:zulip/model/store.dart';
1213
import 'package:zulip/notifications/receive.dart';
1314

@@ -427,53 +428,61 @@ void main() {
427428
check(store.userSettings!.twentyFourHourTime).isTrue();
428429
}));
429430

430-
void checkRetry(String description, void Function() prepareError) {
431-
test(description, () {
432-
awaitFakeAsync((async) async {
433-
await prepareStore(lastEventId: 1);
434-
updateMachine.debugPauseLoop();
435-
updateMachine.poll();
436-
check(async.pendingTimers).length.equals(0);
437-
438-
// Make the request, inducing an error in it.
439-
prepareError();
440-
updateMachine.debugAdvanceLoop();
441-
async.elapse(Duration.zero);
442-
checkLastRequest(lastEventId: 1);
443-
check(store).isLoading.isTrue();
444-
445-
// Polling doesn't resume immediately; there's a timer.
446-
check(async.pendingTimers).length.equals(1);
447-
updateMachine.debugAdvanceLoop();
448-
async.flushMicrotasks();
449-
check(connection.lastRequest).isNull();
450-
check(async.pendingTimers).length.equals(1);
451-
452-
// Polling continues after a timer.
453-
connection.prepare(json: GetEventsResult(events: [
454-
HeartbeatEvent(id: 2),
455-
], queueId: null).toJson());
456-
async.flushTimers();
457-
checkLastRequest(lastEventId: 1);
458-
check(updateMachine.lastEventId).equals(2);
459-
check(store).isLoading.isFalse();
460-
});
431+
void checkRetry(void Function() prepareError, {
432+
String? errorMessage,
433+
}) {
434+
awaitFakeAsync((async) async {
435+
await prepareStore(lastEventId: 1);
436+
updateMachine.debugPauseLoop();
437+
updateMachine.poll();
438+
check(async.pendingTimers).length.equals(0);
439+
440+
// Make the request, inducing an error in it.
441+
prepareError();
442+
updateMachine.debugAdvanceLoop();
443+
check(debugLastReportedError).isNull();
444+
async.elapse(Duration.zero);
445+
if (errorMessage == null) {
446+
check(debugTakeLastReportedError()).isNull();
447+
} else {
448+
check(debugTakeLastReportedError()).isNotNull().contains(errorMessage);
449+
}
450+
checkLastRequest(lastEventId: 1);
451+
check(store).isLoading.isTrue();
452+
453+
// Polling doesn't resume immediately; there's a timer.
454+
check(async.pendingTimers).length.equals(1);
455+
updateMachine.debugAdvanceLoop();
456+
async.flushMicrotasks();
457+
check(connection.lastRequest).isNull();
458+
check(async.pendingTimers).length.equals(1);
459+
460+
// Polling continues after a timer.
461+
connection.prepare(json: GetEventsResult(events: [
462+
HeartbeatEvent(id: 2),
463+
], queueId: null).toJson());
464+
async.flushTimers();
465+
checkLastRequest(lastEventId: 1);
466+
check(updateMachine.lastEventId).equals(2);
467+
check(store).isLoading.isFalse();
461468
});
462469
}
463470

464471
group('retries', () {
465-
checkRetry('retries on Server5xxException',
466-
() => connection.prepare(httpStatus: 500, body: 'splat'));
472+
test('retries on Server5xxException', () =>
473+
checkRetry(() => connection.prepare(httpStatus: 500, body: 'splat')));
467474

468-
checkRetry('retries on NetworkException',
469-
() => connection.prepare(exception: Exception("failed")));
475+
test('retries on NetworkException', () =>
476+
checkRetry(() => connection.prepare(exception: Exception("failed"))));
470477

471-
checkRetry('retries on ZulipApiException',
472-
() => connection.prepare(httpStatus: 400, json: {
473-
'result': 'error', 'code': 'BAD_REQUEST', 'msg': 'Bad request'}));
478+
test('retries on ZulipApiException', () =>
479+
checkRetry(() => connection.prepare(httpStatus: 400, json: {
480+
'result': 'error', 'code': 'BAD_REQUEST', 'msg': 'Bad request'}),
481+
errorMessage: 'Error loading server data. Will retry'));
474482

475-
checkRetry('retries on MalformedServerResponseException',
476-
() => connection.prepare(httpStatus: 200, body: 'nonsense'));
483+
test('retries on MalformedServerResponseException', () =>
484+
checkRetry(() => connection.prepare(httpStatus: 200, body: 'nonsense'),
485+
errorMessage: 'Error loading server data. Will retry'));
477486
});
478487
});
479488

0 commit comments

Comments
 (0)