Skip to content

fake_api: Prepare API exceptions conveniently, distinct from HTTP exceptions #1359

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

Merged
merged 4 commits into from
Feb 19, 2025
Merged
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
2 changes: 1 addition & 1 deletion test/api/core_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ Future<T> tryRequest<T extends Object?>({
fromJson ??= (((Map<String, dynamic> x) => x) as T Function(Map<String, dynamic>));
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', {});
});
}
Expand Down
56 changes: 49 additions & 7 deletions test/api/fake_api.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -209,28 +210,69 @@ class FakeApiConnection extends ApiConnection {

List<http.BaseRequest> 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<String, dynamic>? 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,
);
Expand Down
37 changes: 36 additions & 1 deletion test/api/fake_api_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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<NetworkException>()
..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<Error>().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<ZulipApiException>()
..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),
Expand All @@ -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)
Expand Down
8 changes: 2 additions & 6 deletions test/api/route/messages_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 12 additions & 0 deletions test/example_data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down
4 changes: 2 additions & 2 deletions test/model/actions_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 3 additions & 6 deletions test/model/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<ZulipApiException>();
checkNotified(count: 2);
Expand Down Expand Up @@ -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<ZulipApiException>();
final backoffTimerA = async.pendingTimers.single;
Expand Down Expand Up @@ -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<ZulipApiException>();
final backoffTimerB = async.pendingTimers.last;
Expand Down
8 changes: 4 additions & 4 deletions test/model/store_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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() {
Expand Down
4 changes: 4 additions & 0 deletions test/stdlib_checks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ extension NullableMapChecks<K, V> on Subject<Map<K, V>?> {
}
}

extension ErrorChecks on Subject<Error> {
Subject<String> 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
Expand Down
37 changes: 11 additions & 26 deletions test/widgets/action_sheet_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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', () {
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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');
Expand All @@ -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');
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion test/widgets/actions_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 2 additions & 7 deletions test/widgets/compose_box_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading