From a60e4d1f36b32cdb22b7ea535930fbbf62bc6e99 Mon Sep 17 00:00:00 2001 From: chunhtai <47866232+chunhtai@users.noreply.github.com> Date: Mon, 27 Apr 2020 14:49:03 -0700 Subject: [PATCH] Relands Fix FlutterError.onError in debug mode (#55500) --- .../lib/src/foundation/assertions.dart | 15 ++- .../lib/src/widgets/widget_inspector.dart | 6 +- ...widget_inspector_structure_error_test.dart | 91 +++++++++++++++++++ .../test/widgets/widget_inspector_test.dart | 22 +++-- 4 files changed, 123 insertions(+), 11 deletions(-) create mode 100644 packages/flutter/test/widgets/widget_inspector_structure_error_test.dart diff --git a/packages/flutter/lib/src/foundation/assertions.dart b/packages/flutter/lib/src/foundation/assertions.dart index b9774908b8f..84c4a189e22 100644 --- a/packages/flutter/lib/src/foundation/assertions.dart +++ b/packages/flutter/lib/src/foundation/assertions.dart @@ -773,7 +773,7 @@ class FlutterError extends Error with DiagnosticableTreeMixin implements Asserti /// Called whenever the Flutter framework catches an error. /// - /// The default behavior is to call [dumpErrorToConsole]. + /// The default behavior is to call [presentError]. /// /// You can set this to your own function to override this default behavior. /// For example, you could report all errors to your server. @@ -783,7 +783,18 @@ class FlutterError extends Error with DiagnosticableTreeMixin implements Asserti /// /// Set this to null to silently catch and ignore errors. This is not /// recommended. - static FlutterExceptionHandler onError = dumpErrorToConsole; + static FlutterExceptionHandler onError = (FlutterErrorDetails details) => presentError(details); + + /// Called whenever the Flutter framework wants to present an error to the + /// users. + /// + /// The default behavior is to call [dumpErrorToConsole]. + /// + /// Plugins can override how an error is to be presented to the user. For + /// example, the structured errors service extension sets its own method when + /// the extension is enabled. If you want to change how Flutter responds to an + /// error, use [onError] instead. + static FlutterExceptionHandler presentError = dumpErrorToConsole; static int _errorCount = 0; diff --git a/packages/flutter/lib/src/widgets/widget_inspector.dart b/packages/flutter/lib/src/widgets/widget_inspector.dart index 3e17698d0e2..361ff1e4856 100644 --- a/packages/flutter/lib/src/widgets/widget_inspector.dart +++ b/packages/flutter/lib/src/widgets/widget_inspector.dart @@ -959,13 +959,13 @@ mixin WidgetInspectorService { SchedulerBinding.instance.addPersistentFrameCallback(_onFrameStart); final FlutterExceptionHandler structuredExceptionHandler = _reportError; - final FlutterExceptionHandler defaultExceptionHandler = FlutterError.onError; + final FlutterExceptionHandler defaultExceptionHandler = FlutterError.presentError; _registerBoolServiceExtension( name: 'structuredErrors', - getter: () async => FlutterError.onError == structuredExceptionHandler, + getter: () async => FlutterError.presentError == structuredExceptionHandler, setter: (bool value) { - FlutterError.onError = value ? structuredExceptionHandler : defaultExceptionHandler; + FlutterError.presentError = value ? structuredExceptionHandler : defaultExceptionHandler; return Future.value(); }, ); diff --git a/packages/flutter/test/widgets/widget_inspector_structure_error_test.dart b/packages/flutter/test/widgets/widget_inspector_structure_error_test.dart new file mode 100644 index 00000000000..c87746d3bcf --- /dev/null +++ b/packages/flutter/test/widgets/widget_inspector_structure_error_test.dart @@ -0,0 +1,91 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:async'; +import 'dart:convert'; + +import 'package:flutter/foundation.dart'; +import 'package:flutter/rendering.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + StructureErrorTestWidgetInspectorService.runTests(); +} + +typedef InspectorServiceExtensionCallback = FutureOr> Function(Map parameters); + +class StructureErrorTestWidgetInspectorService extends Object with WidgetInspectorService { + final Map extensions = {}; + + final Map>> eventsDispatched = >>{}; + + @override + void registerServiceExtension({ + @required String name, + @required FutureOr> callback(Map parameters), + }) { + assert(!extensions.containsKey(name)); + extensions[name] = callback; + } + + @override + void postEvent(String eventKind, Map eventData) { + getEventsDispatched(eventKind).add(eventData); + } + + List> getEventsDispatched(String eventKind) { + return eventsDispatched.putIfAbsent(eventKind, () => >[]); + } + + Iterable> getServiceExtensionStateChangedEvents(String extensionName) { + return getEventsDispatched('Flutter.ServiceExtensionStateChanged') + .where((Map event) => event['extension'] == extensionName); + } + + Future testBoolExtension(String name, Map arguments) async { + expect(extensions, contains(name)); + // Encode and decode to JSON to match behavior using a real service + // extension where only JSON is allowed. + return json.decode(json.encode(await extensions[name](arguments)))['enabled'] as String; + } + + + static void runTests() { + final StructureErrorTestWidgetInspectorService service = StructureErrorTestWidgetInspectorService(); + WidgetInspectorService.instance = service; + + test('ext.flutter.inspector.structuredErrors still report error to original on error', () async { + final FlutterExceptionHandler oldHandler = FlutterError.onError; + + FlutterErrorDetails actualError; + // Creates a spy onError. This spy needs to be set before widgets binding + // initializes. + FlutterError.onError = (FlutterErrorDetails details) { + actualError = details; + }; + + WidgetsFlutterBinding.ensureInitialized(); + try { + // Enables structured errors. + expect(await service.testBoolExtension( + 'structuredErrors', {'enabled': 'true'}), + equals('true')); + + // Creates an error. + final FlutterErrorDetails expectedError = FlutterErrorDetailsForRendering( + library: 'rendering library', + context: ErrorDescription('during layout'), + exception: StackTrace.current, + ); + FlutterError.reportError(expectedError); + + // Validates the spy still received an error. + expect(actualError, expectedError); + } finally { + FlutterError.onError = oldHandler; + } + }); + } +} \ No newline at end of file diff --git a/packages/flutter/test/widgets/widget_inspector_test.dart b/packages/flutter/test/widgets/widget_inspector_test.dart index a0b5b5cd2b9..e57ddccf646 100644 --- a/packages/flutter/test/widgets/widget_inspector_test.dart +++ b/packages/flutter/test/widgets/widget_inspector_test.dart @@ -2276,11 +2276,11 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { ); }, skip: isBrowser); - testWidgets('ext.flutter.inspector.structuredErrors', (WidgetTester tester) async { + test('ext.flutter.inspector.structuredErrors', () async { List> flutterErrorEvents = service.getEventsDispatched('Flutter.Error'); expect(flutterErrorEvents, isEmpty); - final FlutterExceptionHandler oldHandler = FlutterError.onError; + final FlutterExceptionHandler oldHandler = FlutterError.presentError; try { // Enable structured errors. @@ -2320,9 +2320,19 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { error = flutterErrorEvents.last; expect(error['errorsSinceReload'], 1); - // Reload the app. - tester.binding.reassembleApplication(); - await tester.pump(); + // Reloads the app. + final FlutterExceptionHandler oldHandler = FlutterError.onError; + final TestWidgetsFlutterBinding binding = TestWidgetsFlutterBinding.ensureInitialized() as TestWidgetsFlutterBinding; + // We need the runTest to setup the fake async in the test binding. + await binding.runTest(() async { + binding.reassembleApplication(); + await binding.pump(); + }, () { }); + // The run test overrides the flutter error handler, so we should + // restore it back for the structure error to continue working. + FlutterError.onError = oldHandler; + // Cleans up the fake async so it does not bleed into next test. + binding.postTest(); // Send another error. FlutterError.reportError(FlutterErrorDetailsForRendering( @@ -2337,7 +2347,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { error = flutterErrorEvents.last; expect(error['errorsSinceReload'], 0); } finally { - FlutterError.onError = oldHandler; + FlutterError.presentError = oldHandler; } });