From b75abd9f7efb29c78ac414fcaccd4f27f35ffc7d Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Tue, 19 Nov 2019 17:35:47 -0800 Subject: [PATCH] Try a mildly prettier FlutterError and make it less drastic in release mode. (#44967) --- .../lib/src/foundation/assertions.dart | 5 + packages/flutter/lib/src/rendering/error.dart | 99 +++++++++++++------ .../flutter/lib/src/widgets/framework.dart | 83 +++++++++++++++- .../flutter/test/rendering/error_test.dart | 40 +++++++- .../widgets/error_widget_builder_test.dart | 21 ++++ 5 files changed, 213 insertions(+), 35 deletions(-) diff --git a/packages/flutter/lib/src/foundation/assertions.dart b/packages/flutter/lib/src/foundation/assertions.dart index 50eabff8e6..510f7ba6fe 100644 --- a/packages/flutter/lib/src/foundation/assertions.dart +++ b/packages/flutter/lib/src/foundation/assertions.dart @@ -479,6 +479,11 @@ class FlutterErrorDetails extends Diagnosticable { /// Error class used to report Flutter-specific assertion failures and /// contract violations. +/// +/// See also: +/// +/// * , more information about error +/// handling in Flutter. class FlutterError extends Error with DiagnosticableTreeMixin implements AssertionError { /// Create an error message from a string. /// diff --git a/packages/flutter/lib/src/rendering/error.dart b/packages/flutter/lib/src/rendering/error.dart index d34c5f4f7b..966be17f1c 100644 --- a/packages/flutter/lib/src/rendering/error.dart +++ b/packages/flutter/lib/src/rendering/error.dart @@ -10,9 +10,6 @@ import 'object.dart'; const double _kMaxWidth = 100000.0; const double _kMaxHeight = 100000.0; -// Line length to fit small phones without dynamically checking size. -const String _kLine = '\n\n────────────────────\n\n'; - /// A render object used as a placeholder when an error occurs. /// /// The box will be painted in the color given by the @@ -25,8 +22,9 @@ const String _kLine = '\n\n───────────────── /// [RenderErrorBox.textStyle] and [RenderErrorBox.paragraphStyle] static /// properties. /// -/// Again to help simplify the class, this box tries to be 100000.0 pixels wide -/// and high, to approximate being infinitely high but without using infinities. +/// Again to help simplify the class, if the parent has left the constraints +/// unbounded, this box tries to be 100000.0 pixels wide and high, to +/// approximate being infinitely high but without using infinities. class RenderErrorBox extends RenderBox { /// Creates a RenderErrorBox render object. /// @@ -45,13 +43,10 @@ class RenderErrorBox extends RenderBox { // see the paragraph.dart file and the RenderParagraph class. final ui.ParagraphBuilder builder = ui.ParagraphBuilder(paragraphStyle); builder.pushStyle(textStyle); - builder.addText( - '$message$_kLine$message$_kLine$message$_kLine$message$_kLine$message$_kLine$message$_kLine' - '$message$_kLine$message$_kLine$message$_kLine$message$_kLine$message$_kLine$message' - ); + builder.addText(message); _paragraph = builder.build(); } - } catch (e) { + } catch (error) { // Intentionally left empty. } } @@ -82,39 +77,87 @@ class RenderErrorBox extends RenderBox { size = constraints.constrain(const Size(_kMaxWidth, _kMaxHeight)); } + /// The distance to place around the text. + /// + /// This is intended to ensure that if the [RenderErrorBox] is placed at the top left + /// of the screen, under the system's status bar, the error text is still visible in + /// the area below the status bar. + /// + /// The padding is ignored if the error box is smaller than the padding. + /// + /// See also: + /// + /// * [minimumWidth], which controls how wide the box must be before the + // horizontal padding is applied. + static EdgeInsets padding = const EdgeInsets.fromLTRB(64.0, 96.0, 64.0, 12.0); + + /// The width below which the horizontal padding is not applied. + /// + /// If the left and right padding would reduce the available width to less than + /// this value, then the text is rendered flush with the left edge. + static double minimumWidth = 200.0; + /// The color to use when painting the background of [RenderErrorBox] objects. - static Color backgroundColor = const Color(0xF0900000); + /// + /// Defaults to red in debug mode, a light gray otherwise. + static Color backgroundColor = _initBackgroundColor(); + + static Color _initBackgroundColor() { + Color result = const Color(0xF0C0C0C0); + assert(() { + result = const Color(0xF0900000); + return true; + }()); + return result; + } /// The text style to use when painting [RenderErrorBox] objects. - static ui.TextStyle textStyle = ui.TextStyle( - color: const Color(0xFFFFFF66), - fontFamily: 'monospace', - fontSize: 14.0, - fontWeight: FontWeight.bold, - ); + /// + /// Defaults to a yellow monospace font in debug mode, and a dark gray + /// sans-serif font otherwise. + static ui.TextStyle textStyle = _initTextStyle(); + + static ui.TextStyle _initTextStyle() { + ui.TextStyle result = ui.TextStyle( + color: const Color(0xFF303030), + fontFamily: 'sans-serif', + fontSize: 18.0, + ); + assert(() { + result = ui.TextStyle( + color: const Color(0xFFFFFF66), + fontFamily: 'monospace', + fontSize: 14.0, + fontWeight: FontWeight.bold, + ); + return true; + }()); + return result; + } /// The paragraph style to use when painting [RenderErrorBox] objects. static ui.ParagraphStyle paragraphStyle = ui.ParagraphStyle( - height: 1.0, + textDirection: TextDirection.ltr, + textAlign: TextAlign.left, ); @override void paint(PaintingContext context, Offset offset) { try { context.canvas.drawRect(offset & size, Paint() .. color = backgroundColor); - double width; if (_paragraph != null) { - // See the comment in the RenderErrorBox constructor. This is not the - // code you want to be copying and pasting. :-) - if (parent is RenderBox) { - final RenderBox parentBox = parent; - width = parentBox.size.width; - } else { - width = size.width; + double width = size.width; + double left = 0.0; + double top = 0.0; + if (width > padding.left + minimumWidth + padding.right) { + width -= padding.left + padding.right; + left += padding.left; } _paragraph.layout(ui.ParagraphConstraints(width: width)); - - context.canvas.drawParagraph(_paragraph, offset); + if (size.height > padding.top + _paragraph.height + padding.bottom) { + top += padding.top; + } + context.canvas.drawParagraph(_paragraph, offset + Offset(left, top)); } } catch (e) { // Intentionally left empty. diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart index f46e282bce..2d78cd40fa 100644 --- a/packages/flutter/lib/src/widgets/framework.dart +++ b/packages/flutter/lib/src/widgets/framework.dart @@ -3810,13 +3810,71 @@ typedef ErrorWidgetBuilder = Widget Function(FlutterErrorDetails details); /// where the problem lies. Exceptions are also logged to the console, which you /// can read using `flutter logs`. The console will also include additional /// information such as the stack trace for the exception. +/// +/// It is possible to override this widget. +/// +/// {@tool snippet --template=freeform} +/// ```dart +/// import 'package:flutter/material.dart'; +/// +/// void main() { +/// ErrorWidget.builder = (FlutterErrorDetails details) { +/// bool inDebug = false; +/// assert(() { inDebug = true; return true; }()); +/// // In debug mode, use the normal error widget which shows +/// // the error message: +/// if (inDebug) +/// return ErrorWidget(details.exception); +/// // In release builds, show a yellow-on-blue message instead: +/// return Container( +/// alignment: Alignment.center, +/// child: Text( +/// 'Error!', +/// style: TextStyle(color: Colors.yellow), +/// textDirection: TextDirection.ltr, +/// ), +/// ); +/// }; +/// // Here we would normally runApp() the root widget, but to demonstrate +/// // the error handling we artificially fail: +/// return runApp(Builder( +/// builder: (BuildContext context) { +/// throw 'oh no, an error'; +/// }, +/// )); +/// } +/// ``` +/// {@end-tool} +/// +/// See also: +/// +/// * [FlutterError.onError], which can be set to a method that exits the +/// application if that is preferable to showing an error message. +/// * , more information about error +/// handling in Flutter. class ErrorWidget extends LeafRenderObjectWidget { - /// Creates a widget that displays the given error message. + /// Creates a widget that displays the given exception. + /// + /// The message will be the stringification of the given exception, unless + /// computing that value itself throws an exception, in which case it will + /// be the string "Error". + /// + /// If this object is inspected from an IDE or the devtools, and the original + /// exception is a [FlutterError] object, the original exception itself will + /// be shown in the inspection output. ErrorWidget(Object exception) : message = _stringify(exception), _flutterError = exception is FlutterError ? exception : null, super(key: UniqueKey()); + /// Creates a widget that displays the given error message. + /// + /// An explicit [FlutterError] can be provided to be reported to inspection + /// tools. It need not match the message. + ErrorWidget.withDetails({ this.message = '', FlutterError error }) + : _flutterError = error, + super(key: UniqueKey()); + /// The configurable factory for [ErrorWidget]. /// /// When an error occurs while building a widget, the broken widget is @@ -3835,17 +3893,28 @@ class ErrorWidget extends LeafRenderObjectWidget { /// corresponds to a [RenderBox] that can handle the most absurd of incoming /// constraints. The default constructor maps to a [RenderErrorBox]. /// + /// The default behavior is to show the exception's message in debug mode, + /// and to show nothing but a gray background in release builds. + /// /// See also: /// /// * [FlutterError.onError], which is typically called with the same /// [FlutterErrorDetails] object immediately prior to this callback being /// invoked, and which can also be configured to control how errors are /// reported. - static ErrorWidgetBuilder builder = (FlutterErrorDetails details) => ErrorWidget(details.exception); + /// * , more information about error + /// handling in Flutter. + static ErrorWidgetBuilder builder = _defaultErrorWidgetBuilder; - /// The message to display. - final String message; - final FlutterError _flutterError; + static Widget _defaultErrorWidgetBuilder(FlutterErrorDetails details) { + String message = ''; + assert(() { + message = _stringify(details.exception) + '\nSee also: https://flutter.dev/docs/testing/errors'; + return true; + }()); + final Object exception = details.exception; + return ErrorWidget.withDetails(message: message, error: exception is FlutterError ? exception : null); + } static String _stringify(Object exception) { try { @@ -3856,6 +3925,10 @@ class ErrorWidget extends LeafRenderObjectWidget { return 'Error'; } + /// The message to display. + final String message; + final FlutterError _flutterError; + @override RenderBox createRenderObject(BuildContext context) => RenderErrorBox(message); diff --git a/packages/flutter/test/rendering/error_test.dart b/packages/flutter/test/rendering/error_test.dart index 565faf1ac0..8efa4c0eb7 100644 --- a/packages/flutter/test/rendering/error_test.dart +++ b/packages/flutter/test/rendering/error_test.dart @@ -14,9 +14,45 @@ void main() { testWidgets('test draw error paragraph', (WidgetTester tester) async { await tester.pumpWidget(ErrorWidget(Exception(errorMessage))); - expect(find.byType(ErrorWidget), paints ..rect(rect: const Rect.fromLTWH(0.0, 0.0, 800.0, 600.0)) - ..paragraph(offset: Offset.zero)); + ..paragraph(offset: const Offset(64.0, 96.0))); + + final Widget _error = Builder(builder: (BuildContext context) => throw 'pillow'); + + await tester.pumpWidget(Center(child: SizedBox(width: 100.0, child: _error))); + expect(tester.takeException(), 'pillow'); + expect(find.byType(ErrorWidget), paints + ..rect(rect: const Rect.fromLTWH(0.0, 0.0, 100.0, 600.0)) + ..paragraph(offset: const Offset(0.0, 96.0))); + + await tester.pumpWidget(Center(child: SizedBox(height: 100.0, child: _error))); + expect(tester.takeException(), null); + + await tester.pumpWidget(Center(child: SizedBox(key: UniqueKey(), height: 100.0, child: _error))); + expect(tester.takeException(), 'pillow'); + expect(find.byType(ErrorWidget), paints + ..rect(rect: const Rect.fromLTWH(0.0, 0.0, 800.0, 100.0)) + ..paragraph(offset: const Offset(64.0, 0.0))); + + RenderErrorBox.minimumWidth = 800.0; + await tester.pumpWidget(Center(child: _error)); + expect(tester.takeException(), 'pillow'); + expect(find.byType(ErrorWidget), paints + ..rect(rect: const Rect.fromLTWH(0.0, 0.0, 800.0, 600.0)) + ..paragraph(offset: const Offset(0.0, 96.0))); + + await tester.pumpWidget(Center(child: _error)); + expect(tester.takeException(), null); + expect(find.byType(ErrorWidget), paints + ..rect(color: const Color(0xF0900000)) + ..paragraph()); + + RenderErrorBox.backgroundColor = const Color(0xFF112233); + await tester.pumpWidget(Center(child: _error)); + expect(tester.takeException(), null); + expect(find.byType(ErrorWidget), paints + ..rect(color: const Color(0xFF112233)) + ..paragraph()); }); } diff --git a/packages/flutter/test/widgets/error_widget_builder_test.dart b/packages/flutter/test/widgets/error_widget_builder_test.dart index eda17850fd..c52a2d4a5a 100644 --- a/packages/flutter/test/widgets/error_widget_builder_test.dart +++ b/packages/flutter/test/widgets/error_widget_builder_test.dart @@ -5,6 +5,8 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:flutter/widgets.dart'; +import '../rendering/mock_canvas.dart'; + void main() { testWidgets('ErrorWidget.builder', (WidgetTester tester) async { final ErrorWidgetBuilder oldBuilder = ErrorWidget.builder; @@ -24,4 +26,23 @@ void main() { expect(find.text('oopsie!'), findsOneWidget); ErrorWidget.builder = oldBuilder; }); + + testWidgets('ErrorWidget.builder', (WidgetTester tester) async { + final ErrorWidgetBuilder oldBuilder = ErrorWidget.builder; + ErrorWidget.builder = (FlutterErrorDetails details) { + return ErrorWidget(''); + }; + await tester.pumpWidget( + SizedBox( + child: Builder( + builder: (BuildContext context) { + throw 'test'; + }, + ), + ), + ); + expect(tester.takeException().toString(), 'test'); + expect(find.byType(ErrorWidget), isNot(paints..paragraph())); + ErrorWidget.builder = oldBuilder; + }); }