firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Report exceptions from key event handlers and listeners (#91792)

Browse Source 
This PR ensures that all 3 ways of handling key events are wrapped with exception handling logic, so that user exceptions do not cause unstable states and are correctly reported.
This commit is contained in:
Tong Mu 2021-10-20 12:08:24 -07:00 committed by GitHub
parent f84a2dc7bb
commit 5f4ac10d16
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 226 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

72
packages/flutter/lib/src/services/hardware_keyboard.dart
View File

@ -514,18 +514,19 @@ class HardwareKeyboard {
final bool thisResult = handler(event);
handled = handled || thisResult;
} catch (exception, stack) {
InformationCollector? collector;
assert(() {
collector = () sync* {
yield DiagnosticsProperty<KeyEvent>('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<HardwareKeyboard>(
'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>('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();

23
packages/flutter/lib/src/services/raw_keyboard.dart
View File

@ -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<RawKeyEvent> listener in List<ValueChanged<RawKeyEvent>>.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<RawKeyEvent>('Event', event);
};
return true;
}());
FlutterError.reportError(FlutterErrorDetails(
exception: exception,
stack: stack,
library: 'services library',
context: ErrorDescription('while processing a raw key listener'),
informationCollector: collector,
));
}
}

103
packages/flutter/test/services/hardware_keyboard_test.dart
View File

@ -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<String, DiagnosticsNode> infos = _groupDiagnosticsByName(record!.informationCollector!());
expect(infos['KeyMessage'], isA<DiagnosticsProperty<KeyMessage>>());
// 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<String, DiagnosticsNode> infos = _groupDiagnosticsByName(record!.informationCollector!());
expect(infos['Event'], isA<DiagnosticsProperty<KeyEvent>>());
// 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<void> _runWhileOverridingOnError(AsyncCallback body, {required FlutterExceptionHandler onError}) async {
final FlutterExceptionHandler? oldFlutterErrorOnError = FlutterError.onError;
FlutterError.onError = onError;
try {
await body();
} finally {
FlutterError.onError = oldFlutterErrorOnError;
}
}
Map<String, DiagnosticsNode> _groupDiagnosticsByName(Iterable<DiagnosticsNode> infos) {
return Map<String, DiagnosticsNode>.fromIterable(
infos,
key: (dynamic node) => (node as DiagnosticsNode).name ?? '',
);
}

58
packages/flutter/test/services/raw_keyboard_test.dart
View File

@ -753,6 +753,46 @@ void main() {
expect(logs, <int>[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<String, DiagnosticsNode> infos = _groupDiagnosticsByName(record!.informationCollector!());
expect(infos['Event'], isA<DiagnosticsProperty<RawKeyEvent>>());
// 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<void> _runWhileOverridingOnError(AsyncCallback body, {required FlutterExceptionHandler onError}) async {
final FlutterExceptionHandler? oldFlutterErrorOnError = FlutterError.onError;
FlutterError.onError = onError;
try {
await body();
} finally {
FlutterError.onError = oldFlutterErrorOnError;
}
}
Map<String, DiagnosticsNode> _groupDiagnosticsByName(Iterable<DiagnosticsNode> infos) {
return Map<String, DiagnosticsNode>.fromIterable(
infos,
key: (dynamic node) => (node as DiagnosticsNode).name ?? '',
);
}