Skip to content

Commit ae7a0e4

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

File tree

3 files changed

+42
-17
lines changed

3 files changed

+42
-17
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

+27-16
Original file line numberDiff line numberDiff line change
@@ -430,25 +430,32 @@ void main() {
430430

431431
void checkRetry(void Function() prepareError, {
432432
String? errorMessage,
433+
int failureCountNofityThreshold = 0,
433434
}) {
434435
awaitFakeAsync((async) async {
435436
await prepareStore(lastEventId: 1);
436437
updateMachine.debugPauseLoop();
437438
updateMachine.poll();
438439
check(async.pendingTimers).length.equals(0);
439440

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

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

471478
group('retries', () {
472-
test('retries on Server5xxException', () =>
473-
checkRetry(() => connection.prepare(httpStatus: 500, body: 'splat')));
474-
475-
test('retries on NetworkException', () =>
476-
checkRetry(() => connection.prepare(exception: Exception("failed"))));
479+
test('too many retries on Server5xxException', () =>
480+
checkRetry(() => connection.prepare(httpStatus: 500, body: 'splat'),
481+
errorMessage: 'Failed to reach server. Will retry',
482+
failureCountNofityThreshold: 5));
483+
484+
test('too many retries on NetworkException', () =>
485+
checkRetry(() => connection.prepare(exception: Exception("failed")),
486+
errorMessage: 'Failed to reach server. Will retry',
487+
failureCountNofityThreshold: 5));
477488

478489
test('retries on ZulipApiException', () =>
479490
checkRetry(() => connection.prepare(httpStatus: 400, json: {

0 commit comments

Comments
 (0)