Skip to content

Commit e82ea7e

Browse files
committed
store: Better error message on handleEvent failure
1 parent 2595cb0 commit e82ea7e

File tree

7 files changed

+86
-9
lines changed

7 files changed

+86
-9
lines changed

assets/l10n/app_en.arb

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,19 @@
188188
"error": {"type": "String", "example": "Invalid format"}
189189
}
190190
},
191+
"errorHandlingEventTitle": "Error handling a Zulip event. Retrying connection…",
192+
"@errorHandlingEventTitle": {
193+
"description": "Error title on failing to handle a Zulip server event."
194+
},
195+
"errorHandlingEventDetails": "Error handling a Zulip event from {serverUrl}; will retry.\n\nError: {error}\n\nEvent: {event}",
196+
"@errorHandlingEventDetails": {
197+
"description": "Error details on failing to handle a Zulip server event.",
198+
"placeholders": {
199+
"serverUrl": {"type": "String", "example": "https://chat.example.com"},
200+
"error": {"type": "String", "example": "Unexpected null value"},
201+
"event": {"type": "String", "example": "UpdateMessageEvent(id: 123, messageIds: [2345, 3456], newTopic: 'dinner')"}
202+
}
203+
},
191204
"errorSharingFailed": "Sharing failed",
192205
"@errorSharingFailed": {
193206
"description": "Error message when sharing a message failed."

lib/generated/l10n/zulip_localizations.dart

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,18 @@ abstract class ZulipLocalizations {
343343
/// **'Error connecting to Zulip at {serverUrl}. Will retry:\n\n{error}'**
344344
String errorConnectingToServerDetails(String serverUrl, String error);
345345

346+
/// Error title on failing to handle a Zulip server event.
347+
///
348+
/// In en, this message translates to:
349+
/// **'Error handling a Zulip event. Retrying connection…'**
350+
String get errorHandlingEventTitle;
351+
352+
/// Error details on failing to handle a Zulip server event.
353+
///
354+
/// In en, this message translates to:
355+
/// **'Error handling a Zulip event from {serverUrl}; will retry.\n\nError: {error}\n\nEvent: {event}'**
356+
String errorHandlingEventDetails(String serverUrl, String error, String event);
357+
346358
/// Error message when sharing a message failed.
347359
///
348360
/// In en, this message translates to:

lib/generated/l10n/zulip_localizations_ar.dart

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,14 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
157157
return 'Error connecting to Zulip at $serverUrl. Will retry:\n\n$error';
158158
}
159159

160+
@override
161+
String get errorHandlingEventTitle => 'Error handling a Zulip event. Retrying connection…';
162+
163+
@override
164+
String errorHandlingEventDetails(String serverUrl, String error, String event) {
165+
return 'Error handling a Zulip event from $serverUrl; will retry.\n\nError: $error\n\nEvent: $event';
166+
}
167+
160168
@override
161169
String get errorSharingFailed => 'Sharing failed';
162170

lib/generated/l10n/zulip_localizations_en.dart

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,14 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
157157
return 'Error connecting to Zulip at $serverUrl. Will retry:\n\n$error';
158158
}
159159

160+
@override
161+
String get errorHandlingEventTitle => 'Error handling a Zulip event. Retrying connection…';
162+
163+
@override
164+
String errorHandlingEventDetails(String serverUrl, String error, String event) {
165+
return 'Error handling a Zulip event from $serverUrl; will retry.\n\nError: $error\n\nEvent: $event';
166+
}
167+
160168
@override
161169
String get errorSharingFailed => 'Sharing failed';
162170

lib/generated/l10n/zulip_localizations_ja.dart

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,14 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
157157
return 'Error connecting to Zulip at $serverUrl. Will retry:\n\n$error';
158158
}
159159

