Skip to content

Commit d79c8eb

Browse files
committed
store: Report if transient polling errors persist.
Signed-off-by: Zixuan James Li <[email protected]>
1 parent fa035cd commit d79c8eb

File tree

3 files changed

+39
-14
lines changed

3 files changed

+39
-14
lines changed

assets/l10n/app_en.arb

+7
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,13 @@
167167
"error": {"type": "String", "example": "Invalid format"}
168168
}
169169
},
170+
"errorConnectingToServer": "Failed to reach server. Will retry:\n\n{error}",
171+
"@errorConnectingToServer": {
172+
"description": "Dialog message when the the server cannot be reached",
173+
"placeholders": {
174+
"error": {"type": "String", "example": "Network request failed: HTTP status 500"}
175+
}
176+
},
170177
"errorSharingFailed": "Sharing failed",
171178
"@errorSharingFailed": {
172179
"description": "Error message when sharing a message failed."

lib/model/store.dart

+8-1
Original file line numberDiff line numberDiff line change
@@ -767,6 +767,8 @@ class UpdateMachine {
767767

768768
void poll() async {
769769
BackoffMachine? backoffMachine;
770+
int accumulatedTransientFailureCount = 0;
771+
const transientFailureCountNotifyThreshold = 5;
770772

771773
while (true) {
772774
if (_debugLoopSignal != null) {
@@ -795,8 +797,12 @@ class UpdateMachine {
795797
case Server5xxException() || NetworkException():
796798
assert(debugLog('Transient error polling event queue for $store: $e\n'
797799
'Backing off, then will retry…'));
798-
// TODO tell user if transient polling errors persist
799800
// TODO reset to short backoff eventually
801+
accumulatedTransientFailureCount++;
802+
if (accumulatedTransientFailureCount > transientFailureCountNotifyThreshold) {
803+
reportErrorToUserInDialog(
804+
localizations.errorConnectingToServer(e.toString()));
805+
}
800806
await (backoffMachine ??= BackoffMachine()).wait();
801807
assert(debugLog('… Backoff wait complete, retrying poll.'));
802808
continue;
@@ -830,6 +836,7 @@ class UpdateMachine {
830836
// and failures, the successes themselves should space out the requests.
831837
backoffMachine = null;
832838
store.isLoading = false;
839+
accumulatedTransientFailureCount = 0;
833840

834841
final events = result.events;
835842
for (final event in events) {

test/model/store_test.dart

+24-13
Original file line numberDiff line numberDiff line change
@@ -431,25 +431,32 @@ void main() {
431431
group('retry on errors', () {
432432
void checkRetry(void Function() prepareError, {
433433
String? errorMessage,
434+
int failureCountNofityThreshold = 0,
434435
}) {
435436
awaitFakeAsync((async) async {
436437
await prepareStore(lastEventId: 1);
437438
updateMachine.debugPauseLoop();
438439
updateMachine.poll();
439440
check(async.pendingTimers).length.equals(0);
440441

441-
// Make the request, inducing an error in it.
442-
prepareError();
443-
updateMachine.debugAdvanceLoop();
444-
check(debugLastReportedError).isNull();
445-
async.elapse(Duration.zero);
446-
if (errorMessage == null) {
447-
check(debugTakeLastReportedError()).isNull();
448-
} else {
449-
check(debugTakeLastReportedError()).isNotNull().contains(errorMessage);
442+
for (int i = 0; i < failureCountNofityThreshold + 1; i++) {
443+
// Make the request, inducing an error in it.
444+
prepareError();
445+
if (i > 0) {
446+
// End polling backoff from the previous iteration.
447+
async.flushTimers();
448+
}
449+
updateMachine.debugAdvanceLoop();
450+
check(debugLastReportedError).isNull();
451+
async.elapse(Duration.zero);
452+
if (i < failureCountNofityThreshold || errorMessage == null) {
453+
check(debugTakeLastReportedError()).isNull();
454+
} else {
455+
check(debugTakeLastReportedError()).isNotNull().contains(errorMessage);
456+
}
457+
checkLastRequest(lastEventId: 1);
458+
check(store).isLoading.isTrue();
450459
}
451-
checkLastRequest(lastEventId: 1);
452-
check(store).isLoading.isTrue();
453460

454461
// Polling doesn't resume immediately; there's a timer.
455462
check(async.pendingTimers).length.equals(1);
@@ -470,11 +477,15 @@ void main() {
470477
}
471478

472479
test('Server5xxException', () {
473-
checkRetry(() => connection.prepare(httpStatus: 500, body: 'splat'));
480+
checkRetry(() => connection.prepare(httpStatus: 500, body: 'splat'),
481+
errorMessage: 'Failed to reach server. Will retry',
482+
failureCountNofityThreshold: 5);
474483
});
475484

476485
test('NetworkException', () {
477-
checkRetry(() => connection.prepare(exception: Exception("failed")));
486+
checkRetry(() => connection.prepare(exception: Exception("failed")),
487+
errorMessage: 'Failed to reach server. Will retry',
488+
failureCountNofityThreshold: 5);
478489
});
479490

480491
test('ZulipApiException', () {

0 commit comments

Comments
 (0)