diff --git a/packages/flutter/lib/src/services/hardware_keyboard.dart b/packages/flutter/lib/src/services/hardware_keyboard.dart index e782fed0f4..dca3daac4b 100644 --- a/packages/flutter/lib/src/services/hardware_keyboard.dart +++ b/packages/flutter/lib/src/services/hardware_keyboard.dart @@ -514,18 +514,19 @@ class HardwareKeyboard { final bool thisResult = handler(event); handled = handled || thisResult; } catch (exception, stack) { + InformationCollector? collector; + assert(() { + collector = () sync* { + yield DiagnosticsProperty('Event', event); + }; + return true; + }()); FlutterError.reportError(FlutterErrorDetails( exception: exception, stack: stack, library: 'services library', - context: ErrorDescription('while dispatching notifications for $runtimeType'), - informationCollector: () sync* { - yield DiagnosticsProperty( - 'The $runtimeType sending notification was', - this, - style: DiagnosticsTreeStyle.errorProperty, - ); - }, + context: ErrorDescription('while processing a key handler'), + informationCollector: collector, )); } } @@ -718,15 +719,20 @@ class KeyEventManager { /// Key messages received from the platform are first sent to [RawKeyboard]'s /// listeners and [HardwareKeyboard]'s handlers, then sent to /// [keyMessageHandler], regardless of the results of [HardwareKeyboard]'s - /// handlers. The result from the handlers and [keyMessageHandler] are - /// combined and returned to the platform. The handler result is explained below. + /// handlers. The event results from the handlers and [keyMessageHandler] are + /// combined and returned to the platform. The event result is explained + /// below. /// - /// This handler is normally set by the [FocusManager] so that it can control - /// the key event propagation to focused widgets. Applications that use the - /// focus system (see [Focus] and [FocusManager]) to receive key events - /// do not need to set this field. + /// For most common applications, which use [WidgetsBinding], this field + /// is set by the focus system (see `FocusManger`) on startup and should not + /// be change explicitly. /// - /// ## Handler result + /// If you are not using the focus system to manage focus, set this + /// attribute to a [KeyMessageHandler] that returns true if the propagation + /// on the platform should not be continued. If this field is null, key events + /// will be assumed to not have been handled by Flutter. + /// + /// ## Event result /// /// Key messages on the platform are given to Flutter to be handled by the /// engine. If they are not handled, then the platform will continue to @@ -738,20 +744,14 @@ class KeyEventManager { /// is not handled by other controls either (such as the "bonk" noise on /// macOS). /// - /// If you are not using the [FocusManager] to manage focus, set this - /// attribute to a [KeyMessageHandler] that returns true if the propagation - /// on the platform should not be continued. Otherwise, key events will be - /// assumed to not have been handled by Flutter, and will also be sent to - /// other (possibly non-Flutter) controls in the application. + /// The result from [keyMessageHandler] and [HardwareKeyboard]'s handlers + /// are combined. If any of the handlers claim to handle the event, + /// the overall result will be "event handled". /// /// See also: /// - /// * [Focus.onKeyEvent], a [Focus] callback attribute that will be given - /// key events distributed by the [FocusManager] based on the current - /// primary focus. - /// * [HardwareKeyboard.addHandler], which accepts multiple handlers to - /// control the handler result but only accepts [KeyEvent] instead of - /// [KeyMessage]. + /// * [HardwareKeyboard.addHandler], which accepts multiple global handlers + /// to process [KeyEvent]s KeyMessageHandler? keyMessageHandler; final HardwareKeyboard _hardwareKeyboard; @@ -827,7 +827,25 @@ class KeyEventManager { } if (keyMessageHandler != null) { - handled = keyMessageHandler!(KeyMessage(_keyEventsSinceLastMessage, rawEvent)) || handled; + final KeyMessage message = KeyMessage(_keyEventsSinceLastMessage, rawEvent); + try { + handled = keyMessageHandler!(message) || handled; + } catch (exception, stack) { + InformationCollector? collector; + assert(() { + collector = () sync* { + yield DiagnosticsProperty('KeyMessage', message); + }; + return true; + }()); + FlutterError.reportError(FlutterErrorDetails( + exception: exception, + stack: stack, + library: 'services library', + context: ErrorDescription('while processing the key message handler'), + informationCollector: collector, + )); + } } _keyEventsSinceLastMessage.clear(); diff --git a/packages/flutter/lib/src/services/raw_keyboard.dart b/packages/flutter/lib/src/services/raw_keyboard.dart index d04e592dd4..46501ff37c 100644 --- a/packages/flutter/lib/src/services/raw_keyboard.dart +++ b/packages/flutter/lib/src/services/raw_keyboard.dart @@ -606,7 +606,7 @@ class RawKeyboard { /// This property is only a wrapper over [KeyEventManager.keyMessageHandler], /// and is kept only for backward compatibility. New code should use /// [KeyEventManager.keyMessageHandler] to set custom global key event - /// handler. Setting [keyEventHandler] will cause + /// handler. Setting [keyEventHandler] will cause /// [KeyEventManager.keyMessageHandler] to be set with a converted handler. /// If [KeyEventManager.keyMessageHandler] is set by [FocusManager] (the most /// common situation), then the exact value of [keyEventHandler] is a dummy @@ -673,8 +673,25 @@ class RawKeyboard { ); // Send the event to passive listeners. for (final ValueChanged listener in List>.from(_listeners)) { - if (_listeners.contains(listener)) { - listener(event); + try { + if (_listeners.contains(listener)) { + listener(event); + } + } catch (exception, stack) { + InformationCollector? collector; + assert(() { + collector = () sync* { + yield DiagnosticsProperty('Event', event); + }; + return true; + }()); + FlutterError.reportError(FlutterErrorDetails( + exception: exception, + stack: stack, + library: 'services library', + context: ErrorDescription('while processing a raw key listener'), + informationCollector: collector, + )); } } diff --git a/packages/flutter/test/services/hardware_keyboard_test.dart b/packages/flutter/test/services/hardware_keyboard_test.dart index da766b8a43..806ff17da6 100644 --- a/packages/flutter/test/services/hardware_keyboard_test.dart +++ b/packages/flutter/test/services/hardware_keyboard_test.dart @@ -250,4 +250,107 @@ void main() { expect(events.length, 1); expect(rawEvents.length, 2); }); + + testWidgets('Exceptions from keyMessageHandler are caught and reported', (WidgetTester tester) async { + final KeyMessageHandler? oldKeyMessageHandler = tester.binding.keyEventManager.keyMessageHandler; + addTearDown(() { + tester.binding.keyEventManager.keyMessageHandler = oldKeyMessageHandler; + }); + + // When keyMessageHandler throws an error... + tester.binding.keyEventManager.keyMessageHandler = (KeyMessage message) { + throw 1; + }; + + // Simulate a key down event. + FlutterErrorDetails? record; + await _runWhileOverridingOnError( + () => simulateKeyDownEvent(LogicalKeyboardKey.keyA), + onError: (FlutterErrorDetails details) { + record = details; + } + ); + + // ... the error should be caught. + expect(record, isNotNull); + expect(record!.exception, 1); + final Map infos = _groupDiagnosticsByName(record!.informationCollector!()); + expect(infos['KeyMessage'], isA>()); + + // But the exception should not interrupt recording the state. + // Now the keyMessageHandler no longer throws an error. + tester.binding.keyEventManager.keyMessageHandler = null; + record = null; + + // Simulate a key up event. + await _runWhileOverridingOnError( + () => simulateKeyUpEvent(LogicalKeyboardKey.keyA), + onError: (FlutterErrorDetails details) { + record = details; + } + ); + // If the previous state (key down) wasn't recorded, this key up event will + // trigger assertions. + expect(record, isNull); + }); + + testWidgets('Exceptions from HardwareKeyboard handlers are caught and reported', (WidgetTester tester) async { + bool throwingCallback(KeyEvent event) { + throw 1; + } + + // When the handler throws an error... + HardwareKeyboard.instance.addHandler(throwingCallback); + + // Simulate a key down event. + FlutterErrorDetails? record; + await _runWhileOverridingOnError( + () => simulateKeyDownEvent(LogicalKeyboardKey.keyA), + onError: (FlutterErrorDetails details) { + record = details; + } + ); + + // ... the error should be caught. + expect(record, isNotNull); + expect(record!.exception, 1); + final Map infos = _groupDiagnosticsByName(record!.informationCollector!()); + expect(infos['Event'], isA>()); + + // But the exception should not interrupt recording the state. + // Now the key handler no longer throws an error. + HardwareKeyboard.instance.removeHandler(throwingCallback); + record = null; + + // Simulate a key up event. + await _runWhileOverridingOnError( + () => simulateKeyUpEvent(LogicalKeyboardKey.keyA), + onError: (FlutterErrorDetails details) { + record = details; + } + ); + // If the previous state (key down) wasn't recorded, this key up event will + // trigger assertions. + expect(record, isNull); + }, variant: KeySimulatorTransitModeVariant.all()); +} + + + +Future _runWhileOverridingOnError(AsyncCallback body, {required FlutterExceptionHandler onError}) async { + final FlutterExceptionHandler? oldFlutterErrorOnError = FlutterError.onError; + FlutterError.onError = onError; + + try { + await body(); + } finally { + FlutterError.onError = oldFlutterErrorOnError; + } +} + +Map _groupDiagnosticsByName(Iterable infos) { + return Map.fromIterable( + infos, + key: (dynamic node) => (node as DiagnosticsNode).name ?? '', + ); } diff --git a/packages/flutter/test/services/raw_keyboard_test.dart b/packages/flutter/test/services/raw_keyboard_test.dart index aec3be993a..6e2ce81a35 100644 --- a/packages/flutter/test/services/raw_keyboard_test.dart +++ b/packages/flutter/test/services/raw_keyboard_test.dart @@ -753,6 +753,46 @@ void main() { expect(logs, [1, 3, 2]); logs.clear(); }, variant: KeySimulatorTransitModeVariant.all()); + + testWidgets('Exceptions from RawKeyboard listeners are caught and reported', (WidgetTester tester) async { + void throwingListener(RawKeyEvent event) { + throw 1; + } + + // When the listener throws an error... + RawKeyboard.instance.addListener(throwingListener); + + // Simulate a key down event. + FlutterErrorDetails? record; + await _runWhileOverridingOnError( + () => simulateKeyDownEvent(LogicalKeyboardKey.keyA), + onError: (FlutterErrorDetails details) { + record = details; + } + ); + + // ... the error should be caught. + expect(record, isNotNull); + expect(record!.exception, 1); + final Map infos = _groupDiagnosticsByName(record!.informationCollector!()); + expect(infos['Event'], isA>()); + + // But the exception should not interrupt recording the state. + // Now the key handler no longer throws an error. + RawKeyboard.instance.removeListener(throwingListener); + record = null; + + // Simulate a key up event. + await _runWhileOverridingOnError( + () => simulateKeyUpEvent(LogicalKeyboardKey.keyA), + onError: (FlutterErrorDetails details) { + record = details; + } + ); + // If the previous state (key down) wasn't recorded, this key up event will + // trigger assertions. + expect(record, isNull); + }); }); group('RawKeyEventDataAndroid', () { @@ -2504,3 +2544,21 @@ void main() { }); }); } + +Future _runWhileOverridingOnError(AsyncCallback body, {required FlutterExceptionHandler onError}) async { + final FlutterExceptionHandler? oldFlutterErrorOnError = FlutterError.onError; + FlutterError.onError = onError; + + try { + await body(); + } finally { + FlutterError.onError = oldFlutterErrorOnError; + } +} + +Map _groupDiagnosticsByName(Iterable infos) { + return Map.fromIterable( + infos, + key: (dynamic node) => (node as DiagnosticsNode).name ?? '', + ); +}