From ef1b1ded5d6044f5f23b58c5061552ee01ff8855 Mon Sep 17 00:00:00 2001 From: Tommy Chow Date: Sun, 21 Jun 2026 15:04:11 -0400 Subject: [PATCH] fix: reclassify transient errors as non-fatal and fix top crashes - main: record transient I/O, plugin-teardown, and non-crashing UI errors (network/websocket/timeout, MissingPluginException, image-load, layout overflow) as non-fatal, and add a grouping reason to async errors so distinct errors stop collapsing into one catch-all Crashlytics issue - irc: guard IRCMessage.fromString against malformed/truncated lines that threw RangeError, with regression tests - chat: wrap the emote menu in OverflowBox so it no longer overflows (RenderFlex) while the panel collapses - chat: fix onBadgeError returning a List instead of a Map, which threw a Future.catchError type error during channel asset loads - video: skip setAutoPipMode on Android < 12 where it isn't implemented Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/main.dart | 52 +++++++- lib/models/irc.dart | 14 ++ lib/screens/channel/chat/chat.dart | 124 ++++++++++-------- .../channel/chat/stores/chat_store.dart | 9 +- lib/screens/channel/video/video_store.dart | 9 +- test/models/irc_test.dart | 18 +++ 6 files changed, 156 insertions(+), 70 deletions(-) diff --git a/lib/main.dart b/lib/main.dart index 6941eca0..dd221dff 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -1,16 +1,20 @@ import 'dart:async'; import 'dart:convert'; +import 'dart:io'; import 'package:advanced_in_app_review/advanced_in_app_review.dart'; import 'package:app_links/app_links.dart'; +import 'package:dio/dio.dart'; import 'package:firebase_analytics/firebase_analytics.dart'; import 'package:firebase_core/firebase_core.dart'; import 'package:firebase_crashlytics/firebase_crashlytics.dart'; import 'package:firebase_performance/firebase_performance.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_mobx/flutter_mobx.dart'; import 'package:flutter_secure_storage/flutter_secure_storage.dart'; +import 'package:frosty/apis/base_api_client.dart' show ApiException; import 'package:frosty/apis/bttv_api.dart'; import 'package:frosty/apis/dio_client.dart'; import 'package:frosty/apis/ffz_api.dart'; @@ -35,6 +39,25 @@ import 'package:mobx/mobx.dart'; import 'package:provider/provider.dart'; import 'package:shared_preferences/shared_preferences.dart'; import 'package:url_launcher/url_launcher.dart'; +import 'package:web_socket_channel/web_socket_channel.dart'; + +/// Errors that are expected on a mobile client — transient network/I/O, plugin +/// teardown races, and image fetch failures — and therefore shouldn't be +/// reported to Crashlytics as fatal crashes (which would tank the crash-free +/// metric and bucket unrelated errors together under the global handlers). +/// +/// This is a last-resort net. The layers that own these errors already record +/// them non-fatally with precise reasons (e.g. base_api_client, the IRC +/// websocket handlers); this only catches what escapes that local handling. +bool _isNonFatalError(Object error) { + return error is SocketException || + error is HttpException || + error is WebSocketChannelException || + error is ApiException || + error is MissingPluginException || + error is NetworkImageLoadException || + (error is DioException && error.type != DioExceptionType.badResponse); +} void main() async { WidgetsFlutterBinding.ensureInitialized(); @@ -43,12 +66,33 @@ void main() async { await Firebase.initializeApp(options: DefaultFirebaseOptions.currentPlatform); - // Pass all uncaught "fatal" errors from the framework to Crashlytics - FlutterError.onError = FirebaseCrashlytics.instance.recordFlutterFatalError; + // Route uncaught framework errors to Crashlytics. Non-crashing UI errors + // (layout overflows, image-load failures) and other expected transient + // failures are recorded as non-fatal so they don't inflate the crash count. + FlutterError.onError = (details) { + // Run the cheap type checks first; only fall back to the diagnostics string + // (RenderFlex overflow has no typed signal) when those miss. + final isNonFatal = + _isNonFatalError(details.exception) || + details.exceptionAsString().contains('overflowed'); + if (isNonFatal) { + FirebaseCrashlytics.instance.recordFlutterError(details); + } else { + FirebaseCrashlytics.instance.recordFlutterFatalError(details); + } + }; - // Pass all uncaught asynchronous errors that aren't handled by the Flutter framework to Crashlytics + // Pass uncaught asynchronous errors to Crashlytics. Expected transient errors + // are recorded non-fatally; everything else stays fatal. The reason gives the + // grouping engine a real signal so distinct errors don't collapse into one + // catch-all issue under this handler frame. PlatformDispatcher.instance.onError = (error, stack) { - FirebaseCrashlytics.instance.recordError(error, stack, fatal: true); + FirebaseCrashlytics.instance.recordError( + error, + stack, + reason: error.runtimeType.toString(), + fatal: !_isNonFatalError(error), + ); return true; }; diff --git a/lib/models/irc.dart b/lib/models/irc.dart index 4f49f28c..8e94cbb3 100644 --- a/lib/models/irc.dart +++ b/lib/models/irc.dart @@ -974,6 +974,12 @@ class IRCMessage { // Obtain the index to break off the tags. final tagAndIrcMessageDivider = whole.indexOf(' '); + // Without a separating space there are no tags to slice and no message body + // to parse — bail out rather than throw on the substring/index calls below. + if (tagAndIrcMessageDivider == -1) { + return IRCMessage(raw: whole, command: Command.none, tags: const {}); + } + // Parse the IRCv3 tags into a map. The tags section is everything between // the leading '@' and the first space. We split on the *raw* string: a // literal ';' is always a separator because escaped semicolons inside a @@ -1006,6 +1012,14 @@ class IRCMessage { // Index 3 and beyond is the :message (word by word) that is sent by the user or empty depending on the command final splitMessage = ircMessage.split(' '); + // A well-formed message is at least ` `. A malformed or + // truncated line (e.g. from the recent-messages history API) can split into + // a single token, which would otherwise throw a RangeError on the + // splitMessage[0]/[1] accesses below. + if (splitMessage.length < 2) { + return IRCMessage(raw: whole, command: Command.none, tags: mappedTags); + } + // If the username exists, set it. // tmi.twitch.tv means the message was sent by Twitch rather than a user, so will be irrelevant. final prefix = splitMessage[0]; diff --git a/lib/screens/channel/chat/chat.dart b/lib/screens/channel/chat/chat.dart index 135940ed..70d14750 100644 --- a/lib/screens/channel/chat/chat.dart +++ b/lib/screens/channel/chat/chat.dart @@ -266,76 +266,84 @@ class Chat extends StatelessWidget { } Widget _buildEmoteMenu(BuildContext context) { + final menuHeight = context.screenHeight / (context.isPortrait ? 3 : 2); return AnimatedContainer( curve: Curves.ease, duration: const Duration(milliseconds: 200), - height: chatStore.assetsStore.showEmoteMenu - ? context.screenHeight / (context.isPortrait ? 3 : 2) - : 0, + height: chatStore.assetsStore.showEmoteMenu ? menuHeight : 0, child: AnimatedSwitcher( duration: const Duration(milliseconds: 100), switchInCurve: Curves.easeOut, switchOutCurve: Curves.easeIn, child: chatStore.assetsStore.showEmoteMenu ? ClipRect( - child: Column( - children: [ - const Divider(), - Expanded( - child: FrostyPageView( - headers: [ - 'Recent', - if (chatStore.settings.showTwitchEmotes) - 'Twitch', - if (chatStore.settings.show7TVEmotes) '7TV', - if (chatStore.settings.showBTTVEmotes) 'BTTV', - if (chatStore.settings.showFFZEmotes) 'FFZ', - ], - tabActions: { - if (chatStore - .assetsStore - .recentEmotes - .isNotEmpty) - 0: IconButton( - onPressed: () { - chatStore.assetsStore.recentEmotes.clear(); - chatStore.updateNotification( - 'Recent emotes cleared', - ); - }, - icon: const Icon( - Icons.delete_outline_rounded, + // Lay the content out at its full target height regardless of + // the animated container's current height, then clip — the + // Column would otherwise overflow (RenderFlex error) as the + // height tweens toward zero and can no longer fit the Divider. + child: OverflowBox( + minHeight: 0, + maxHeight: menuHeight, + alignment: Alignment.topCenter, + child: Column( + children: [ + const Divider(), + Expanded( + child: FrostyPageView( + headers: [ + 'Recent', + if (chatStore.settings.showTwitchEmotes) + 'Twitch', + if (chatStore.settings.show7TVEmotes) '7TV', + if (chatStore.settings.showBTTVEmotes) 'BTTV', + if (chatStore.settings.showFFZEmotes) 'FFZ', + ], + tabActions: { + if (chatStore + .assetsStore + .recentEmotes + .isNotEmpty) + 0: IconButton( + onPressed: () { + chatStore.assetsStore.recentEmotes.clear(); + chatStore.updateNotification( + 'Recent emotes cleared', + ); + }, + icon: const Icon( + Icons.delete_outline_rounded, + ), + tooltip: 'Clear recent emotes', + iconSize: 20, + ), + }, + children: [ + RecentEmotesPanel(chatStore: chatStore), + if (chatStore.settings.showTwitchEmotes) + EmoteMenuPanel( + chatStore: chatStore, + twitchEmotes: chatStore + .assetsStore + .userEmoteSectionToEmotes, + ), + ...[ + if (chatStore.settings.show7TVEmotes) + chatStore.assetsStore.sevenTVEmotes, + if (chatStore.settings.showBTTVEmotes) + chatStore.assetsStore.bttvEmotes, + if (chatStore.settings.showFFZEmotes) + chatStore.assetsStore.ffzEmotes, + ].map( + (emotes) => EmoteMenuPanel( + chatStore: chatStore, + emotes: emotes, ), - tooltip: 'Clear recent emotes', - iconSize: 20, ), - }, - children: [ - RecentEmotesPanel(chatStore: chatStore), - if (chatStore.settings.showTwitchEmotes) - EmoteMenuPanel( - chatStore: chatStore, - twitchEmotes: chatStore - .assetsStore - .userEmoteSectionToEmotes, - ), - ...[ - if (chatStore.settings.show7TVEmotes) - chatStore.assetsStore.sevenTVEmotes, - if (chatStore.settings.showBTTVEmotes) - chatStore.assetsStore.bttvEmotes, - if (chatStore.settings.showFFZEmotes) - chatStore.assetsStore.ffzEmotes, - ].map( - (emotes) => EmoteMenuPanel( - chatStore: chatStore, - emotes: emotes, - ), - ), - ], + ], + ), ), - ), - ], + ], + ), ), ) : null, diff --git a/lib/screens/channel/chat/stores/chat_store.dart b/lib/screens/channel/chat/stores/chat_store.dart index d0f9ed6e..750315b8 100644 --- a/lib/screens/channel/chat/stores/chat_store.dart +++ b/lib/screens/channel/chat/stores/chat_store.dart @@ -651,7 +651,7 @@ abstract class ChatStoreBase with Store { }, onBadgeError: (error) { debugPrint(error.toString()); - return []; + return {}; }, showTwitchEmotes: settings.showTwitchEmotes, showTwitchBadges: settings.showTwitchBadges, @@ -798,9 +798,12 @@ abstract class ChatStoreBase with Store { return []; } - List onBadgeError(dynamic error) { + // Returns a Map, not a List: this is attached via `.catchError` to the + // `getBadgesChannel` future (a Future>), so the + // handler must return that type or Dart throws at runtime. + Map onBadgeError(dynamic error) { debugPrint(error.toString()); - return []; + return {}; } final baseAssets = assetsStore.assetsFuture( diff --git a/lib/screens/channel/video/video_store.dart b/lib/screens/channel/video/video_store.dart index d75a2718..f74d58ce 100644 --- a/lib/screens/channel/video/video_store.dart +++ b/lib/screens/channel/video/video_store.dart @@ -340,11 +340,10 @@ abstract class VideoStoreBase with Store implements VideoPlayerInterface { // On Android, enable auto PiP mode (setAutoEnterEnabled) if the device supports it. if (Platform.isAndroid) { _disposeAndroidAutoPipReaction = autorun((_) async { - if (settingsStore.showVideo && await SimplePip.isAutoPipAvailable) { - pip.setAutoPipMode(); - } else { - pip.setAutoPipMode(autoEnter: false); - } + // setAutoPipMode is only implemented on Android S+ — invoking it on + // older versions throws a PlatformException, so skip when unavailable. + if (!await SimplePip.isAutoPipAvailable) return; + pip.setAutoPipMode(autoEnter: settingsStore.showVideo); }); } diff --git a/test/models/irc_test.dart b/test/models/irc_test.dart index f37a39eb..56b1cf99 100644 --- a/test/models/irc_test.dart +++ b/test/models/irc_test.dart @@ -622,6 +622,24 @@ void main() { expect(msg.localEmotes, isNotNull); expect(msg.localEmotes!.containsKey('Kappa'), isTrue); }); + + test('returns Command.none for a truncated line without a command', () { + // A malformed/truncated line (e.g. from the recent-messages history API) + // whose message section is a single token used to throw a RangeError on + // splitMessage[1]. It must now parse safely as a none command. + final msg = IRCMessage.fromString('@historical=1 :justaprefix'); + + expect(msg.command, Command.none); + expect(msg.tags['historical'], '1'); + }); + + test('returns Command.none for a line with no separating space', () { + // No space means there are no tags to slice and no body to parse; must + // not throw on the substring/split calls. + final msg = IRCMessage.fromString('@onlytags'); + + expect(msg.command, Command.none); + }); }); group('USERSTATE', () {