core: Add translations for exceptions related to api requests

sirpengi · sirpengi · commit 3e20a176c7f6 · 2023-09-26T16:45:57.000+01:00
Also hook up some other test suites that require localization
contexts for tests to pass given this change.
diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb
@@ -15,6 +15,10 @@
   "@aboutPageTapToView": {
     "description": "Item subtitle in About Zulip page to navigate to Licenses page"
   },
+  "apiConnectionNetworkRequestFailed": "Network request failed",
+  "@apiConnectionNetworkRequestFailed": {
+    "description": "Message for error dialog when a network request fails."
+  },
   "chooseAccountPageTitle": "Choose account",
   "@chooseAccountPageTitle": {
     "description": "Title for ChooseAccountPage"
@@ -49,6 +53,26 @@
       }
     }
   },
+  "serverExceptionMalformedResponse": "Server gave malformed response; HTTP status {httpStatus}",
+  "@serverExceptionMalformedResponse": {
+    "description": "Message for error dialog when an API call fails because we could not parse it.",
+    "placeholders": {
+      "httpStatus": {
+        "type": "int",
+        "example": "500"
+      }
+    }
+  },
+  "serverExceptionRequestFailed": "Network request failed: HTTP status {httpStatus}",
+  "@serverExceptionRequestFailed": {
+    "description": "Message for error dialog when an API call fails.",
+    "placeholders": {
+      "httpStatus": {
+        "type": "int",
+        "example": "500"
+      }
+    }
+  },
   "userRoleOwner": "Owner",
   "@userRoleOwner": {
     "description": "Label for UserRole.owner"
diff --git a/lib/api/core.dart b/lib/api/core.dart
@@ -1,6 +1,7 @@
 import 'dart:convert';
 import 'dart:io';
 
+import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
 import 'package:http/http.dart' as http;
 
 import '../log.dart';
@@ -84,15 +85,20 @@ class ApiConnection {
     try {
       response = await _client.send(request);
     } catch (e) {
-      final String message;
+      final String Function(ZulipLocalizations) message;
       if (e is http.ClientException) {
-        message = e.message;
+        message = (ZulipLocalizations zulipLocalizations) => e.message;
       } else if (e is TlsException) {
-        message = e.message;
+        message = (ZulipLocalizations zulipLocalizations) => e.message;
       } else {
-        message = 'Network request failed';
+        message = (ZulipLocalizations zulipLocalizations) {
+          return zulipLocalizations.apiConnectionNetworkRequestFailed;
+        };
       }
-      throw NetworkException(routeName: routeName, cause: e, message: message);
+      throw NetworkException(
+        routeName: routeName,
+        cause: e,
+        message: message);
     }
 
     final int httpStatus = response.statusCode;
@@ -165,14 +171,17 @@ ApiRequestException _makeApiException(String routeName, int httpStatus, Map<Stri
     if (json != null && json['result'] == 'error'
         && json['code'] is String? && json['msg'] is String) {
       json.remove('result');
+      final serverMessage = json.remove('msg');
       return ZulipApiException( // TODO(log): systematically log these
         routeName: routeName,
         httpStatus: httpStatus,
         // When `code` is missing, we fall back to `BAD_REQUEST`,
         // the same value the server uses when nobody's made it more specific.
         // TODO(server): `code` should always be present.  Get the "Invalid API key" case fixed.
         code: json.remove('code') ?? 'BAD_REQUEST',
-        message: json.remove('msg'),
+        message: (ZulipLocalizations zulipLocalizations) {
+          return serverMessage;
+        },
         data: json,
       );
     }
diff --git a/lib/api/exception.dart b/lib/api/exception.dart
@@ -1,4 +1,6 @@
 
+import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
+
 /// Some kind of error from a Zulip API network request.
 sealed class ApiRequestException implements Exception {
   /// The name of the Zulip API route for the request.
@@ -15,9 +17,10 @@ sealed class ApiRequestException implements Exception {
   ///
   /// For [ZulipApiException] this is supplied by the server as the `message`
   /// property in the JSON response, and is translated into the user's language.
-  final String message;
+  final String Function(ZulipLocalizations) message;
 
   ApiRequestException({required this.routeName, required this.message});
+
 }
 
 /// An error returned through the Zulip server API.
@@ -89,7 +92,10 @@ class Server5xxException extends ServerException {
     required super.httpStatus,
     required super.data,
   }) : assert(500 <= httpStatus && httpStatus <= 599),
-       super(message: 'Network request failed: HTTP status $httpStatus'); // TODO(i18n)
+       super(
+         message: (ZulipLocalizations zulipLocalizations) {
+           return zulipLocalizations.serverExceptionRequestFailed(httpStatus);
+         });
 }
 
 /// An error where the server's response doesn't match the Zulip API.
@@ -110,5 +116,8 @@ class MalformedServerResponseException extends ServerException {
     required super.routeName,
     required super.httpStatus,
     required super.data,
-  }) : super(message: 'Server gave malformed response; HTTP status $httpStatus'); // TODO(i18n)
+  }) : super(
+         message: (ZulipLocalizations zulipLocalizations) {
+           return zulipLocalizations.serverExceptionMalformedResponse(httpStatus);
+         });
 }
diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart
@@ -1,5 +1,6 @@
 import 'package:flutter/material.dart';
 import 'package:flutter/services.dart';
+import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
 import 'package:share_plus/share_plus.dart';
 
 import '../api/exception.dart';
@@ -104,6 +105,7 @@ Future<String?> fetchRawContentWithFeedback({
     // - If request(s) take(s) a long time, show snackbar with cancel
     //   button, like "Still working on quote-and-reply…".
     //   On final failure or success, auto-dismiss the snackbar.
+    final zulipLocalizations = ZulipLocalizations.of(context);
     try {
       fetchedMessage = await getMessageCompat(PerAccountStoreWidget.of(context).connection,
         messageId: messageId,
@@ -115,7 +117,7 @@ Future<String?> fetchRawContentWithFeedback({
     } catch (e) {
       switch (e) {
         case ZulipApiException():
-          errorMessage = e.message;
+          errorMessage = e.message(zulipLocalizations);
         // TODO specific messages for common errors, like network errors
         //   (support with reusable code)
         default:
diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart
@@ -5,6 +5,7 @@ import 'package:flutter/services.dart';
 import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
 import 'package:image_picker/image_picker.dart';
 
+import '../api/exception.dart';
 import '../api/model/model.dart';
 import '../api/route/messages.dart';
 import '../model/compose.dart';
@@ -474,8 +475,12 @@ Future<void> _uploadFiles({
       if (!context.mounted) return;
       // TODO(#37): Specifically handle `413 Payload Too Large`
       // TODO(#37): On API errors, quote `msg` from server, with "The server said:"
+      final zulipLocalizations = ZulipLocalizations.of(context);
+      final String message = (e is ApiRequestException)
+        ? e.message(zulipLocalizations)
+        : e.toString();
       showErrorDialog(context: context,
-        title: 'Failed to upload file: $filename', message: e.toString());
+        title: 'Failed to upload file: $filename', message: message);
     } finally {
       contentController.registerUploadEnd(tag, url);
     }
@@ -535,6 +540,7 @@ Future<Iterable<_File>> _getFilePickerFiles(BuildContext context, FileType type)
       .pickFiles(allowMultiple: true, withReadStream: true, type: type);
   } catch (e) {
     if (!context.mounted) return [];
+    final zulipLocalizations = ZulipLocalizations.of(context);
     if (e is PlatformException && e.code == 'read_external_storage_denied') {
       // Observed on Android. If Android's error message tells us whether the
       // user has checked "Don't ask again", it seems the library doesn't pass
@@ -550,8 +556,13 @@ Future<Iterable<_File>> _getFilePickerFiles(BuildContext context, FileType type)
           AppSettings.openAppSettings();
         });
     } else {
-      // TODO(i18n)
-      showErrorDialog(context: context, title: 'Error', message: e.toString());
+      final String message = (e is ApiRequestException)
+        ? e.message(zulipLocalizations)
+        : e.toString();
+      showErrorDialog(
+        context: context,
+        title: 'Error',
+        message: message);
     }
     return [];
   }
@@ -629,8 +640,13 @@ class _AttachFromCameraButton extends _AttachUploadsButton {
             AppSettings.openAppSettings();
           });
       } else {
-        // TODO(i18n)
-        showErrorDialog(context: context, title: 'Error', message: e.toString());
+        final String message = (e is ApiRequestException)
+          ? e.message(zulipLocalizations)
+          : e.toString();
+        showErrorDialog(
+          context: context,
+          title: 'Error',
+          message: message);
       }
       return [];
     }
diff --git a/lib/widgets/login.dart b/lib/widgets/login.dart
@@ -1,4 +1,5 @@
 import 'package:flutter/material.dart';
+import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
 
 import '../api/core.dart';
 import '../api/exception.dart';
@@ -289,10 +290,10 @@ class _PasswordLoginPageState extends State<PasswordLoginPage> {
         // TODO(#105) give more helpful feedback. The RN app is
         //   unhelpful here; we should at least recognize invalid auth errors, and
         //   errors for deactivated user or realm (see zulip-mobile#4571).
-        final message = (e is ZulipApiException)
-          ? 'The server said:\n\n${e.message}'
-          : e.message;
-        showErrorDialog(context: context, title: 'Login failed', message: message);
+        final zulipLocalizations = ZulipLocalizations.of(context);
+        showErrorDialog(context: context,
+          title: 'Login failed',
+          message: e.message(zulipLocalizations));
         return;
       }
 
diff --git a/test/api/core_test.dart b/test/api/core_test.dart
@@ -3,6 +3,7 @@ import 'dart:io';
 
 import 'package:checks/checks.dart';
 import 'package:checks/context.dart';
+import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
 import 'package:http/http.dart' as http;
 import 'package:test/scaffolding.dart';
 import 'package:zulip/api/core.dart';
@@ -153,9 +154,13 @@ void main() {
           ..which(condition));
     }
 
