Skip to content

Commit 47aae6c

Browse files
committed
store: Ignore boring NetworkExceptions
Usually when a client goes back from sleep or lose network connection, there will be a bunch of errors when polling event queue. This known error will always be a `NetworkException`, so we make a separate case for it. Previously, we may report these errors, and it ended up being spammy. This filters out the more interesting one to report instead. Signed-off-by: Zixuan James Li <[email protected]>
1 parent 3d61fc1 commit 47aae6c

File tree

2 files changed

+42
-2
lines changed

2 files changed

+42
-2
lines changed

lib/model/store.dart

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -817,7 +817,7 @@ class UpdateMachine {
817817
debugLog('… Event queue replaced.');
818818
return;
819819

820-
case Server5xxException() || NetworkException():
820+
case Server5xxException():
821821
assert(debugLog('Transient error polling event queue for $store: $e\n'
822822
'Backing off, then will retry…'));
823823
maybeReportTransientError(
@@ -828,6 +828,27 @@ class UpdateMachine {
828828
assert(debugLog('… Backoff wait complete, retrying poll.'));
829829
continue;
830830

831+
case NetworkException():
832+
assert(debugLog('Transient error polling event queue for $store: $e\n'
833+
'Backing off, then will retry…'));
834+
if (e.cause is! SocketException) {
835+
// Heuristic check to only report interesting errors to the user.
836+
// This currently ignores [SocketException], which typically
837+
// occurs when the client goes back from sleep.
838+
// TODO: Investigate if there are other cases of [SocketException]
839+
// that might turn out to be interesting.
840+
//
841+
// See also:
842+
// * [NetworkException.cause], which is the underlying exception.
843+
maybeReportTransientError(
844+
localizations.errorConnectingToServerShort,
845+
details: localizations.errorConnectingToServerDetails(
846+
serverUrl, e.toString()));
847+
}
848+
await (backoffMachine ??= BackoffMachine()).wait();
849+
assert(debugLog('… Backoff wait complete, retrying poll.'));
850+
continue;
851+
831852
default:
832853
assert(debugLog('Error polling event queue for $store: $e\n'
833854
'Backing off and retrying even though may be hopeless…'));

test/model/store_test.dart

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import 'dart:async';
2+
import 'dart:io';
23

34
import 'package:checks/checks.dart';
45
import 'package:flutter/foundation.dart';
@@ -476,10 +477,14 @@ void main() {
476477
/// Check if [UpdateMachine.poll] retries as expected when there are
477478
/// errors.
478479
///
479-
/// This verifies that the first user-facing error message when the
480+
/// By default, verify that the first user-facing error message when the
480481
/// `numRetries`'th polling failure occurs.
482+
///
483+
/// If `expectNotifyError` is false, verify that there is no user-facing
484+
/// error message regardless of the retries.
481485
void checkRetry(void Function() prepareError, {
482486
required int numRetries,
487+
bool expectNotifyError = true,
483488
}) {
484489
assert(numRetries > 0);
485490
reportErrorToUserBriefly = logAndReportErrorToUserBriefly;
@@ -510,12 +515,19 @@ void main() {
510515
async.flushTimers();
511516
}
512517

518+
if (!expectNotifyError) {
519+
// No matter how many retries there have been, no request should
520+
// result in an error message.
521+
check(takeLastReportedError()).isNull();
522+
continue;
523+
}
513524
if (i < numRetries - 1) {
514525
// The error message should not appear until the `updateMachine`
515526
// has retried the given number of times.
516527
check(takeLastReportedError()).isNull();
517528
continue;
518529
}
530+
assert(expectNotifyError);
519531
// Only expect an error message from the last retry.
520532
assert(i == numRetries - 1);
521533
check(takeLastReportedError()).isNotNull().contains(expectedErrorMessage);
@@ -549,6 +561,13 @@ void main() {
549561
numRetries: UpdateMachine.transientFailureCountNotifyThreshold + 1);
550562
});
551563

564+
test('NetworkException to be ignored', () {
565+
checkRetry(() => connection.prepare(
566+
exception: const SocketException("failed")),
567+
numRetries: UpdateMachine.transientFailureCountNotifyThreshold + 1,
568+
expectNotifyError: false);
569+
});
570+
552571
test('ZulipApiException', () {
553572
checkRetry(() => connection.prepare(httpStatus: 400, json: {
554573
'result': 'error', 'code': 'BAD_REQUEST', 'msg': 'Bad request'}),

0 commit comments

Comments
 (0)