From 01318c7ddecb26cbab0be4b25c9cdf058db35f6d Mon Sep 17 00:00:00 2001 From: Bretton Date: Sun, 4 Oct 2026 16:10:16 -0700 Subject: [PATCH] fix(errors): show the server's message for plain 4xx errors A plain 4xx ApiException (a rate limit, a ban, a validation-style rejection) always showed the generic fallback, so users never saw why the server said no. getErrorMessage now shows the server's trimmed message for any 4xx other than 401 and 404, and keeps the fallback when that message is blank, is only the machine error code, is the "Request failed with status N" diagnostic, looks like HTML, spans lines, or is longer than 200 characters. The community join button went through none of this: it showed raw exception text. It now uses ErrorMessage.subscription, and a leave that 404s (the subscription isn't indexed yet, or was removed elsewhere) shows "Could not update subscription. Please try again." instead of "Content not found", which also fixes the community feed header. Changes: - error_messages.dart: 4xx server-message passthrough with the fallback rules above; ErrorMessage.subscription maps NotFoundException to the subscription fallback - api_exceptions.dart: requestFailedMessageForStatus names the no-message diagnostic so the helper can recognise it - community_join_button.dart: both catch branches use ErrorMessage.subscription; the ScaffoldMessenger is captured before the await so a failure still shows when a lazy list disposes the button - Provider tests pin that providers rethrow the service's exact exception - Maestro b8 flow: a rate-limited vote shows the server's message - Tests: error_messages_test, community_join_button_test (join, leave 404, unmount mid-request), post_card_test Co-Authored-By: Claude Opus 5.5 (1M context) --- .maestro/b8_vote_rejected_message.yaml | 51 +++ .maestro/config.yaml | 3 + .../scripts/exhaust_appview_rate_limit.js | 63 ++++ lib/services/api_exceptions.dart | 6 +- lib/utils/error_messages.dart | 43 ++- lib/widgets/community_join_button.dart | 19 +- test/providers/block_provider_test.dart | 10 +- test/providers/comments_provider_test.dart | 16 +- .../community_subscription_provider_test.dart | 5 +- test/utils/error_messages_test.dart | 347 ++++++++++++++++++ test/widgets/community_join_button_test.dart | 250 +++++++++++++ test/widgets/post_card_test.dart | 51 +++ 12 files changed, 835 insertions(+), 29 deletions(-) create mode 100644 .maestro/b8_vote_rejected_message.yaml create mode 100644 .maestro/scripts/exhaust_appview_rate_limit.js create mode 100644 test/utils/error_messages_test.dart create mode 100644 test/widgets/community_join_button_test.dart diff --git a/.maestro/b8_vote_rejected_message.yaml b/.maestro/b8_vote_rejected_message.yaml new file mode 100644 index 0000000..8d17b89 --- /dev/null +++ b/.maestro/b8_vote_rejected_message.yaml @@ -0,0 +1,51 @@ +# Section B — check 8: a vote the AppView rejects with a 4xx shows the +# server's own message in the snackbar, not the generic +# "Could not update your vote. Please try again." fallback. +# +# The rejection is real: scripts/exhaust_appview_rate_limit.js spends the +# loopback client's global rate-limit budget (100 requests per 60-second fixed +# window) through Caddy, which the emulator reaches over `adb reverse`, so the +# upvote gets the AppView's 429 plain-text body "Rate limit exceeded. Please try +# again later." +# +# The window opens at the first request after the previous one expired, so a +# budget spent late in a window can refill before the tap and let the upvote +# land. The script therefore spends the current window, polls until it resets, +# and spends the fresh one, which leaves the tap a few seconds into a 60-second +# window. The script waits up to a minute for that reset, with no input to the +# device, so the device must stay awake that long. The vote can only land if +# Maestro takes the rest of that minute to reach the tap; the rejected vote never +# reaches the server, so no data changes. The budget refills about a minute +# after the script ends. +# +# Tagged standalone: every flow run in the minute after this one would also be +# rate limited, so it stays out of the ordered workspace run. Needs a signed-in +# session (a1_login_session) and at least one post on the Discover feed that +# the account has not upvoted. +appId: social.coves.dev +tags: + - standalone +env: + PROXY_URL: http://127.0.0.1:8080 +--- +- launchApp: + stopApp: true +- extendedWaitUntil: + visible: "Discover feed.*" + timeout: 30000 +- extendedWaitUntil: + visible: "Feed post in [\\s\\S]*" + timeout: 15000 +- assertVisible: "[\\s\\S]*Upvote post" +- runScript: scripts/exhaust_appview_rate_limit.js +- assertTrue: ${output.rateLimited} +- tapOn: + text: "[\\s\\S]*Upvote post" + index: 0 +- extendedWaitUntil: + visible: "Rate limit exceeded. Please try again later." + timeout: 10000 +- assertNotVisible: "Could not update your vote. Please try again." +# The optimistic upvote rolls back on the rejection. +- assertVisible: "[\\s\\S]*Upvote post" +- takeScreenshot: build/maestro/b8_vote_rejected_message diff --git a/.maestro/config.yaml b/.maestro/config.yaml index 0ea2762..602d73c 100644 --- a/.maestro/config.yaml +++ b/.maestro/config.yaml @@ -18,6 +18,9 @@ # install), wiping the legal acceptances AND the signed-in session every # stateful flow depends on. Run it standalone, then a1_login_session — # see its header comment. +# b8_vote_rejected_message is NOT in the order: it exhausts the loopback +# client's AppView rate limit for up to a minute, which would fail every flow +# run after it. Run it standalone — see its header comment. # NOTE (2026-07-19 round 2): flows absent from flowsOrder STILL RUN at the # end of a workspace run — f5, g1, c6 and c7 are therefore tagged `standalone` and # excluded here. Tag any future precursor-dependent flow the same way. diff --git a/.maestro/scripts/exhaust_appview_rate_limit.js b/.maestro/scripts/exhaust_appview_rate_limit.js new file mode 100644 index 0000000..9899c5a --- /dev/null +++ b/.maestro/scripts/exhaust_appview_rate_limit.js @@ -0,0 +1,63 @@ +// Spend the loopback client's global AppView rate-limit budget (100 requests +// per 60-second window, keyed by client IP) through the Caddy proxy the app +// itself uses, so the app's next request gets the AppView's real 429 +// plain-text rejection. Sets output.rateLimited only when the budget of a +// window that opened moments ago is spent, which leaves the flow almost the +// whole window to tap before the budget refills. +// +// The AppView limiter (coves internal/api/middleware/ratelimit.go) uses a fixed +// window that opens at the first request after the previous window expired. A +// rejected request neither counts nor moves the window. Stopping at the first +// 429 alone can leave only a moment of that window, so the script runs in +// three phases: +// 1. Spend until the first 429: the current window is exhausted. +// 2. Poll until the first non-429: that poll opened a fresh window. +// 3. Spend until 429 again: the fresh window is exhausted about a second in. +// Phase 2 waits out the rest of the current window, which is up to 60 seconds. +// A window reset in the middle of phase 1 can push it to about 200 requests. +var spendRequestLimit = 250; +var resetWaitLimitMs = 70000; +var resetPollIntervalMs = 200; +var lastStatus = null; + +function isRateLimited() { + lastStatus = http.get(PROXY_URL + '/xrpc/_health').status; + return lastStatus === 429; +} + +function spendUntilRateLimited() { + for (var attempt = 0; attempt < spendRequestLimit; attempt++) { + if (isRateLimited()) { + return true; + } + } + return false; +} + +function waitForWindowReset() { + var deadline = Date.now() + resetWaitLimitMs; + while (Date.now() < deadline) { + if (!isRateLimited()) { + return true; + } + var nextPoll = Date.now() + resetPollIntervalMs; + while (Date.now() < nextPoll) { + // Busy-wait: Maestro's script engine has no sleep or setTimeout. + } + } + return false; +} + +output.rateLimited = false; +if (!spendUntilRateLimited()) { + console.log('exhaust_appview_rate_limit: phase 1 got no 429 in ' + + spendRequestLimit + ' requests; last status ' + lastStatus); +} else if (!waitForWindowReset()) { + console.log('exhaust_appview_rate_limit: phase 2 saw no window reset in ' + + resetWaitLimitMs + ' ms; last status ' + lastStatus); +} else if (!spendUntilRateLimited()) { + console.log('exhaust_appview_rate_limit: phase 3 got no 429 in ' + + spendRequestLimit + ' requests; last status ' + lastStatus); +} else { + output.rateLimited = true; +} diff --git a/lib/services/api_exceptions.dart b/lib/services/api_exceptions.dart index c278a5c..9cf7c52 100644 --- a/lib/services/api_exceptions.dart +++ b/lib/services/api_exceptions.dart @@ -96,7 +96,7 @@ class ApiException implements Exception { ); } return ApiException( - message ?? 'Request failed with status $statusCode', + message ?? requestFailedMessageForStatus(statusCode), statusCode: statusCode, errorCode: errorCode, originalError: error, @@ -214,6 +214,10 @@ class ApiException implements Exception { String toString() => message; } +/// Diagnostic [ApiException.fromDioError] sets when a body has no message. +String requestFailedMessageForStatus(int statusCode) => + 'Request failed with status $statusCode'; + /// Maps [error] to a typed [ApiException] after logging it in debug builds /// (response data is passed through [redactBearerTokens] first). /// [operation] labels the log line, e.g. 'fetch timeline'. diff --git a/lib/utils/error_messages.dart b/lib/utils/error_messages.dart index c6bc9ca..497545d 100644 --- a/lib/utils/error_messages.dart +++ b/lib/utils/error_messages.dart @@ -7,6 +7,9 @@ library; import '../services/api_exceptions.dart'; +/// Longest trimmed 4xx server message [getErrorMessage] shows to the user. +const _maximumServerMessageLength = 200; + /// Converts an exception to a user-friendly error message. /// /// Uses type-based matching for reliability: @@ -16,6 +19,11 @@ import '../services/api_exceptions.dart'; /// - [NotFoundException] → Resource not found /// - [ValidationException] → Shows the validation message /// - [FederationException] → atProto federation issues +/// - Other [ApiException] with a 4xx status other than 401 or 404 → Shows the +/// server's message, trimmed, or the fallback when that message is blank, +/// is only the machine error code, is the [requestFailedMessageForStatus] +/// diagnostic that stands in for a body with no message, starts with `<`, +/// contains a line break, or is longer than 200 characters /// - Other [ApiException] → Uses fallback /// - Unknown exceptions → Uses fallback /// @@ -60,6 +68,27 @@ String getErrorMessage(Object error, {String? fallback}) { } if (error is ApiException) { + final statusCode = error.statusCode; + final message = error.message.trim(); + if (statusCode != null && + statusCode >= 400 && + statusCode <= 499 && + statusCode != 401 && + statusCode != 404 && + message.isNotEmpty && + // A message equal to the error code means the body carried only the + // machine code, not a human-readable message. + message != error.errorCode && + // A body with no message leaves fromDioError's status diagnostic. + message != requestFailedMessageForStatus(statusCode) && + // A plain-text body is used as-is, so a proxy's HTML page or a long + // or multi-line body would otherwise render verbatim. + !message.startsWith('<') && + !message.contains('\n') && + !message.contains('\r') && + message.length <= _maximumServerMessageLength) { + return message; + } return defaultFallback; } @@ -112,10 +141,16 @@ abstract final class ErrorMessage { ); /// Error message for subscription operations (join/leave community) - static String subscription(Object error) => getErrorMessage( - error, - fallback: 'Could not update subscription. Please try again.', - ); + /// + /// A [NotFoundException] gets the fallback: a leave 404s when the + /// subscription isn't indexed yet, and "content removed" copy would wrongly + /// imply the community was deleted. + static String subscription(Object error) { + const fallback = 'Could not update subscription. Please try again.'; + return error is NotFoundException + ? fallback + : getErrorMessage(error, fallback: fallback); + } /// Error message for block operations static String block(Object error) => getErrorMessage( diff --git a/lib/widgets/community_join_button.dart b/lib/widgets/community_join_button.dart index f440a6d..6a73b61 100644 --- a/lib/widgets/community_join_button.dart +++ b/lib/widgets/community_join_button.dart @@ -5,6 +5,7 @@ import 'package:sentry_flutter/sentry_flutter.dart'; import '../constants/app_colors.dart'; import '../providers/community_subscription_provider.dart'; import '../services/api_exceptions.dart'; +import '../utils/error_messages.dart'; /// Style variant for the join button. enum CommunityJoinButtonStyle { @@ -62,20 +63,22 @@ class CommunityJoinButton extends StatelessWidget { BuildContext context, CommunitySubscriptionProvider provider, ) async { + // Captured before the await: a lazy list can dispose this button while + // the request is in flight, and the failure must still be shown. + final messenger = ScaffoldMessenger.of(context); try { await provider.toggleSubscription(communityDid: communityDid); } on ApiException catch (e) { - if (context.mounted) { - ScaffoldMessenger.of(context) - .showSnackBar(SnackBar(content: Text(e.message))); + if (messenger.mounted) { + messenger.showSnackBar( + SnackBar(content: Text(ErrorMessage.subscription(e))), + ); } } on Exception catch (e, stackTrace) { await Sentry.captureException(e, stackTrace: stackTrace); - if (context.mounted) { - ScaffoldMessenger.of(context).showSnackBar( - const SnackBar( - content: Text('Failed to update membership. Please try again.'), - ), + if (messenger.mounted) { + messenger.showSnackBar( + SnackBar(content: Text(ErrorMessage.subscription(e))), ); } } diff --git a/test/providers/block_provider_test.dart b/test/providers/block_provider_test.dart index 712fc99..8aa73e4 100644 --- a/test/providers/block_provider_test.dart +++ b/test/providers/block_provider_test.dart @@ -111,12 +111,13 @@ void main() { test( 'rollback: failed toggle reverts, notifies, and unblocks seeds', () async { + final failure = ApiException('Server error', statusCode: 500); when(mockApiService.blockUser(actor: anyNamed('actor'))) - .thenThrow(ApiException('Server error', statusCode: 500)); + .thenThrow(failure); await expectLater( provider.toggleUserBlock(userDid: userDid), - throwsA(isA()), + throwsA(same(failure)), ); expect(provider.isUserBlocked(userDid), isFalse); @@ -243,12 +244,13 @@ void main() { }); test('failed community toggle reverts and unblocks seeds', () async { + final failure = ApiException('Server error', statusCode: 500); when(mockApiService.blockCommunity(community: anyNamed('community'))) - .thenThrow(ApiException('Server error', statusCode: 500)); + .thenThrow(failure); await expectLater( provider.toggleCommunityBlock(communityDid: communityDid), - throwsA(isA()), + throwsA(same(failure)), ); expect(provider.isCommunityBlocked(communityDid), isFalse); diff --git a/test/providers/comments_provider_test.dart b/test/providers/comments_provider_test.dart index 59ba5f0..272593f 100644 --- a/test/providers/comments_provider_test.dart +++ b/test/providers/comments_provider_test.dart @@ -1764,6 +1764,7 @@ void main() { test('should rethrow exception from CommentService', () async { await providerWithCommentService.loadComments(refresh: true); + final failure = ApiException('Network error'); when( mockCommentService.createComment( rootUri: anyNamed('rootUri'), @@ -1772,18 +1773,12 @@ void main() { parentCid: anyNamed('parentCid'), content: anyNamed('content'), ), - ).thenThrow(ApiException('Network error')); + ).thenThrow(failure); expect( () => providerWithCommentService.createComment(content: 'Test comment'), - throwsA( - isA().having( - (e) => e.message, - 'message', - contains('Network error'), - ), - ), + throwsA(same(failure)), ); }); @@ -2429,6 +2424,7 @@ void main() { test('should propagate fetch errors and clear loading state', () async { await commentsProvider.loadComments(refresh: true); + final failure = ApiException('Network error'); when( mockApiService.getComments( postUri: anyNamed('postUri'), @@ -2438,11 +2434,11 @@ void main() { limit: anyNamed('limit'), parentRkey: argThat(isNotNull, named: 'parentRkey'), ), - ).thenThrow(ApiException('Network error')); + ).thenThrow(failure); await expectLater( commentsProvider.loadMoreReplies(parentUri), - throwsA(isA()), + throwsA(same(failure)), ); expect(commentsProvider.loadingMoreReplies, isEmpty); // Tree is untouched on failure diff --git a/test/providers/community_subscription_provider_test.dart b/test/providers/community_subscription_provider_test.dart index 8678917..1b887e7 100644 --- a/test/providers/community_subscription_provider_test.dart +++ b/test/providers/community_subscription_provider_test.dart @@ -128,13 +128,14 @@ void main() { test( 'rollback: failed toggle reverts, notifies, and unblocks seeds', () async { + final failure = ApiException('Server error', statusCode: 500); when( mockApiService.subscribeToCommunity(community: anyNamed('community')), - ).thenThrow(ApiException('Server error', statusCode: 500)); + ).thenThrow(failure); await expectLater( provider.toggleSubscription(communityDid: did), - throwsA(isA()), + throwsA(same(failure)), ); expect(provider.isSubscribed(did), isFalse); diff --git a/test/utils/error_messages_test.dart b/test/utils/error_messages_test.dart new file mode 100644 index 0000000..80a7d0d --- /dev/null +++ b/test/utils/error_messages_test.dart @@ -0,0 +1,347 @@ +import 'package:coves_flutter/services/api_exceptions.dart'; +import 'package:coves_flutter/utils/error_messages.dart'; +import 'package:dio/dio.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + ApiException mapResponse(int statusCode, Object? data) { + final requestOptions = RequestOptions(path: '/xrpc/x'); + return ApiException.fromDioError( + DioException( + requestOptions: requestOptions, + response: Response( + requestOptions: requestOptions, + statusCode: statusCode, + data: data, + ), + type: DioExceptionType.badResponse, + ), + ); + } + + group( + 'getErrorMessage shows the server message for plain 4xx ApiExceptions', + () { + final humanMessageCases = <({int statusCode, String message})>[ + (statusCode: 400, message: 'Post title is required'), + (statusCode: 403, message: 'You can only delete your own comments'), + ( + statusCode: 409, + message: 'You are already subscribed to this community', + ), + (statusCode: 429, message: 'Too many votes. Slow down.'), + (statusCode: 499, message: 'Client closed the request'), + ]; + + for (final testCase in humanMessageCases) { + test('status ${testCase.statusCode} returns "${testCase.message}"', () { + final error = ApiException( + testCase.message, + statusCode: testCase.statusCode, + ); + + expect(error.errorCode, isNull); + expect( + getErrorMessage(error, fallback: 'Fallback copy.'), + testCase.message, + ); + }); + } + + test('ErrorMessage.comment of a mapped 403 Banned body returns its ' + 'human message', () { + final error = mapResponse(403, { + 'error': 'Banned', + 'message': 'User is banned from this community', + }); + + expect( + ErrorMessage.comment(error), + 'User is banned from this community', + ); + }); + + test('ErrorMessage.vote of a mapped 429 plain-text body returns it ' + 'trimmed', () { + final error = mapResponse( + 429, + 'Rate limit exceeded. Please try again later.\n', + ); + + expect( + ErrorMessage.vote(error), + 'Rate limit exceeded. Please try again later.', + ); + }); + }, + ); + + group('getErrorMessage existing behavior guards (pass before and after the ' + 'fix)', () { + test('plain ApiException with status 399 returns the fallback', () { + final error = ApiException('Odd status message', statusCode: 399); + + expect( + getErrorMessage(error, fallback: 'Fallback copy.'), + 'Fallback copy.', + ); + }); + + test('plain ApiException with status 500 returns the fallback', () { + final error = ApiException('Unexpected error: boom', statusCode: 500); + + expect( + getErrorMessage(error, fallback: 'Fallback copy.'), + 'Fallback copy.', + ); + }); + + test('plain ApiException without a status returns the fallback', () { + final error = ApiException('CommentService not available'); + + expect( + getErrorMessage(error, fallback: 'Fallback copy.'), + 'Fallback copy.', + ); + }); + + test('plain 403 ApiException with an empty message returns the ' + 'fallback', () { + final error = ApiException('', statusCode: 403); + + expect( + getErrorMessage(error, fallback: 'Fallback copy.'), + 'Fallback copy.', + ); + }); + + test('plain 403 ApiException with a whitespace-only message returns the ' + 'fallback', () { + final error = ApiException(' ', statusCode: 403); + + expect( + getErrorMessage(error, fallback: 'Fallback copy.'), + 'Fallback copy.', + ); + }); + + test('plain ApiException constructed with status 401 returns the ' + 'fallback', () { + final error = ApiException('Token expired', statusCode: 401); + + expect( + getErrorMessage(error, fallback: 'Fallback copy.'), + 'Fallback copy.', + ); + }); + + test('plain ApiException constructed with status 404 returns the ' + 'fallback', () { + final error = ApiException('Record not found', statusCode: 404); + + expect( + getErrorMessage(error, fallback: 'Fallback copy.'), + 'Fallback copy.', + ); + }); + + test('AuthenticationException keeps its sign-in copy', () { + final error = AuthenticationException('Token expired'); + + expect( + getErrorMessage(error, fallback: 'Fallback copy.'), + 'Please sign in to continue.', + ); + }); + + test('NotFoundException keeps its not-found copy', () { + final error = NotFoundException('Post not found'); + + expect( + getErrorMessage(error, fallback: 'Fallback copy.'), + 'Content not found. It may have been removed.', + ); + }); + + test('ServerException from a mapped 500 JSON body keeps its server ' + 'copy', () { + final error = mapResponse(500, { + 'error': 'InternalError', + 'message': 'database connection lost', + }); + + expect(error, isA()); + expect( + getErrorMessage(error, fallback: 'Fallback copy.'), + 'Server error. Please try again later.', + ); + }); + + test('NetworkException keeps its connection copy', () { + final error = NetworkException('Connection refused'); + + expect( + getErrorMessage(error, fallback: 'Fallback copy.'), + 'No connection. Please check your internet.', + ); + }); + + test('FederationException keeps its federation copy', () { + final error = FederationException('DNS lookup failed'); + + expect( + getErrorMessage(error, fallback: 'Fallback copy.'), + 'Could not reach the server. Please try again.', + ); + }); + + test('ValidationException returns its own message', () { + final error = ValidationException('Name too long'); + + expect( + getErrorMessage(error, fallback: 'Fallback copy.'), + 'Name too long', + ); + }); + + test('a non-API error returns the given fallback', () { + expect( + getErrorMessage(StateError('x'), fallback: 'Fallback copy.'), + 'Fallback copy.', + ); + }); + + test('a non-API error without a fallback returns the default copy', () { + expect( + getErrorMessage(StateError('x')), + 'Something went wrong. Please try again.', + ); + }); + + test('ErrorMessage.vote of a plain ApiException without a status returns ' + 'the vote fallback', () { + final error = ApiException('VoteService not available'); + + expect( + ErrorMessage.vote(error), + 'Could not update your vote. Please try again.', + ); + }); + }); + + group('getErrorMessage hides a machine error code standing in for the ' + 'message', () { + test('ErrorMessage.vote of a mapped 429 code-only body returns the vote ' + 'fallback', () { + final error = mapResponse(429, { + 'error': 'RateLimitExceeded', + }); + + expect(error.message, 'RateLimitExceeded'); + expect(error.errorCode, 'RateLimitExceeded'); + expect( + ErrorMessage.vote(error), + 'Could not update your vote. Please try again.', + ); + }); + + test('plain 403 ApiException whose message equals its error code returns ' + 'the fallback', () { + final error = ApiException( + 'Banned', + statusCode: 403, + errorCode: 'Banned', + ); + + expect( + getErrorMessage(error, fallback: 'Fallback copy.'), + 'Fallback copy.', + ); + }); + + test('plain 403 ApiException with a human message beside its error code ' + 'returns the human message', () { + final error = ApiException( + 'User is banned from this community', + statusCode: 403, + errorCode: 'Banned', + ); + + expect( + getErrorMessage(error, fallback: 'Fallback copy.'), + 'User is banned from this community', + ); + }); + }); + + group('getErrorMessage hides the synthetic diagnostic for an empty 4xx ' + 'body', () { + final emptyBodyCases = <({String description, Object? data})>[ + (description: 'a null body', data: null), + (description: 'an empty String body', data: ''), + (description: 'an empty JSON map body', data: {}), + ]; + + for (final testCase in emptyBodyCases) { + test('ErrorMessage.comment of a mapped 413 with ' + '${testCase.description} returns the comment fallback', () { + final error = mapResponse(413, testCase.data); + + expect(error.message, 'Request failed with status 413'); + expect(error.errorCode, isNull); + expect( + ErrorMessage.comment(error), + 'Could not post comment. Please try again.', + ); + }); + } + }); + + group('getErrorMessage hides a 4xx body that is not a short one-line ' + 'message', () { + final unsuitableBodyCases = <({String description, String data})>[ + ( + description: 'an HTML error page', + data: '

403 Forbidden

', + ), + (description: 'a 5,000-character single-line body', data: 'a' * 5000), + ( + description: 'a multi-line body split by a line feed', + data: 'Request blocked.\nReference: 12345', + ), + ( + description: 'a multi-line body split by a carriage return', + data: 'Request blocked.\rReference: 12345', + ), + ]; + + for (final testCase in unsuitableBodyCases) { + test('ErrorMessage.comment of a mapped 403 with ' + '${testCase.description} returns the comment fallback', () { + final error = mapResponse(403, testCase.data); + + expect(error.message, testCase.data); + expect( + ErrorMessage.comment(error), + 'Could not post comment. Please try again.', + ); + }); + } + + test('a mapped 403 body of exactly 200 characters is shown', () { + final body = 'a' * 200; + final error = mapResponse(403, body); + + expect(getErrorMessage(error, fallback: 'Fallback copy.'), body); + }); + + test('a mapped 403 body of 201 characters returns the fallback', () { + final error = mapResponse(403, 'a' * 201); + + expect( + getErrorMessage(error, fallback: 'Fallback copy.'), + 'Fallback copy.', + ); + }); + }); +} diff --git a/test/widgets/community_join_button_test.dart b/test/widgets/community_join_button_test.dart new file mode 100644 index 0000000..30618cc --- /dev/null +++ b/test/widgets/community_join_button_test.dart @@ -0,0 +1,250 @@ +import 'dart:async'; + +import 'package:coves_flutter/providers/community_subscription_provider.dart'; +import 'package:coves_flutter/services/api_exceptions.dart'; +import 'package:coves_flutter/widgets/community_join_button.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; +import 'package:provider/provider.dart'; + +import '../test_helpers/test_mocks.dart'; +import '../test_helpers/theme_pump.dart'; + +const _communityDid = 'did:plc:joinbuttoncommunity'; +const _subscriptionFallback = + 'Could not update subscription. Please try again.'; + +void main() { + late MockCovesApiService apiService; + late MockAuthProvider authProvider; + + setUp(() { + apiService = MockCovesApiService(); + authProvider = MockAuthProvider(); + when(authProvider.isAuthenticated).thenReturn(true); + }); + + /// Pumps a signed-in [CommunityJoinButton] backed by the real + /// [CommunitySubscriptionProvider], makes subscribe fail with [error], taps + /// Join, and settles the request and the SnackBar entrance animation. + Future tapJoinWhenSubscribeThrows( + WidgetTester tester, + Exception error, + ) async { + when(apiService.subscribeToCommunity(community: anyNamed('community'))) + .thenAnswer((_) async => throw error); + + await pumpUnderAppTheme( + tester, + ChangeNotifierProvider( + create: (_) => CommunitySubscriptionProvider( + authProvider: authProvider, + apiService: apiService, + ), + child: const Center( + child: CommunityJoinButton(communityDid: _communityDid), + ), + ), + ); + + await tester.tap(find.text('Join')); + await tester.pumpAndSettle(); + + verify(apiService.subscribeToCommunity(community: _communityDid)).called(1); + expect(find.byType(SnackBar), findsOneWidget); + } + + /// Asserts the SnackBar reads exactly [text], reporting what it did read. + void expectSnackBarText(WidgetTester tester, String text) { + final shown = tester + .widgetList( + find.descendant( + of: find.byType(SnackBar), + matching: find.byType(Text), + ), + ) + .map((widget) => widget.data) + .toList(); + expect( + find.descendant(of: find.byType(SnackBar), matching: find.text(text)), + findsOneWidget, + reason: 'SnackBar showed $shown', + ); + } + + group('CommunityJoinButton join failure SnackBar', () { + testWidgets( + 'given subscribe throws a non-ApiException, when the user taps Join, ' + 'then the SnackBar shows the subscription fallback and no developer ' + 'text', + (tester) async { + await tapJoinWhenSubscribeThrows( + tester, + const FormatException('bad json'), + ); + + expectSnackBarText(tester, _subscriptionFallback); + expect(find.textContaining('Unexpected error'), findsNothing); + expect(find.textContaining('bad json'), findsNothing); + }, + ); + + testWidgets( + 'given subscribe throws AuthenticationException, when the user taps ' + 'Join, then the SnackBar asks the user to sign in', + (tester) async { + const raw = 'Authentication failed. Token expired or invalid'; + await tapJoinWhenSubscribeThrows(tester, AuthenticationException(raw)); + + expectSnackBarText(tester, 'Please sign in to continue.'); + expect(find.text(raw), findsNothing); + }, + ); + + testWidgets( + 'given subscribe throws a 4xx with a readable server message, when the ' + 'user taps Join, then the SnackBar shows the server message', + (tester) async { + const raw = 'User is banned from this community'; + await tapJoinWhenSubscribeThrows( + tester, + ApiException(raw, statusCode: 403, errorCode: 'Banned'), + ); + + expectSnackBarText(tester, raw); + }, + ); + + testWidgets( + 'given subscribe throws a 4xx whose message is only its error code, ' + 'when the user taps Join, then the SnackBar shows the subscription ' + 'fallback', + (tester) async { + const raw = 'RateLimitExceeded'; + await tapJoinWhenSubscribeThrows( + tester, + ApiException(raw, statusCode: 429, errorCode: 'RateLimitExceeded'), + ); + + expectSnackBarText(tester, _subscriptionFallback); + expect(find.text(raw), findsNothing); + }, + ); + + testWidgets( + 'given subscribe throws a 4xx whose message is an HTML body, when the ' + 'user taps Join, then the SnackBar shows the subscription fallback', + (tester) async { + const raw = 'Bad Gateway'; + await tapJoinWhenSubscribeThrows( + tester, + ApiException(raw, statusCode: 400), + ); + + expectSnackBarText(tester, _subscriptionFallback); + expect(find.text(raw), findsNothing); + }, + ); + + testWidgets( + 'given subscribe throws ServerException, when the user taps Join, ' + 'then the SnackBar shows the server error message', + (tester) async { + const raw = 'upstream exploded'; + await tapJoinWhenSubscribeThrows( + tester, + ServerException(raw, statusCode: 502), + ); + + expectSnackBarText(tester, 'Server error. Please try again later.'); + expect(find.text(raw), findsNothing); + }, + ); + + testWidgets('given the button unmounts while subscribe is in flight, when ' + 'subscribe then fails, then the SnackBar still shows the failure', ( + tester, + ) async { + final request = Completer(); + when(apiService.subscribeToCommunity(community: anyNamed('community'))) + .thenAnswer((_) => request.future); + final showButton = ValueNotifier(true); + addTearDown(showButton.dispose); + + await pumpUnderAppTheme( + tester, + ChangeNotifierProvider( + create: (_) => CommunitySubscriptionProvider( + authProvider: authProvider, + apiService: apiService, + ), + child: ValueListenableBuilder( + valueListenable: showButton, + builder: (context, show, _) => show + ? const Center( + child: CommunityJoinButton(communityDid: _communityDid), + ) + : const SizedBox.shrink(), + ), + ), + ); + + await tester.tap(find.text('Join')); + await tester.pump(); + showButton.value = false; + await tester.pump(); + expect(find.byType(CommunityJoinButton), findsNothing); + + request.completeError(const FormatException('bad json')); + await tester.pumpAndSettle(); + + expect(find.byType(SnackBar), findsOneWidget); + expectSnackBarText(tester, _subscriptionFallback); + }); + }); + + group('CommunityJoinButton leave failure SnackBar', () { + testWidgets( + 'given unsubscribe throws NotFoundException, when the user taps ' + 'Joined, then the SnackBar shows the subscription fallback rather than ' + 'content-removed copy', + (tester) async { + when( + apiService.unsubscribeFromCommunity(community: anyNamed('community')), + ).thenAnswer( + (_) async => throw NotFoundException( + 'subscription not found', + errorCode: 'NotFound', + ), + ); + + await pumpUnderAppTheme( + tester, + ChangeNotifierProvider( + create: (_) => + CommunitySubscriptionProvider( + authProvider: authProvider, + apiService: apiService, + )..setInitialSubscriptionState( + communityDid: _communityDid, + isSubscribed: true, + ), + child: const Center( + child: CommunityJoinButton(communityDid: _communityDid), + ), + ), + ); + + await tester.tap(find.text('Joined')); + await tester.pumpAndSettle(); + + verify(apiService.unsubscribeFromCommunity(community: _communityDid)) + .called(1); + expectSnackBarText(tester, _subscriptionFallback); + expect(find.textContaining('Content not found'), findsNothing); + expect(find.text('Joined'), findsOneWidget); + }, + ); + }); +} diff --git a/test/widgets/post_card_test.dart b/test/widgets/post_card_test.dart index 30c7198..676911c 100644 --- a/test/widgets/post_card_test.dart +++ b/test/widgets/post_card_test.dart @@ -4,6 +4,7 @@ import 'package:coves_flutter/constants/app_colors.dart'; import 'package:coves_flutter/models/post.dart'; import 'package:coves_flutter/providers/auth_provider.dart'; import 'package:coves_flutter/providers/vote_provider.dart'; +import 'package:coves_flutter/services/api_exceptions.dart'; import 'package:coves_flutter/services/streamable_service.dart'; import 'package:coves_flutter/widgets/icons/animated_heart_icon.dart'; import 'package:coves_flutter/widgets/icons/lucide_icon_painter.dart'; @@ -783,6 +784,56 @@ void main() { ).called(1); }); + testWidgets( + 'rejected upvote shows the server message instead of the vote fallback', + (tester) async { + final requestOptions = RequestOptions(path: '/xrpc/x'); + final bannedException = ApiException.fromDioError( + DioException( + requestOptions: requestOptions, + response: Response( + requestOptions: requestOptions, + statusCode: 403, + data: { + 'error': 'Banned', + 'message': 'User is banned from this community', + }, + ), + type: DioExceptionType.badResponse, + ), + ); + when( + mockVoteProvider.toggleVote( + postUri: anyNamed('postUri'), + postCid: anyNamed('postCid'), + direction: anyNamed('direction'), + ), + ).thenAnswer((_) => Future.error(bannedException)); + await pumpVotePost(tester); + + final upvote = upvoteControl('Upvote post'); + expect(upvote, findsOneWidget); + await tester.tap(upvote); + await tester.pumpAndSettle(); + + verify( + mockVoteProvider.toggleVote(postUri: postUri, postCid: postCid), + ).called(1); + expect(find.byType(SnackBar), findsOneWidget); + expect( + find.descendant( + of: find.byType(SnackBar), + matching: find.text('User is banned from this community'), + ), + findsOneWidget, + ); + expect( + find.text('Could not update your vote. Please try again.'), + findsNothing, + ); + }, + ); + testWidgets('compact card keeps the comment action without its count', ( tester, ) async { -- 2.54.0