Commit ecf1cce8 authored by Ian Hickson's avatar Ian Hickson

Provide details when reporting invalid constraints (#3281)

This also shrinks the width of the error messages a bit because now that
we use 'package:' URLs the stacks are a bit narrower.
parent 3525aa1b
...@@ -43,7 +43,10 @@ class SectorConstraints extends Constraints { ...@@ -43,7 +43,10 @@ class SectorConstraints extends Constraints {
bool get isNormalized => minDeltaRadius <= maxDeltaRadius && minDeltaTheta <= maxDeltaTheta; bool get isNormalized => minDeltaRadius <= maxDeltaRadius && minDeltaTheta <= maxDeltaTheta;
@override @override
bool debugAssertIsValid({ bool isAppliedConstraint: false }) { bool debugAssertIsValid({
bool isAppliedConstraint: false,
InformationCollector informationCollector
}) {
assert(isNormalized); assert(isNormalized);
return isNormalized; return isNormalized;
} }
......
...@@ -134,7 +134,7 @@ class FlutterErrorDetailsForPointerEventDispatcher extends FlutterErrorDetails { ...@@ -134,7 +134,7 @@ class FlutterErrorDetailsForPointerEventDispatcher extends FlutterErrorDetails {
String context, String context,
this.event, this.event,
this.hitTestEntry, this.hitTestEntry,
FlutterInformationCollector informationCollector, InformationCollector informationCollector,
bool silent: false bool silent: false
}) : super( }) : super(
exception: exception, exception: exception,
......
...@@ -89,7 +89,7 @@ class FlutterErrorDetailsForPointerRouter extends FlutterErrorDetails { ...@@ -89,7 +89,7 @@ class FlutterErrorDetailsForPointerRouter extends FlutterErrorDetails {
this.router, this.router,
this.route, this.route,
this.event, this.event,
FlutterInformationCollector informationCollector, InformationCollector informationCollector,
bool silent: false bool silent: false
}) : super( }) : super(
exception: exception, exception: exception,
......
...@@ -303,8 +303,17 @@ class BoxConstraints extends Constraints { ...@@ -303,8 +303,17 @@ class BoxConstraints extends Constraints {
} }
@override @override
bool debugAssertIsValid({ bool isAppliedConstraint: false }) { bool debugAssertIsValid({
bool isAppliedConstraint: false,
InformationCollector informationCollector
}) {
assert(() { assert(() {
void throwError(String message) {
StringBuffer information = new StringBuffer();
if (informationCollector != null)
informationCollector(information);
throw new FlutterError('$message\n${information}The offending constraints were:\n $this');
}
if (minWidth.isNaN || maxWidth.isNaN || minHeight.isNaN || maxHeight.isNaN) { if (minWidth.isNaN || maxWidth.isNaN || minHeight.isNaN || maxHeight.isNaN) {
List<String> affectedFieldsList = <String>[]; List<String> affectedFieldsList = <String>[];
if (minWidth.isNaN) if (minWidth.isNaN)
...@@ -326,27 +335,27 @@ class BoxConstraints extends Constraints { ...@@ -326,27 +335,27 @@ class BoxConstraints extends Constraints {
} else { } else {
whichFields = affectedFieldsList.single; whichFields = affectedFieldsList.single;
} }
throw new FlutterError('BoxConstraints has ${affectedFieldsList.length == 1 ? 'a NaN value' : 'NaN values' } in $whichFields.\n$this'); throwError('BoxConstraints has ${affectedFieldsList.length == 1 ? 'a NaN value' : 'NaN values' } in $whichFields.');
} }
if (minWidth < 0.0 && minHeight < 0.0) if (minWidth < 0.0 && minHeight < 0.0)
throw new FlutterError('BoxConstraints has both a negative minimum width and a negative minimum height.\n$this'); throwError('BoxConstraints has both a negative minimum width and a negative minimum height.');
if (minWidth < 0.0) if (minWidth < 0.0)
throw new FlutterError('BoxConstraints has a negative minimum width.\n$this'); throwError('BoxConstraints has a negative minimum width.');
if (minHeight < 0.0) if (minHeight < 0.0)
throw new FlutterError('BoxConstraints has a negative minimum height.\n$this'); throwError('BoxConstraints has a negative minimum height.');
if (maxWidth < minWidth && maxHeight < minHeight) if (maxWidth < minWidth && maxHeight < minHeight)
throw new FlutterError('BoxConstraints has both width and height constraints non-normalized.\n$this'); throwError('BoxConstraints has both width and height constraints non-normalized.');
if (maxWidth < minWidth) if (maxWidth < minWidth)
throw new FlutterError('BoxConstraints has non-normalized width constraints.\n$this'); throwError('BoxConstraints has non-normalized width constraints.');
if (maxHeight < minHeight) if (maxHeight < minHeight)
throw new FlutterError('BoxConstraints has non-normalized height constraints.\n$this'); throwError('BoxConstraints has non-normalized height constraints.');
if (isAppliedConstraint) { if (isAppliedConstraint) {
if (minWidth.isInfinite && minHeight.isInfinite) if (minWidth.isInfinite && minHeight.isInfinite)
throw new FlutterError('BoxConstraints requires infinite width and infinite height.\n$this'); throwError('BoxConstraints forces an infinite width and infinite height.');
if (minWidth.isInfinite) if (minWidth.isInfinite)
throw new FlutterError('BoxConstraints requires infinite width.\n$this'); throwError('BoxConstraints forces an infinite width.');
if (minHeight.isInfinite) if (minHeight.isInfinite)
throw new FlutterError('BoxConstraints requires infinite height.\n$this'); throwError('BoxConstraints forces an infinite height.');
} }
assert(isNormalized); assert(isNormalized);
return true; return true;
......
...@@ -20,7 +20,7 @@ import 'binding.dart'; ...@@ -20,7 +20,7 @@ import 'binding.dart';
export 'package:flutter/gestures.dart' show HitTestEntry, HitTestResult; export 'package:flutter/gestures.dart' show HitTestEntry, HitTestResult;
export 'package:flutter/painting.dart'; export 'package:flutter/painting.dart';
export 'package:flutter/services.dart' show FlutterError; export 'package:flutter/services.dart' show FlutterError, InformationCollector;
/// Base class for data associated with a [RenderObject] by its parent. /// Base class for data associated with a [RenderObject] by its parent.
/// ///
...@@ -389,7 +389,7 @@ abstract class Constraints { ...@@ -389,7 +389,7 @@ abstract class Constraints {
/// For example, the [BoxConstraints] subclass verifies that the /// For example, the [BoxConstraints] subclass verifies that the
/// constraints are not [NaN]. /// constraints are not [NaN].
/// ///
/// If the [isAppliedConstraint] argument is true, then even /// If the `isAppliedConstraint` argument is true, then even
/// stricter rules are enforced. This argument is set to true when /// stricter rules are enforced. This argument is set to true when
/// checking constraints that are about to be applied to a /// checking constraints that are about to be applied to a
/// [RenderObject] during layout, as opposed to constraints that may /// [RenderObject] during layout, as opposed to constraints that may
...@@ -399,8 +399,16 @@ abstract class Constraints { ...@@ -399,8 +399,16 @@ abstract class Constraints {
/// argument, but the asserts for verifying the argument passed to /// argument, but the asserts for verifying the argument passed to
/// the [layout] method do. /// the [layout] method do.
/// ///
/// The `informationCollector` argument takes an optional callback
/// which is called when an exception is to be thrown. The collected
/// information is then included in the message after the error
/// line.
///
/// Returns the same as [isNormalized] if asserts are disabled. /// Returns the same as [isNormalized] if asserts are disabled.
bool debugAssertIsValid({ bool isAppliedConstraint: false }); bool debugAssertIsValid({
bool isAppliedConstraint: false,
InformationCollector informationCollector
});
} }
typedef void RenderObjectVisitor(RenderObject child); typedef void RenderObjectVisitor(RenderObject child);
...@@ -900,13 +908,13 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { ...@@ -900,13 +908,13 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
context: 'during $method()', context: 'during $method()',
renderObject: this, renderObject: this,
informationCollector: (StringBuffer information) { informationCollector: (StringBuffer information) {
information.writeln('The following RenderObject was being processed when the exception was fired:\n${this}'); information.writeln('The following RenderObject was being processed when the exception was fired:\n $this');
if (debugCreator != null) if (debugCreator != null)
information.writeln('This RenderObject had the following creator:\n $debugCreator'); information.writeln('This RenderObject had the following creator information:\n $debugCreator');
List<String> descendants = <String>[]; List<String> descendants = <String>[];
const int maxDepth = 5; const int maxDepth = 5;
int depth = 0; int depth = 0;
const int maxLines = 30; const int maxLines = 25;
int lines = 0; int lines = 0;
void visitor(RenderObject child) { void visitor(RenderObject child) {
if (lines < maxLines) { if (lines < maxLines) {
...@@ -1147,12 +1155,39 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { ...@@ -1147,12 +1155,39 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
/// override performResize and/or performLayout. /// override performResize and/or performLayout.
/// ///
/// The parent's performLayout method should call the layout of all its /// The parent's performLayout method should call the layout of all its
/// children unconditionally. It is the layout functions's responsibility (as /// children unconditionally. It is the layout function's responsibility (as
/// implemented here) to return early if the child does not need to do any /// implemented here) to return early if the child does not need to do any
/// work to update its layout information. /// work to update its layout information.
void layout(Constraints constraints, { bool parentUsesSize: false }) { void layout(Constraints constraints, { bool parentUsesSize: false }) {
assert(constraints != null); assert(constraints != null);
assert(constraints.debugAssertIsValid(isAppliedConstraint: true)); assert(constraints.debugAssertIsValid(
isAppliedConstraint: true,
informationCollector: (StringBuffer information) {
List<String> stack = StackTrace.current.toString().split('\n');
int targetFrame;
Pattern layoutFramePattern = new RegExp(r'^#[0-9]+ +RenderObject.layout \(');
for (int i = 0; i < stack.length; i += 1) {
if (layoutFramePattern.matchAsPrefix(stack[i]) != null) {
targetFrame = i + 1;
break;
}
}
if (targetFrame != null && targetFrame < stack.length) {
information.writeln(
'These invalid constraints were provided to $runtimeType\'s method() '
'function by the following function, which probably computed the '
'invalid constraints in question:'
);
Pattern targetFramePattern = new RegExp(r'^#[0-9]+ +(.+)$');
Match targetFrameMatch = targetFramePattern.matchAsPrefix(stack[targetFrame]);
if (targetFrameMatch != null && targetFrameMatch.groupCount > 0) {
information.writeln(' ${targetFrameMatch.group(1)}');
} else {
information.writeln(stack[targetFrame]);
}
}
}
));
assert(!_debugDoingThisResize); assert(!_debugDoingThisResize);
assert(!_debugDoingThisLayout); assert(!_debugDoingThisLayout);
final RenderObject parent = this.parent; final RenderObject parent = this.parent;
...@@ -2189,7 +2224,7 @@ class FlutterErrorDetailsForRendering extends FlutterErrorDetails { ...@@ -2189,7 +2224,7 @@ class FlutterErrorDetailsForRendering extends FlutterErrorDetails {
String library, String library,
String context, String context,
this.renderObject, this.renderObject,
FlutterInformationCollector informationCollector, InformationCollector informationCollector,
bool silent: false bool silent: false
}) : super( }) : super(
exception: exception, exception: exception,
......
...@@ -7,11 +7,9 @@ import 'print.dart'; ...@@ -7,11 +7,9 @@ import 'print.dart';
/// Signature for [FlutterError.onException] handler. /// Signature for [FlutterError.onException] handler.
typedef void FlutterExceptionHandler(FlutterErrorDetails details); typedef void FlutterExceptionHandler(FlutterErrorDetails details);
/// Signature for [FlutterErrorDetails.informationCollector] callback. /// Signature for [FlutterErrorDetails.informationCollector] callback
/// /// and other callbacks that collect information into a string buffer.
/// The text written to the information argument may contain newlines but should typedef void InformationCollector(StringBuffer information);
/// not end with a newline.
typedef void FlutterInformationCollector(StringBuffer information);
/// Class for information provided to [FlutterExceptionHandler] callbacks. /// Class for information provided to [FlutterExceptionHandler] callbacks.
/// ///
...@@ -56,7 +54,10 @@ class FlutterErrorDetails { ...@@ -56,7 +54,10 @@ class FlutterErrorDetails {
/// ///
/// Information collector callbacks can be expensive, so the generated information /// Information collector callbacks can be expensive, so the generated information
/// should be cached, rather than the callback being invoked multiple times. /// should be cached, rather than the callback being invoked multiple times.
final FlutterInformationCollector informationCollector; ///
/// The text written to the information argument may contain newlines but should
/// not end with a newline.
final InformationCollector informationCollector;
/// Whether this error should be ignored by the default error reporting /// Whether this error should be ignored by the default error reporting
/// behavior in release mode. /// behavior in release mode.
...@@ -124,7 +125,7 @@ class FlutterError extends AssertionError { ...@@ -124,7 +125,7 @@ class FlutterError extends AssertionError {
static int _errorCount = 0; static int _errorCount = 0;
static const int _kWrapWidth = 120; static const int _kWrapWidth = 100;
/// Prints the given exception details to the console. /// Prints the given exception details to the console.
/// ///
......
...@@ -43,7 +43,8 @@ void main() { ...@@ -43,7 +43,8 @@ void main() {
} }
expect(result, equals( expect(result, equals(
'BoxConstraints has NaN values in minWidth, maxWidth, and maxHeight.\n' 'BoxConstraints has NaN values in minWidth, maxWidth, and maxHeight.\n'
'BoxConstraints(NaN<=w<=NaN, 2.0<=h<=NaN; NOT NORMALIZED)' 'The offending constraints were:\n'
' BoxConstraints(NaN<=w<=NaN, 2.0<=h<=NaN; NOT NORMALIZED)'
)); ));
result = 'no exception'; result = 'no exception';
...@@ -55,7 +56,8 @@ void main() { ...@@ -55,7 +56,8 @@ void main() {
} }
expect(result, equals( expect(result, equals(
'BoxConstraints has a NaN value in minHeight.\n' 'BoxConstraints has a NaN value in minHeight.\n'
'BoxConstraints(0.0<=w<=Infinity, NaN<=h<=Infinity; NOT NORMALIZED)' 'The offending constraints were:\n'
' BoxConstraints(0.0<=w<=Infinity, NaN<=h<=Infinity; NOT NORMALIZED)'
)); ));
result = 'no exception'; result = 'no exception';
...@@ -67,7 +69,8 @@ void main() { ...@@ -67,7 +69,8 @@ void main() {
} }
expect(result, equals( expect(result, equals(
'BoxConstraints has NaN values in maxWidth and minHeight.\n' 'BoxConstraints has NaN values in maxWidth and minHeight.\n'
'BoxConstraints(0.0<=w<=NaN, NaN<=h<=Infinity; NOT NORMALIZED)' 'The offending constraints were:\n'
' BoxConstraints(0.0<=w<=NaN, NaN<=h<=Infinity; NOT NORMALIZED)'
)); ));
}); });
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment