Skip to content

Commit e5716c7

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

File tree

3 files changed

+44
-17
lines changed

3 files changed

+44
-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

+29-16
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,7 @@ void main() {
430430

431431
void checkRetry(String description, void Function() prepareError, {
432432
String? errorMessage,
433+
int failureCountNofityThreshold = 0,
433434
}) {
434435
test(description, () {
435436
awaitFakeAsync((async) async {
@@ -438,18 +439,26 @@ void main() {
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(takeLastReportedError()).isNull();
448-
} else {
449-
check(takeLastReportedError()).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+
final error = debugTakeLastReportedError();
456+
print(error);
457+
check(error).isNotNull().contains(errorMessage);
458+
}
459+
checkLastRequest(lastEventId: 1);
460+
check(store).isLoading.isTrue();
450461
}
451-
checkLastRequest(lastEventId: 1);
452-
check(store).isLoading.isTrue();
453462

454463
// Polling doesn't resume immediately; there's a timer.
455464
check(async.pendingTimers).length.equals(1);
@@ -471,11 +480,15 @@ void main() {
471480
}
472481

473482
group('retries', () {
474-
checkRetry('retries on Server5xxException',
475-
() => connection.prepare(httpStatus: 500, body: 'splat'));
476-
477-
checkRetry('retries on NetworkException',
478-
() => connection.prepare(exception: Exception("failed")));
483+
checkRetry('too many retries on Server5xxException',
484+
() => connection.prepare(httpStatus: 500, body: 'splat'),
485+
errorMessage: 'Failed to reach server. Will retry',
486+
failureCountNofityThreshold: 5);
487+
488+
checkRetry('too many retries on NetworkException',
489+
() => connection.prepare(exception: Exception("failed")),
490+
errorMessage: 'Failed to reach server. Will retry',
491+
failureCountNofityThreshold: 5);
479492

480493
checkRetry('retries on ZulipApiException',
481494
() => connection.prepare(httpStatus: 400, json: {

0 commit comments

Comments
 (0)