160+
@override
161+
String get errorHandlingEventTitle => 'Error handling a Zulip event. Retrying connection…';
162+
163+
@override
164+
String errorHandlingEventDetails(String serverUrl, String error, String event) {
165+
return 'Error handling a Zulip event from $serverUrl; will retry.\n\nError: $error\n\nEvent: $event';
166+
}
167+
160168
@override
161169
String get errorSharingFailed => 'Sharing failed';
162170

lib/model/store.dart

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,8 +1117,14 @@ class UpdateMachine {
11171117

11181118
final events = result.events;
11191119
for (final event in events) {
1120-
await store.handleEvent(event);
1121-
if (_disposed) return;
1120+
try {
1121+
await store.handleEvent(event);
1122+
if (_disposed) return;
1123+
} catch (e, stackTrace) {
1124+
if (_disposed) return;
1125+
Error.throwWithStackTrace(
1126+
_EventHandlingException(cause: e, event: event), stackTrace);
1127+
}
11221128
}
11231129
if (events.isNotEmpty) {
11241130
lastEventId = events.last.id;
@@ -1142,10 +1148,14 @@ class UpdateMachine {
11421148
// The old event queue is gone, so we need a new one. This is normal.
11431149
isUnexpected = false;
11441150

1145-
default:
1146-
assert(debugLog('BUG: Unexpected error in event polling: $e\n' // TODO(log)
1151+
case _EventHandlingException(:final cause, :final event):
1152+
assert(debugLog('BUG: Error handling an event: $cause\n' // TODO(log)
1153+
' event: $event\n'
11471154
'Replacing event queue…'));
1148-
_reportToUserErrorConnectingToServer(e);
1155+
reportErrorToUserBriefly(
1156+
GlobalLocalizations.zulipLocalizations.errorHandlingEventTitle,
1157+
details: GlobalLocalizations.zulipLocalizations.errorHandlingEventDetails(
1158+
store.realmUrl.toString(), cause.toString(), event.toString()));
11491159
// We can't just continue with the next event, because our state
11501160
// may be garbled due to failing to apply this one (and worse,
11511161
// any invariants that were left in a broken state from where
@@ -1154,6 +1164,14 @@ class UpdateMachine {
11541164
// the *change* in state represented by the event, and when we get the
11551165
// new state in a fresh InitialSnapshot we'll handle that just fine.
11561166
isUnexpected = true;
1167+
1168+
default:
1169+
assert(debugLog('BUG: Unexpected error in event polling: $e\n' // TODO(log)
1170+
'Replacing event queue…'));
1171+
_reportToUserErrorConnectingToServer(e);
1172+
// Similar story to the _EventHandlingException case;
1173+
// separate only so that that other case can print more context.
1174+
isUnexpected = true;
11571175
}
11581176

11591177
if (isUnexpected) {
@@ -1257,3 +1275,10 @@ class UpdateMachine {
12571275
@override
12581276
String toString() => '${objectRuntimeType(this, 'UpdateMachine')}#${shortHash(this)}';
12591277
}
1278+
1279+
class _EventHandlingException implements Exception {
1280+
final Object cause;
1281+
final Event event;
1282+
1283+
_EventHandlingException({required this.cause, required this.event});
1284+
}

test/model/store_test.dart

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -982,10 +982,13 @@ void main() {
982982
checkNotReported(prepareExpiredEventQueue);
983983
});
984984

985-
test('report handleEvent error', () {
986-
checkReported(prepareHandleEventError).startsWith(
987-
"Error connecting to Zulip. Retrying…\n"
988-
"Error connecting to Zulip at");
985+
test('nicely report handleEvent error', () {
986+
checkReported(prepareHandleEventError).matchesPattern(RegExp(
987+
r"Error handling a Zulip event\. Retrying connection…\n"
988+
r"Error handling a Zulip event from \S+; will retry\.\n"
989+
r"\n"
990+
r"Error: .*channel\.dart.. Failed assertion.*"
991+
));
989992
});
990993
});
991994
});

0 commit comments

Comments
 (0)