diff --git a/test/api/core_test.dart b/test/api/core_test.dart index b45cdeaebf..297dcfc441 100644 --- a/test/api/core_test.dart +++ b/test/api/core_test.dart @@ -504,7 +504,7 @@ Future tryRequest({ fromJson ??= (((Map x) => x) as T Function(Map)); return FakeApiConnection.with_((connection) { connection.prepare( - exception: exception, httpStatus: httpStatus, json: json, body: body); + httpException: exception, httpStatus: httpStatus, json: json, body: body); return connection.get(kExampleRouteName, fromJson!, 'example/route', {}); }); } diff --git a/test/api/fake_api.dart b/test/api/fake_api.dart index 5e91a55cb2..6953515c6b 100644 --- a/test/api/fake_api.dart +++ b/test/api/fake_api.dart @@ -4,6 +4,7 @@ import 'dart:convert'; import 'package:flutter/foundation.dart'; import 'package:http/http.dart' as http; import 'package:zulip/api/core.dart'; +import 'package:zulip/api/exception.dart'; import 'package:zulip/model/store.dart'; import '../example_data.dart' as eg; @@ -209,28 +210,69 @@ class FakeApiConnection extends ApiConnection { List takeRequests() => client.takeRequests(); - /// Prepare the response for the next request. + /// Prepare the HTTP response for the next request. /// - /// If `exception` is null, the next request will produce an [http.Response] + /// If `httpException` and `apiException` are both null, then + /// the next request will produce an [http.Response] /// with the given `httpStatus`, defaulting to 200. The body of the response /// will be `body` if non-null, or `jsonEncode(json)` if `json` is non-null, /// or else ''. The `body` and `json` parameters must not both be non-null. /// - /// If `exception` is non-null, then `httpStatus`, `body`, and `json` must - /// all be null, and the next request will throw the given exception. + /// If `httpException` is non-null, then `apiException`, + /// `httpStatus`, `body`, and `json` must all be null, and the next request + /// will throw the given exception within the HTTP client layer, + /// causing the API request to throw a [NetworkException] + /// wrapping the given exception. /// - /// In either case, the next request will complete a duration of `delay` + /// If `apiException` is non-null, then `httpException`, + /// `httpStatus`, `body`, and `json` must all be null, and the next request + /// will throw an exception equivalent to the given exception + /// (except [ApiRequestException.routeName], which is ignored). + /// + /// In each case, the next request will complete a duration of `delay` /// after being started. void prepare({ - Object? exception, + Object? httpException, + ZulipApiException? apiException, int? httpStatus, Map? json, String? body, Duration delay = Duration.zero, }) { assert(isOpen); + + // The doc on [http.BaseClient.send] goes further than the following + // condition, suggesting that any exception thrown there should be an + // [http.ClientException]. But from the upstream implementation, in the + // actual live app, we already get TlsException and SocketException, + // without them getting wrapped in http.ClientException as that specifies. + // So naturally our tests need to simulate those too. + if (httpException is ApiRequestException) { + throw FlutterError.fromParts([ + ErrorSummary('FakeApiConnection.prepare was passed an ApiRequestException.'), + ErrorDescription( + 'The `httpException` parameter to FakeApiConnection.prepare describes ' + 'an exception for the underlying HTTP request to throw. ' + 'In the actual app, that will never be a Zulip-specific exception ' + 'like an ApiRequestException.'), + ErrorHint('Try using the `apiException` parameter instead.') + ]); + } + + if (apiException != null) { + assert(httpException == null + && httpStatus == null && json == null && body == null); + httpStatus = apiException.httpStatus; + json = { + 'result': 'error', + 'code': apiException.code, + 'msg': apiException.message, + ...apiException.data, + }; + } + client.prepare( - exception: exception, + exception: httpException, httpStatus: httpStatus, json: json, body: body, delay: delay, ); diff --git a/test/api/fake_api_test.dart b/test/api/fake_api_test.dart index 25b4bcff2e..53dae4e870 100644 --- a/test/api/fake_api_test.dart +++ b/test/api/fake_api_test.dart @@ -5,6 +5,7 @@ import 'package:test/scaffolding.dart'; import 'package:zulip/api/exception.dart'; import '../fake_async.dart'; +import '../stdlib_checks.dart'; import 'exception_checks.dart'; import 'fake_api.dart'; @@ -25,6 +26,40 @@ void main() { ..asString.contains('FakeApiConnection.prepare')); }); + test('prepare HTTP exception -> get NetworkException', () async { + final connection = FakeApiConnection(); + final exception = Exception('oops'); + connection.prepare(httpException: exception); + await check(connection.get('aRoute', (json) => json, '/', null)) + .throws((it) => it.isA() + ..cause.identicalTo(exception)); + }); + + test('error message on prepare API exception as "HTTP exception"', () async { + final connection = FakeApiConnection(); + final exception = ZulipApiException(routeName: 'someRoute', + httpStatus: 456, code: 'SOME_ERROR', + data: {'foo': ['bar']}, message: 'Something failed'); + check(() => connection.prepare(httpException: exception)) + .throws().asString.contains('apiException'); + }); + + test('prepare API exception', () async { + final connection = FakeApiConnection(); + final exception = ZulipApiException(routeName: 'someRoute', + httpStatus: 456, code: 'SOME_ERROR', + data: {'foo': ['bar']}, message: 'Something failed'); + connection.prepare(apiException: exception); + await check(connection.get('aRoute', (json) => json, '/', null)) + .throws((it) => it.isA() + ..routeName.equals('aRoute') // actual route, not the prepared one + ..routeName.not((it) => it.equals(exception.routeName)) + ..httpStatus.equals(exception.httpStatus) + ..code.equals(exception.code) + ..data.deepEquals(exception.data) + ..message.equals(exception.message)); + }); + test('delay success', () => awaitFakeAsync((async) async { final connection = FakeApiConnection(); connection.prepare(delay: const Duration(seconds: 2), @@ -44,7 +79,7 @@ void main() { test('delay exception', () => awaitFakeAsync((async) async { final connection = FakeApiConnection(); connection.prepare(delay: const Duration(seconds: 2), - exception: Exception("oops")); + httpException: Exception("oops")); Object? error; unawaited(connection.get('aRoute', (json) => null, '/', null) diff --git a/test/api/route/messages_test.dart b/test/api/route/messages_test.dart index 4da4334bae..0d969f0531 100644 --- a/test/api/route/messages_test.dart +++ b/test/api/route/messages_test.dart @@ -65,12 +65,8 @@ void main() { test('modern; message not found', () { return FakeApiConnection.with_((connection) async { final message = eg.streamMessage(); - final fakeResponseJson = { - 'code': 'BAD_REQUEST', - 'msg': 'Invalid message(s)', - 'result': 'error', - }; - connection.prepare(httpStatus: 400, json: fakeResponseJson); + connection.prepare( + apiException: eg.apiBadRequest(message: 'Invalid message(s)')); final result = await checkGetMessageCompat(connection, expectLegacy: false, messageId: message.id, diff --git a/test/example_data.dart b/test/example_data.dart index 6cab496157..116a9a1bd8 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -27,6 +27,18 @@ Object nullCheckError() { try { null!; } catch (e) { return e; } // ignore: null_check_always_fails } +/// A Zulip API error with the generic "BAD_REQUEST" error code. +/// +/// The server returns this error code for a wide range of error conditions; +/// it's the default within the server code when no more-specific code is chosen. +ZulipApiException apiBadRequest({ + String routeName = 'someRoute', String message = 'Something failed'}) { + return ZulipApiException( + routeName: routeName, + httpStatus: 400, code: 'BAD_REQUEST', + data: {}, message: message); +} + /// The error the server gives when the client's credentials /// (API key together with email and realm URL) are no longer valid. /// diff --git a/test/model/actions_test.dart b/test/model/actions_test.dart index 46cc2c6bdd..ab97625c98 100644 --- a/test/model/actions_test.dart +++ b/test/model/actions_test.dart @@ -103,7 +103,7 @@ void main() { assert(unregisterDelay > TestGlobalStore.removeAccountDuration); final exception = eg.apiExceptionUnauthorized(routeName: 'removeEtcEtcToken'); final newConnection = separateConnection() - ..prepare(delay: unregisterDelay, exception: exception); + ..prepare(delay: unregisterDelay, apiException: exception); final future = logOutAccount(testBinding.globalStore, eg.selfAccount.id); // Unregister-token request and account removal dispatched together @@ -165,7 +165,7 @@ void main() { final exception = eg.apiExceptionUnauthorized(routeName: 'removeEtcEtcToken'); final newConnection = separateConnection() - ..prepare(exception: exception); + ..prepare(apiException: exception); final future = unregisterToken(testBinding.globalStore, eg.selfAccount.id); async.elapse(Duration.zero); await future; diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 6a1d103c84..2fa97fd5e3 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -247,8 +247,7 @@ void main() { await prepareMessages(foundOldest: false, messages: initialMessages); check(connection.takeRequests()).single; - connection.prepare(httpStatus: 400, json: { - 'result': 'error', 'code': 'BAD_REQUEST', 'msg': 'Bad request'}); + connection.prepare(apiException: eg.apiBadRequest()); check(async.pendingTimers).isEmpty(); await check(model.fetchOlder()).throws(); checkNotified(count: 2); @@ -1061,8 +1060,7 @@ void main() { addTearDown(() => BackoffMachine.debugDuration = null); await prepareNarrow(narrow, initialMessages); - connection.prepare(httpStatus: 400, json: { - 'result': 'error', 'code': 'BAD_REQUEST', 'msg': 'Bad request'}); + connection.prepare(apiException: eg.apiBadRequest()); BackoffMachine.debugDuration = const Duration(seconds: 1); await check(model.fetchOlder()).throws(); final backoffTimerA = async.pendingTimers.single; @@ -1094,8 +1092,7 @@ void main() { check(model).fetchOlderCoolingDown.isFalse(); check(backoffTimerA.isActive).isTrue(); - connection.prepare(httpStatus: 400, json: { - 'result': 'error', 'code': 'BAD_REQUEST', 'msg': 'Bad request'}); + connection.prepare(apiException: eg.apiBadRequest()); BackoffMachine.debugDuration = const Duration(seconds: 2); await check(model.fetchOlder()).throws(); final backoffTimerB = async.pendingTimers.last; diff --git a/test/model/store_test.dart b/test/model/store_test.dart index bc393d6d6f..ad80a6263d 100644 --- a/test/model/store_test.dart +++ b/test/model/store_test.dart @@ -473,7 +473,7 @@ void main() { // Try to load, inducing an error in the request. globalStore.useCachedApiConnections = true; - connection.prepare(exception: Exception('failed')); + connection.prepare(httpException: Exception('failed')); final future = UpdateMachine.load(globalStore, eg.selfAccount.id); bool complete = false; unawaited(future.whenComplete(() => complete = true)); @@ -541,7 +541,7 @@ void main() { check(store.debugServerEmojiData).isNull(); // Try to fetch, inducing an error in the request. - connection.prepare(exception: Exception('failed')); + connection.prepare(httpException: Exception('failed')); final future = updateMachine.fetchEmojiData(emojiDataUrl); bool complete = false; unawaited(future.whenComplete(() => complete = true)); @@ -712,11 +712,11 @@ void main() { } void prepareNetworkExceptionSocketException() { - connection.prepare(exception: const SocketException('failed')); + connection.prepare(httpException: const SocketException('failed')); } void prepareNetworkException() { - connection.prepare(exception: Exception("failed")); + connection.prepare(httpException: Exception("failed")); } void prepareServer5xxException() { diff --git a/test/stdlib_checks.dart b/test/stdlib_checks.dart index 2d55ba0c37..792588ee75 100644 --- a/test/stdlib_checks.dart +++ b/test/stdlib_checks.dart @@ -25,6 +25,10 @@ extension NullableMapChecks on Subject?> { } } +extension ErrorChecks on Subject { + Subject get asString => has((x) => x.toString(), 'toString'); // TODO(checks): what's a good convention for this? +} + /// Convert [object] to a pure JSON-like value. /// /// The result is similar to `jsonDecode(jsonEncode(object))`, but without diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 7da94cfd36..5a4c22c604 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -100,12 +100,7 @@ void main() { } void prepareRawContentResponseError() { - final fakeResponseJson = { - 'code': 'BAD_REQUEST', - 'msg': 'Invalid message(s)', - 'result': 'error', - }; - connection.prepare(httpStatus: 400, json: fakeResponseJson); + connection.prepare(apiException: eg.apiBadRequest(message: 'Invalid message(s)')); } group('topic action sheet', () { @@ -377,8 +372,7 @@ void main() { isChannelMuted: false, visibilityPolicy: UserTopicVisibilityPolicy.followed); - connection.prepare(httpStatus: 400, json: { - 'result': 'error', 'code': 'BAD_REQUEST', 'msg': ''}); + connection.prepare(apiException: eg.apiBadRequest()); await tester.tap(unfollow); await tester.pumpAndSettle(); @@ -545,7 +539,7 @@ void main() { await prepare(topic: 'zulip'); await showFromRecipientHeader(tester, message: message); connection.takeRequests(); - connection.prepare(exception: http.ClientException('Oops')); + connection.prepare(httpException: http.ClientException('Oops')); await tester.tap(findButtonForLabel('Mark as resolved')); await tester.pumpAndSettle(); checkRequest(message.id, '✔ zulip'); @@ -559,7 +553,7 @@ void main() { await prepare(topic: '✔ zulip'); await showFromRecipientHeader(tester, message: message); connection.takeRequests(); - connection.prepare(exception: http.ClientException('Oops')); + connection.prepare(httpException: http.ClientException('Oops')); await tester.tap(findButtonForLabel('Mark as unresolved')); await tester.pumpAndSettle(); checkRequest(message.id, 'zulip'); @@ -629,11 +623,8 @@ void main() { final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - connection.prepare(httpStatus: 400, json: { - 'code': 'BAD_REQUEST', - 'msg': 'Invalid message(s)', - 'result': 'error', - }); + connection.prepare( + apiException: eg.apiBadRequest(message: 'Invalid message(s)')); await tapButton(tester); await tester.pump(Duration.zero); // error arrives; error dialog shows @@ -698,11 +689,8 @@ void main() { await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); final zulipLocalizations = GlobalLocalizations.zulipLocalizations; - connection.prepare(httpStatus: 400, json: { - 'code': 'BAD_REQUEST', - 'msg': 'Invalid message(s)', - 'result': 'error', - }); + connection.prepare( + apiException: eg.apiBadRequest(message: 'Invalid message(s)')); await tapButton(tester); await tester.pump(Duration.zero); // error arrives; error dialog shows @@ -716,11 +704,8 @@ void main() { await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); final zulipLocalizations = GlobalLocalizations.zulipLocalizations; - connection.prepare(httpStatus: 400, json: { - 'code': 'BAD_REQUEST', - 'msg': 'Invalid message(s)', - 'result': 'error', - }); + connection.prepare( + apiException: eg.apiBadRequest(message: 'Invalid message(s)')); await tapButton(tester, starred: true); await tester.pump(Duration.zero); // error arrives; error dialog shows @@ -1016,7 +1001,7 @@ void main() { final message = eg.streamMessage(flags: [MessageFlag.read]); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - connection.prepare(exception: http.ClientException('Oops')); + connection.prepare(httpException: http.ClientException('Oops')); final zulipLocalizations = GlobalLocalizations.zulipLocalizations; await tester.ensureVisible(find.byIcon(Icons.mark_chat_unread_outlined, skipOffstage: false)); diff --git a/test/widgets/actions_test.dart b/test/widgets/actions_test.dart index 290d6f3e53..9148cf4fbf 100644 --- a/test/widgets/actions_test.dart +++ b/test/widgets/actions_test.dart @@ -329,7 +329,7 @@ void main() { testWidgets('catch-all api errors', (tester) async { await prepare(tester); - connection.prepare(exception: http.ClientException('Oops')); + connection.prepare(httpException: http.ClientException('Oops')); final didPass = invokeUpdateMessageFlagsStartingFromAnchor(); await tester.pump(Duration.zero); checkErrorDialog(tester, diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index f1eb9bb3ba..896dec7efe 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -553,13 +553,8 @@ void main() { testWidgets('ZulipApiException', (tester) async { await setupAndTapSend(tester, prepareResponse: (message) { - connection.prepare( - httpStatus: 400, - json: { - 'result': 'error', - 'code': 'BAD_REQUEST', - 'msg': 'You do not have permission to initiate direct message conversations.', - }); + connection.prepare(apiException: eg.apiBadRequest( + message: 'You do not have permission to initiate direct message conversations.')); }); final zulipLocalizations = GlobalLocalizations.zulipLocalizations; await tester.tap(find.byWidget(checkErrorDialog(tester, diff --git a/test/widgets/emoji_reaction_test.dart b/test/widgets/emoji_reaction_test.dart index 89333ee1af..7ed54590ef 100644 --- a/test/widgets/emoji_reaction_test.dart +++ b/test/widgets/emoji_reaction_test.dart @@ -452,12 +452,7 @@ void main() { connection.prepare( delay: const Duration(seconds: 2), - httpStatus: 400, json: { - 'code': 'BAD_REQUEST', - 'msg': 'Invalid message(s)', - 'result': 'error', - }); - + apiException: eg.apiBadRequest(message: 'Invalid message(s)')); await tester.tap(find.descendant( of: find.byType(BottomSheet), matching: find.text('\u{1f4a4}'))); // 'zzz' emoji diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 3f79f8cae6..58782a30ec 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -595,15 +595,10 @@ void main() { await setupMessageListPage(tester, narrow: narrow, messages: [message], unreadMsgs: unreadMsgs); check(isMarkAsReadButtonVisible(tester)).isTrue(); - - connection.prepare(httpStatus: 400, json: { - 'code': 'BAD_REQUEST', - 'msg': 'Invalid message(s)', - 'result': 'error', - }); - checkAppearsLoading(tester, false); + connection.prepare( + apiException: eg.apiBadRequest(message: 'Invalid message(s)')); await tester.tap(find.byType(MarkAsReadWidget)); await tester.pump(); checkAppearsLoading(tester, true); @@ -694,7 +689,7 @@ void main() { narrow: narrow, messages: [message], unreadMsgs: unreadMsgs); check(isMarkAsReadButtonVisible(tester)).isTrue(); - connection.prepare(exception: http.ClientException('Oops')); + connection.prepare(httpException: http.ClientException('Oops')); await tester.tap(find.byType(MarkAsReadWidget)); await tester.pumpAndSettle(); checkErrorDialog(tester,