Skip to content

Commit f6ece61

Browse files
committed
wip; inspect the cause
Signed-off-by: Zixuan James Li <[email protected]>
1 parent c112a3e commit f6ece61

File tree

2 files changed

+45
-10
lines changed

2 files changed

+45
-10
lines changed

lib/model/store.dart

+25-4
Original file line numberDiff line numberDiff line change
@@ -777,6 +777,15 @@ class UpdateMachine {
777777
int accumulatedTransientFailureCount = 0;
778778
const transientFailureCountNotifyThreshold = 5;
779779

780+
/// This only reports transient errors after reaching
781+
/// a pre-defined threshold of retries.
782+
void maybeReportTransientError(String? message, {String? details}) {
783+
accumulatedTransientFailureCount++;
784+
if (accumulatedTransientFailureCount > transientFailureCountNotifyThreshold) {
785+
reportErrorToUserBriefly(message, details: details);
786+
}
787+
}
788+
780789
while (true) {
781790
if (_debugLoopSignal != null) {
782791
await _debugLoopSignal!.future;
@@ -802,12 +811,24 @@ class UpdateMachine {
802811
debugLog('… Event queue replaced.');
803812
return;
804813

805-
case Server5xxException() || NetworkException():
814+
case Server5xxException():
815+
assert(debugLog('Transient error polling event queue for $store: $e\n'
816+
'Backing off, then will retry…'));
817+
maybeReportTransientError(
818+
localizations.errorConnectingToServerShort,
819+
details: localizations.errorConnectingToServerDetails(
820+
serverUrl, e.toString()));
821+
await (backoffMachine ??= BackoffMachine()).wait();
822+
assert(debugLog('… Backoff wait complete, retrying poll.'));
823+
continue;
824+
825+
case NetworkException():
806826
assert(debugLog('Transient error polling event queue for $store: $e\n'
807827
'Backing off, then will retry…'));
808-
accumulatedTransientFailureCount++;
809-
if (accumulatedTransientFailureCount > transientFailureCountNotifyThreshold) {
810-
reportErrorToUserBriefly(
828+
// Heuristic check to skip reporting boring errors.
829+
// This error occurs when the client has no network connection.
830+
if (!e.message.startsWith('Failed host lookup')) {
831+
maybeReportTransientError(
811832
localizations.errorConnectingToServerShort,
812833
details: localizations.errorConnectingToServerDetails(
813834
serverUrl, e.toString()));

test/model/store_test.dart

+20-6
Original file line numberDiff line numberDiff line change
@@ -471,8 +471,17 @@ void main() {
471471
}));
472472

473473
group('retries on errors', () {
474+
/// Check if [UpdateMachine.poll] retries as expected when there are
475+
/// errors.
476+
///
477+
/// If `expectedFailureCountNotifyThreshold` is null,
478+
/// no user-facing error message should be shown .
479+
///
480+
/// If `expectedFailureCountNotifyThreshold` is non-null,
481+
/// the first user-facing error message should be shown when
482+
/// the `expectedFailureCountNotifyThreshold`'th polling failure occurs.
474483
void checkRetry(void Function() prepareError, {
475-
int expectedFailureCountNotifyThreshold = 0,
484+
int? expectedFailureCountNotifyThreshold = 0,
476485
}) {
477486
reportErrorToUserBriefly = logAndReportErrorToUserBriefly;
478487
addTearDown(() => reportErrorToUserBriefly = defaultReportErrorToUserBriefly);
@@ -489,7 +498,8 @@ void main() {
489498

490499
// Need to add 1 to the upperbound for that one additional request
491500
// to trigger error reporting.
492-
for (int i = 0; i < expectedFailureCountNotifyThreshold + 1; i++) {
501+
final numRetries = expectedFailureCountNotifyThreshold ?? 0 + 1;
502+
for (int i = 0; i < numRetries; i++) {
493503
// Make the request, inducing an error in it.
494504
prepareError();
495505
if (i > 0) {
@@ -499,15 +509,19 @@ void main() {
499509
updateMachine.debugAdvanceLoop();
500510
check(lastReportedError).isNull();
501511
async.elapse(Duration.zero);
502-
if (i < expectedFailureCountNotifyThreshold) {
512+
513+
checkLastRequest(lastEventId: 1);
514+
check(store).isLoading.isTrue();
515+
if (expectedFailureCountNotifyThreshold == null
516+
|| i < expectedFailureCountNotifyThreshold) {
503517
// The error message should not appear until the `updateMachine`
504518
// has retried the given number of times.
505519
check(takeLastReportedError()).isNull();
506-
} else {
520+
continue;
521+
}
522+
if (i == expectedFailureCountNotifyThreshold) {
507523
check(takeLastReportedError()).isNotNull().contains(expectedErrorMessage);
508524
}
509-
checkLastRequest(lastEventId: 1);
510-
check(store).isLoading.isTrue();
511525
}
512526

513527
// Polling doesn't resume immediately; there's a timer.

0 commit comments

Comments
 (0)