-    checkRequest(http.ClientException('Oops'), it()..message.equals('Oops'));
-    checkRequest(const TlsException('Oops'), it()..message.equals('Oops'));
-    checkRequest((foo: 'bar'), it()..message.equals('Network request failed'));
+    final zulipLocalizations = lookupZulipLocalizations(ZulipLocalizations.supportedLocales.first);
+    checkRequest(http.ClientException('Oops'), it()..message(zulipLocalizations).equals('Oops'));
+    checkRequest(const TlsException('Oops'), it()..message(zulipLocalizations).equals('Oops'));
+    final expectedMessage = zulipLocalizations.apiConnectionNetworkRequestFailed;
+    checkRequest((foo: 'bar'), it()
+      ..message(zulipLocalizations)
+      .equals(expectedMessage));
   });
 
   test('API 4xx errors, well formed', () async {
@@ -166,6 +171,7 @@ void main() {
       String message = 'A thing failed',
       Map<String, dynamic> data = const {},
     }) async {
+      final zulipLocalizations = lookupZulipLocalizations(ZulipLocalizations.supportedLocales.first);
       final json = {
         'result': 'error',
         if (code != null) 'code': code,
@@ -178,7 +184,7 @@ void main() {
           ..httpStatus.equals(httpStatus)
           ..code.equals(expectedCode ?? code!)
           ..data.deepEquals(data)
-          ..message.equals(message));
+          ..message(zulipLocalizations).equals(message));
     }
 
     await checkRequest();
diff --git a/test/api/exception_checks.dart b/test/api/exception_checks.dart
@@ -1,9 +1,11 @@
 import 'package:checks/checks.dart';
+import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
 import 'package:zulip/api/exception.dart';
 
 extension ApiRequestExceptionChecks on Subject<ApiRequestException> {
   Subject<String> get routeName => has((e) => e.routeName, 'routeName');
-  Subject<String> get message => has((e) => e.message, 'message');
+  Subject<String> message (ZulipLocalizations zulipLocalizations) =>
+    has((e) => e.message(zulipLocalizations), 'message');
 }
 
 extension ZulipApiExceptionChecks on Subject<ZulipApiException> {
diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart
@@ -1,6 +1,7 @@
 import 'package:checks/checks.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/services.dart';
+import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:zulip/api/model/model.dart';
 import 'package:zulip/api/route/messages.dart';
@@ -48,6 +49,8 @@ Future<void> setupToMessageActionSheet(WidgetTester tester, {
 
   await tester.pumpWidget(
     MaterialApp(
+      localizationsDelegates: ZulipLocalizations.localizationsDelegates,
+      supportedLocales: ZulipLocalizations.supportedLocales,
       home: GlobalStoreWidget(
         child: PerAccountStoreWidget(
           accountId: eg.selfAccount.id,
diff --git a/test/widgets/autocomplete_test.dart b/test/widgets/autocomplete_test.dart
@@ -1,5 +1,6 @@
 import 'package:checks/checks.dart';
 import 'package:flutter/material.dart';
+import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:zulip/api/model/model.dart';
 import 'package:zulip/api/route/messages.dart';
@@ -40,6 +41,8 @@ Future<Finder> setupToComposeInput(WidgetTester tester, {
 
   await tester.pumpWidget(
     MaterialApp(
+      localizationsDelegates: ZulipLocalizations.localizationsDelegates,
+      supportedLocales: ZulipLocalizations.supportedLocales,
       home: GlobalStoreWidget(
         child: PerAccountStoreWidget(
           accountId: eg.selfAccount.id,
diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart
@@ -3,6 +3,7 @@ import 'dart:io';
 import 'package:checks/checks.dart';
 import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
+import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:url_launcher/url_launcher.dart';
 import 'package:zulip/api/core.dart';
@@ -68,6 +69,8 @@ void main() {
       addTearDown(testBinding.reset);
 
       await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp(
+        localizationsDelegates: ZulipLocalizations.localizationsDelegates,
+        supportedLocales: ZulipLocalizations.supportedLocales,
         home: PerAccountStoreWidget(accountId: eg.selfAccount.id,
           child: BlockContentList(
             nodes: parseContent(html).nodes)))));
