Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testing branch for #1340 #1

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions assets/l10n/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,8 @@
"url": {"type": "String", "example": "http://example.com/"}
}
},
"errorLoginCouldNotConnectTitle": "Could not connect",
"@errorLoginCouldNotConnectTitle": {
"errorCouldNotConnectTitle": "Could not connect",
"@errorCouldNotConnectTitle": {
"description": "Error title when the app could not connect to the server."
},
"errorMessageDoesNotSeemToExist": "That message does not seem to exist.",
Expand Down Expand Up @@ -515,6 +515,13 @@
"@topicValidationErrorMandatoryButEmpty": {
"description": "Topic validation error when topic is required but was empty."
},
"errorInvalidApiKeyMessage": "Your account at {url} could not be authenticated. Please try logging in again or use another account.",
"@errorInvalidApiKeyMessage": {
"description": "Error message in the dialog for invalid API key.",
"placeholders": {
"url": {"type": "String", "example": "http://chat.example.com/"}
}
},
"errorInvalidResponse": "The server sent an invalid response",
"@errorInvalidResponse": {
"description": "Error message when an API call returned an invalid response."
Expand Down
8 changes: 7 additions & 1 deletion lib/generated/l10n/zulip_localizations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ abstract class ZulipLocalizations {
///
/// In en, this message translates to:
/// **'Could not connect'**
String get errorLoginCouldNotConnectTitle;
String get errorCouldNotConnectTitle;

/// Error message when loading a message that does not exist.
///
Expand Down Expand Up @@ -795,6 +795,12 @@ abstract class ZulipLocalizations {
/// **'Topics are required in this organization.'**
String get topicValidationErrorMandatoryButEmpty;

/// Error message in the dialog for invalid API key.
///
/// In en, this message translates to:
/// **'Your account at {url} could not be authenticated. Please try logging in again or use another account.'**
String errorInvalidApiKeyMessage(String url);

/// Error message when an API call returned an invalid response.
///
/// In en, this message translates to:
Expand Down
7 changes: 6 additions & 1 deletion lib/generated/l10n/zulip_localizations_ar.dart
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
}

@override
String get errorLoginCouldNotConnectTitle => 'Could not connect';
String get errorCouldNotConnectTitle => 'Could not connect';

@override
String get errorMessageDoesNotSeemToExist => 'That message does not seem to exist.';
Expand Down Expand Up @@ -399,6 +399,11 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
@override
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';

@override
String errorInvalidApiKeyMessage(String url) {
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
}

@override
String get errorInvalidResponse => 'The server sent an invalid response';

Expand Down
7 changes: 6 additions & 1 deletion lib/generated/l10n/zulip_localizations_en.dart
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
}

@override
String get errorLoginCouldNotConnectTitle => 'Could not connect';
String get errorCouldNotConnectTitle => 'Could not connect';

@override
String get errorMessageDoesNotSeemToExist => 'That message does not seem to exist.';
Expand Down Expand Up @@ -399,6 +399,11 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
@override
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';

@override
String errorInvalidApiKeyMessage(String url) {
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
}

@override
String get errorInvalidResponse => 'The server sent an invalid response';

Expand Down
7 changes: 6 additions & 1 deletion lib/generated/l10n/zulip_localizations_ja.dart
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
}

@override
String get errorLoginCouldNotConnectTitle => 'Could not connect';
String get errorCouldNotConnectTitle => 'Could not connect';

@override
String get errorMessageDoesNotSeemToExist => 'That message does not seem to exist.';
Expand Down Expand Up @@ -399,6 +399,11 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
@override
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';

@override
String errorInvalidApiKeyMessage(String url) {
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
}

@override
String get errorInvalidResponse => 'The server sent an invalid response';

Expand Down
7 changes: 6 additions & 1 deletion lib/generated/l10n/zulip_localizations_nb.dart
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class ZulipLocalizationsNb extends ZulipLocalizations {
}

@override
String get errorLoginCouldNotConnectTitle => 'Could not connect';
String get errorCouldNotConnectTitle => 'Could not connect';

@override
String get errorMessageDoesNotSeemToExist => 'That message does not seem to exist.';
Expand Down Expand Up @@ -399,6 +399,11 @@ class ZulipLocalizationsNb extends ZulipLocalizations {
@override
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';

@override
String errorInvalidApiKeyMessage(String url) {
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
}

@override
String get errorInvalidResponse => 'The server sent an invalid response';

Expand Down
7 changes: 6 additions & 1 deletion lib/generated/l10n/zulip_localizations_pl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class ZulipLocalizationsPl extends ZulipLocalizations {
}

@override
String get errorLoginCouldNotConnectTitle => 'Nie można połączyć';
String get errorCouldNotConnectTitle => 'Could not connect';

@override
String get errorMessageDoesNotSeemToExist => 'Taka wiadomość raczej nie istnieje.';
Expand Down Expand Up @@ -399,6 +399,11 @@ class ZulipLocalizationsPl extends ZulipLocalizations {
@override
String get topicValidationErrorMandatoryButEmpty => 'Wątki są wymagane przez tę organizację.';

@override
String errorInvalidApiKeyMessage(String url) {
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
}

@override
String get errorInvalidResponse => 'Nieprawidłowa odpowiedź serwera';

Expand Down
7 changes: 6 additions & 1 deletion lib/generated/l10n/zulip_localizations_ru.dart
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class ZulipLocalizationsRu extends ZulipLocalizations {
}

@override
String get errorLoginCouldNotConnectTitle => 'Не удалось подключиться';
String get errorCouldNotConnectTitle => 'Could not connect';

@override
String get errorMessageDoesNotSeemToExist => 'Это сообщение, похоже, отсутствует.';
Expand Down Expand Up @@ -399,6 +399,11 @@ class ZulipLocalizationsRu extends ZulipLocalizations {
@override
String get topicValidationErrorMandatoryButEmpty => 'Темы обязательны в этой организации.';

@override
String errorInvalidApiKeyMessage(String url) {
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
}

@override
String get errorInvalidResponse => 'Получен недопустимый ответ сервера';

Expand Down
7 changes: 6 additions & 1 deletion lib/generated/l10n/zulip_localizations_sk.dart
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class ZulipLocalizationsSk extends ZulipLocalizations {
}

@override
String get errorLoginCouldNotConnectTitle => 'Nepodarilo sa pripojiť';
String get errorCouldNotConnectTitle => 'Could not connect';

@override
String get errorMessageDoesNotSeemToExist => 'Správa zrejme neexistuje.';
Expand Down Expand Up @@ -399,6 +399,11 @@ class ZulipLocalizationsSk extends ZulipLocalizations {
@override
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';

@override
String errorInvalidApiKeyMessage(String url) {
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
}

@override
String get errorInvalidResponse => 'Server poslal nesprávnu odpoveď';

Expand Down
18 changes: 15 additions & 3 deletions lib/log.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ bool debugLog(String message) {
return true;
}

typedef ReportErrorCallback = void Function(String? message, {String? details});
typedef ReportErrorCancellablyCallback = void Function(String? message, {String? details});
typedef ReportErrorCallback = void Function(String message, {String? details});

/// Show the user an error message, without requiring them to interact with it.
///
Expand All @@ -48,9 +49,20 @@ typedef ReportErrorCallback = void Function(String? message, {String? details});
// This gets set in [ZulipApp]. We need this indirection to keep `lib/log.dart`
// from importing widget code, because the file is a dependency for the rest of
// the app.
ReportErrorCallback reportErrorToUserBriefly = defaultReportErrorToUserBriefly;
ReportErrorCancellablyCallback reportErrorToUserBriefly = reportErrorToConsole;

void defaultReportErrorToUserBriefly(String? message, {String? details}) {
/// Show the user a dismissable error message in a modal popup.
///
/// Typically this shows an [AlertDialog] containing the message.
/// If called before the app's widget tree is ready (see [ZulipApp.ready]),
/// then we give up on showing the message to the user,
/// and just log the message to the console.
// This gets set in [ZulipApp]. We need this indirection to keep `lib/log.dart`
// from importing widget code, because the file is a dependency for the rest of
// the app.
ReportErrorCallback reportErrorToUserModally = reportErrorToConsole;

void reportErrorToConsole(String? message, {String? details}) {
// Error dismissing is a no-op to the default handler.
if (message == null) return;
// If this callback is still in place, then the app's widget tree
Expand Down
36 changes: 36 additions & 0 deletions lib/model/actions.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import 'dart:async';

import '../notifications/receive.dart';
import 'store.dart';

// TODO: Make this a part of GlobalStore
Future<void> logOutAccount(GlobalStore globalStore, int accountId) async {
final account = globalStore.getAccount(accountId);
if (account == null) return; // TODO(log)

// Unawaited, to not block removing the account on this request.
unawaited(unregisterToken(globalStore, accountId));

await globalStore.removeAccount(accountId);
}

Future<void> unregisterToken(GlobalStore globalStore, int accountId) async {
final account = globalStore.getAccount(accountId);
if (account == null) return; // TODO(log)

// TODO(#322) use actual acked push token; until #322, this is just null.
final token = account.ackedPushToken
// Try the current token as a fallback; maybe the server has registered
// it and we just haven't recorded that fact in the client.
?? NotificationService.instance.token.value;
if (token == null) return;

final connection = globalStore.apiConnectionFromAccount(account);
try {
await NotificationService.unregisterToken(connection, token: token);
} catch (e) {
// TODO retry? handle failures?
} finally {
connection.close();
}
}
56 changes: 48 additions & 8 deletions lib/model/store.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import '../api/backoff.dart';
import '../api/route/realm.dart';
import '../log.dart';
import '../notifications/receive.dart';
import 'actions.dart';
import 'autocomplete.dart';
import 'database.dart';
import 'emoji.dart';
Expand Down Expand Up @@ -149,7 +150,29 @@ abstract class GlobalStore extends ChangeNotifier {
/// and/or [perAccountSync].
Future<PerAccountStore> loadPerAccount(int accountId) async {
assert(_accounts.containsKey(accountId));
final store = await doLoadPerAccount(accountId);
final realmUrl = getAccount(accountId)!.realmUrl;
final PerAccountStore store;
try {
store = await doLoadPerAccount(accountId);
} catch (e) {
switch (e) {
case HttpException(httpStatus: 401):
// The API key is invalid and the store can never be loaded
// unless the user retries manually.
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
reportErrorToUserModally(
zulipLocalizations.errorCouldNotConnectTitle,
details: zulipLocalizations.errorInvalidApiKeyMessage(
realmUrl.toString()));
// The account could be logged out from the choose-account page.
if (_accounts.containsKey(accountId)) {
await logOutAccount(this, accountId);
}
throw AccountNotFoundException();
default:
rethrow;
}
}
if (!_accounts.containsKey(accountId)) {
// [removeAccount] was called during [doLoadPerAccount].
store.dispose();
Expand Down Expand Up @@ -913,12 +936,19 @@ class UpdateMachine {
try {
return await registerQueue(connection);
} catch (e, s) {
assert(debugLog('Error fetching initial snapshot: $e'));
// Print stack trace in its own log entry; log entries are truncated
// at 1 kiB (at least on Android), and stack can be longer than that.
assert(debugLog('Stack:\n$s'));
// TODO(#890): tell user if initial-fetch errors persist, or look non-transient
switch (e) {
case HttpException(httpStatus: 401):
// We cannot recover from this error through retrying.
// Leave it to [GlobalStore.loadPerAccount].
rethrow;
default:
assert(debugLog('Error fetching initial snapshot: $e'));
// Print stack trace in its own log entry; log entries are truncated
// at 1 kiB (at least on Android), and stack can be longer than that.
assert(debugLog('Stack:\n$s'));
}
assert(debugLog('Backing off, then will retry…'));
// TODO tell user if initial-fetch errors persist, or look non-transient
await (backoffMachine ??= BackoffMachine()).wait();
assert(debugLog('… Backoff wait complete, retrying initial fetch.'));
}
Expand Down Expand Up @@ -1177,6 +1207,7 @@ class UpdateMachine {
store.isLoading = true;

bool isUnexpected;
// TODO(#1054): handle auth failure
switch (error) {
case ZulipApiException(code: 'BAD_EVENT_QUEUE_ID'):
assert(debugLog('Lost event queue for $store. Replacing…'));
Expand Down Expand Up @@ -1218,8 +1249,17 @@ class UpdateMachine {
if (_disposed) return;
}

await store._globalStore._reloadPerAccount(store.accountId);
assert(_disposed);
try {
await store._globalStore._reloadPerAccount(store.accountId);
} on AccountNotFoundException {
// The account was logged out while we retry to replace the store,
// but that's OK as long as other code will be removing it from the UI
// (usually by using [routeToRemoveOnLogout]).
assert(debugLog('… Event queue not replaced; account logged out.'));
return;
} finally {
assert(_disposed);
}
assert(debugLog('… Event queue replaced.'));
}

Expand Down
Loading
Loading