Unverified Commit 505af78a authored by Dan Field's avatar Dan Field Committed by GitHub

StackTrace parser, fix assertion error message (#48343)

parent 4552724f
......@@ -52,5 +52,6 @@ export 'src/foundation/platform.dart';
export 'src/foundation/print.dart';
export 'src/foundation/profile.dart';
export 'src/foundation/serialization.dart';
export 'src/foundation/stack_frame.dart';
export 'src/foundation/synchronous_future.dart';
export 'src/foundation/unicode.dart';
......@@ -8,6 +8,7 @@ import 'basic_types.dart';
import 'constants.dart';
import 'diagnostics.dart';
import 'print.dart';
import 'stack_frame.dart';
/// Signature for [FlutterError.onError] handler.
typedef FlutterExceptionHandler = void Function(FlutterErrorDetails details);
......@@ -418,39 +419,31 @@ class FlutterErrorDetails extends Diagnosticable {
}
}
final Iterable<String> stackLines = (stack != null) ? stack.toString().trimRight().split('\n') : null;
if (exception is AssertionError && diagnosticable == null) {
bool ourFault = true;
if (stackLines != null) {
final List<String> stackList = stackLines.take(2).toList();
if (stackList.length >= 2) {
// TODO(ianh): This has bitrotted and is no longer matching. https://github.com/flutter/flutter/issues/4021
final RegExp throwPattern = RegExp(
r'^#0 +_AssertionError._throwNew \(dart:.+\)$');
final RegExp assertPattern = RegExp(
r'^#1 +[^(]+ \((.+?):([0-9]+)(?::[0-9]+)?\)$');
if (throwPattern.hasMatch(stackList[0])) {
final Match assertMatch = assertPattern.firstMatch(stackList[1]);
if (assertMatch != null) {
assert(assertMatch.groupCount == 2);
final RegExp ourLibraryPattern = RegExp(r'^package:flutter/');
ourFault = ourLibraryPattern.hasMatch(assertMatch.group(1));
}
}
if (stack != null) {
if (exception is AssertionError && diagnosticable == null) {
// After popping off any dart: stack frames, are there at least two more
// stack frames coming from package flutter?
//
// If not: Error is in user code (user violated assertion in framework).
// If so: Error is in Framework. We either need an assertion higher up
// in the stack, or we've violated our own assertions.
final List<StackFrame> stackFrames = StackFrame.fromStackTrace(stack)
.skipWhile((StackFrame frame) => frame.packageScheme == 'dart')
.toList();
final bool ourFault = stackFrames.length >= 2
&& stackFrames[0].package == 'flutter'
&& stackFrames[1].package == 'flutter';
if (ourFault) {
properties.add(ErrorSpacer());
properties.add(ErrorHint(
'Either the assertion indicates an error in the framework itself, or we should '
'provide substantially more information in this error message to help you determine '
'and fix the underlying cause.\n'
'In either case, please report this assertion by filing a bug on GitHub:\n'
' https://github.com/flutter/flutter/issues/new?template=BUG.md'
));
}
}
if (ourFault) {
properties.add(ErrorSpacer());
properties.add(ErrorHint(
'Either the assertion indicates an error in the framework itself, or we should '
'provide substantially more information in this error message to help you determine '
'and fix the underlying cause.\n'
'In either case, please report this assertion by filing a bug on GitHub:\n'
' https://github.com/flutter/flutter/issues/new?template=BUG.md'
));
}
}
if (stack != null) {
properties.add(ErrorSpacer());
properties.add(DiagnosticsStackTrace('When the exception was thrown, this was the stack', stack, stackFilter: stackFilter));
}
......@@ -672,39 +665,27 @@ class FlutterError extends Error with DiagnosticableTreeMixin implements Asserti
/// format but the frame numbers will not be consecutive (frames are elided)
/// and the final line may be prose rather than a stack frame.
static Iterable<String> defaultStackFilter(Iterable<String> frames) {
const List<String> filteredPackages = <String>[
const Set<String> filteredPackages = <String>{
'dart:async-patch',
'dart:async',
'package:stack_trace',
];
const List<String> filteredClasses = <String>[
};
const Set<String> filteredClasses = <String>{
'_AssertionError',
'_FakeAsync',
'_FrameCallbackEntry',
];
final RegExp stackParser = RegExp(r'^#[0-9]+ +([^.]+).* \(([^/\\]*)[/\\].+:[0-9]+(?::[0-9]+)?\)$');
final RegExp packageParser = RegExp(r'^([^:]+):(.+)$');
};
final List<String> result = <String>[];
final List<String> skipped = <String>[];
for (final String line in frames) {
final Match match = stackParser.firstMatch(line);
if (match != null) {
assert(match.groupCount == 2);
if (filteredPackages.contains(match.group(2))) {
final Match packageMatch = packageParser.firstMatch(match.group(2));
if (packageMatch != null && packageMatch.group(1) == 'package') {
skipped.add('package ${packageMatch.group(2)}'); // avoid "package package:foo"
} else {
skipped.add('package ${match.group(2)}');
}
continue;
}
if (filteredClasses.contains(match.group(1))) {
skipped.add('class ${match.group(1)}');
continue;
}
final StackFrame frameLine = StackFrame.fromStackTraceLine(line);
if (filteredClasses.contains(frameLine.className)) {
skipped.add('class ${frameLine.className}');
} else if (filteredPackages.contains(frameLine.packageScheme + ':' + frameLine.package)) {
skipped.add('package ${frameLine.packageScheme == 'dart' ? 'dart:' : ''}${frameLine.package}');
} else {
result.add(line);
}
result.add(line);
}
if (skipped.length == 1) {
result.add('(elided one frame from ${skipped.single})');
......@@ -794,9 +775,11 @@ class DiagnosticsStackTrace extends DiagnosticsBlock {
}) : super(
name: name,
value: stack,
properties: (stackFilter ?? FlutterError.defaultStackFilter)(stack.toString().trimRight().split('\n'))
.map<DiagnosticsNode>(_createStackFrame)
.toList(),
properties: stack == null
? <DiagnosticsNode>[]
: (stackFilter ?? FlutterError.defaultStackFilter)(stack.toString().trimRight().split('\n'))
.map<DiagnosticsNode>(_createStackFrame)
.toList(),
style: DiagnosticsTreeStyle.flat,
showSeparator: showSeparator,
allowTruncate: true,
......
......@@ -1630,6 +1630,7 @@ abstract class DiagnosticsNode {
return toStringDeep(parentConfiguration: parentConfiguration, minLevel: minLevel);
final String description = toDescription(parentConfiguration: parentConfiguration);
assert(description != null);
if (name == null || name.isEmpty || !showName)
return description;
......
// 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:ui' show hashValues;
import 'package:meta/meta.dart';
/// A object representation of a frame from a stack trace.
///
/// {@tool sample}
///
/// This example creates a traversable list of parsed [StackFrame] objects from
/// the current [StackTrace].
///
/// ```dart
/// final List<StackFrame> currentFrames = StackFrame.fromStackTrace(StackTrace.current);
/// ```
/// {@end-tool}
@immutable
class StackFrame {
/// Creates a new StackFrame instance.
///
/// All parameters must not be null. The [className] may be the empty string
/// if there is no class (e.g. for a top level library method).
const StackFrame({
@required this.number,
@required this.column,
@required this.line,
@required this.packageScheme,
@required this.package,
@required this.packagePath,
this.className = '',
@required this.method,
this.isConstructor = false,
@required this.source,
}) : assert(number != null),
assert(column != null),
assert(line != null),
assert(method != null),
assert(packageScheme != null),
assert(package != null),
assert(packagePath != null),
assert(className != null),
assert(isConstructor != null),
assert(source != null);
/// A stack frame representing an asynchronous suspension.
static const StackFrame asynchronousSuspension = StackFrame(
number: -1,
column: -1,
line: -1,
method: 'asynchronous suspension',
packageScheme: '',
package: '',
packagePath: '',
source: '<asynchronous suspension>',
);
/// Parses a list of [StackFrame]s from a [StackTrace] object.
///
/// This is normally useful with [StackTrace.current].
static List<StackFrame> fromStackTrace(StackTrace stack) {
assert(stack != null);
return fromStackString(stack.toString());
}
/// Parses a list of [StackFrame]s from the [StackTrace.toString] method.
static List<StackFrame> fromStackString(String stack) {
assert(stack != null);
return stack
.trim()
.split('\n')
.map(fromStackTraceLine)
.toList();
}
static StackFrame _parseWebFrame(String line) {
final bool hasPackage = line.startsWith('package');
final RegExp parser = hasPackage
? RegExp(r'^(package:.+) (\d+):(\d+)\s+(.+)$')
: RegExp(r'^(.+) (\d+):(\d+)\s+(.+)$');
final Match match = parser.firstMatch(line);
assert(match != null, 'Expecgted $line to match $parser.');
String package = '<unknown>';
String packageScheme = '<unknown>';
String packagePath = '<unknown>';
if (hasPackage) {
packageScheme = 'package';
final Uri packageUri = Uri.parse(match.group(1));
package = packageUri.pathSegments[0];
packagePath = packageUri.path.replaceFirst(packageUri.pathSegments[0] + '/', '');
}
return StackFrame(
number: -1,
packageScheme: packageScheme,
package: package,
packagePath: packagePath,
line: int.parse(match.group(2)),
column: int.parse(match.group(3)),
className: '<unknown>',
method: match.group(4),
source: line,
);
}
/// Parses a single [StackFrame] from a single line of a [StackTrace].
static StackFrame fromStackTraceLine(String line) {
assert(line != null);
if (line == '<asynchronous suspension>') {
return asynchronousSuspension;
}
// Web frames.
if (!line.startsWith('#')) {
return _parseWebFrame(line);
}
final RegExp parser = RegExp(r'^#(\d+) +(.+) \((.+?):?(\d+){0,1}:?(\d+){0,1}\)$');
final Match match = parser.firstMatch(line);
assert(match != null, 'Expected $line to match $parser.');
bool isConstructor = false;
String className = '';
String method = match.group(2).replaceAll('.<anonymous closure>', '');
if (method.startsWith('new')) {
className = method.split(' ')[1];
method = '';
if (className.contains('.')) {
final List<String> parts = className.split('.');
className = parts[0];
method = parts[1];
}
isConstructor = true;
} else if (method.contains('.')) {
final List<String> parts = method.split('.');
className = parts[0];
method = parts[1];
}
final Uri packageUri = Uri.parse(match.group(3));
String package = '<unknown>';
String packagePath = packageUri.path;
if (packageUri.scheme == 'dart' || packageUri.scheme == 'package') {
package = packageUri.pathSegments[0];
packagePath = packageUri.path.replaceFirst(packageUri.pathSegments[0] + '/', '');
}
return StackFrame(
number: int.parse(match.group(1)),
className: className,
method: method,
packageScheme: packageUri.scheme,
package: package,
packagePath: packagePath,
line: match.group(4) == null ? -1 : int.parse(match.group(4)),
column: match.group(5) == null ? -1 : int.parse(match.group(5)),
isConstructor: isConstructor,
source: line,
);
}
/// The original source of this stack frame.
final String source;
/// The zero-indexed frame number.
///
/// This value may be -1 to indicate an unknown frame number.
final int number;
/// The scheme of the package for this frame, e.g. "dart" for
/// dart:core/errors_patch.dart or "package" for
/// package:flutter/src/widgets/text.dart.
///
/// The path property refers to the source file.
final String packageScheme;
/// The package for this frame, e.g. "core" for
/// dart:core/errors_patch.dart or "flutter" for
/// package:flutter/src/widgets/text.dart.
final String package;
/// The path of the file for this frame, e.g. "errors_patch.dart" for
/// dart:core/errors_patch.dart or "src/widgets/text.dart" for
/// package:flutter/src/widgets/text.dart.
final String packagePath;
/// The source line number.
final int line;
/// The source column number.
final int column;
/// The class name, if any, for this frame.
///
/// This may be null for top level methods in a library or anonymous closure
/// methods.
final String className;
/// The method name for this frame.
///
/// This will be an empty string if the stack frame is from the default
/// constructor.
final String method;
/// Whether or not this was thrown from a constructor.
final bool isConstructor;
@override
int get hashCode => hashValues(number, package, line, column, className, method, source);
@override
bool operator ==(Object other) {
if (runtimeType != other.runtimeType)
return false;
return other is StackFrame &&
number == other.number &&
package == other.package &&
line == other.line &&
column == other.column &&
className == other.className &&
method == other.method &&
source == other.source;
}
@override
String toString() => '$runtimeType(#$number, $packageScheme:$package/$packagePath:$line:$column, className: $className, method: $method)';
}
......@@ -352,4 +352,80 @@ void main() {
expect(summary.value, equals(<String>['Invalid argument(s) (myArgument): Must not be null']));
}
});
test('Identifies user fault', () {
// User fault because they called `new Text(null)` from their own code.
final StackTrace stack = StackTrace.fromString('''#0 _AssertionError._doThrowNew (dart:core-patch/errors_patch.dart:42:39)
#1 _AssertionError._throwNew (dart:core-patch/errors_patch.dart:38:5)
#2 new Text (package:flutter/src/widgets/text.dart:287:10)
#3 _MyHomePageState.build (package:hello_flutter/main.dart:72:16)
#4 StatefulElement.build (package:flutter/src/widgets/framework.dart:4414:27)
#5 ComponentElement.performRebuild (package:flutter/src/widgets/framework.dart:4303:15)
#6 Element.rebuild (package:flutter/src/widgets/framework.dart:4027:5)
#7 ComponentElement._firstBuild (package:flutter/src/widgets/framework.dart:4286:5)
#8 StatefulElement._firstBuild (package:flutter/src/widgets/framework.dart:4461:11)
#9 ComponentElement.mount (package:flutter/src/widgets/framework.dart:4281:5)
#10 Element.inflateWidget (package:flutter/src/widgets/framework.dart:3276:14)
#11 Element.updateChild (package:flutter/src/widgets/framework.dart:3070:12)
#12 SingleChildRenderObjectElement.mount (package:flutter/blah.dart:999:9)''');
final FlutterErrorDetails details = FlutterErrorDetails(
exception: AssertionError('Test assertion'),
stack: stack,
);
final DiagnosticPropertiesBuilder builder = DiagnosticPropertiesBuilder();
details.debugFillProperties(builder);
expect(builder.properties.length, 4);
expect(builder.properties[0].toString(), 'The following assertion was thrown:');
expect(builder.properties[1].toString(), 'Assertion failed');
expect(builder.properties[2] is ErrorSpacer, true);
final DiagnosticsStackTrace trace = builder.properties[3] as DiagnosticsStackTrace;
expect(trace, isNotNull);
expect(trace.value, stack);
});
test('Identifies our fault', () {
// Our fault because we should either have an assertion in `text_helper.dart`
// or we should make sure not to pass bad values into new Text.
final StackTrace stack = StackTrace.fromString('''#0 _AssertionError._doThrowNew (dart:core-patch/errors_patch.dart:42:39)
#1 _AssertionError._throwNew (dart:core-patch/errors_patch.dart:38:5)
#2 new Text (package:flutter/src/widgets/text.dart:287:10)
#3 new SomeWidgetUsingText (package:flutter/src/widgets/text_helper.dart:287:10)
#4 _MyHomePageState.build (package:hello_flutter/main.dart:72:16)
#5 StatefulElement.build (package:flutter/src/widgets/framework.dart:4414:27)
#6 ComponentElement.performRebuild (package:flutter/src/widgets/framework.dart:4303:15)
#7 Element.rebuild (package:flutter/src/widgets/framework.dart:4027:5)
#8 ComponentElement._firstBuild (package:flutter/src/widgets/framework.dart:4286:5)
#9 StatefulElement._firstBuild (package:flutter/src/widgets/framework.dart:4461:11)
#10 ComponentElement.mount (package:flutter/src/widgets/framework.dart:4281:5)
#11 Element.inflateWidget (package:flutter/src/widgets/framework.dart:3276:14)
#12 Element.updateChild (package:flutter/src/widgets/framework.dart:3070:12)
#13 SingleChildRenderObjectElement.mount (package:flutter/blah.dart:999:9)''');
final FlutterErrorDetails details = FlutterErrorDetails(
exception: AssertionError('Test assertion'),
stack: stack,
);
final DiagnosticPropertiesBuilder builder = DiagnosticPropertiesBuilder();
details.debugFillProperties(builder);
expect(builder.properties.length, 6);
expect(builder.properties[0].toString(), 'The following assertion was thrown:');
expect(builder.properties[1].toString(), 'Assertion failed');
expect(builder.properties[2] is ErrorSpacer, true);
expect(
builder.properties[3].toString(),
'Either the assertion indicates an error in the framework itself, or we should '
'provide substantially more information in this error message to help you determine '
'and fix the underlying cause.\n'
'In either case, please report this assertion by filing a bug on GitHub:\n'
' https://github.com/flutter/flutter/issues/new?template=BUG.md',
);
expect(builder.properties[4] is ErrorSpacer, true);
final DiagnosticsStackTrace trace = builder.properties[5] as DiagnosticsStackTrace;
expect(trace, isNotNull);
expect(trace.value, stack);
});
}
......@@ -1849,18 +1849,18 @@ void main() {
test('Stack trace test', () {
final StackTrace stack = StackTrace.fromString(
'#0 someMethod() file:///diagnostics_test.dart:42:19\n'
'#1 someMethod2() file:///diagnostics_test.dart:12:3\n'
'#2 someMethod3() file:///foo.dart:4:1\n'
'#0 someMethod (file:///diagnostics_test.dart:42:19)\n'
'#1 someMethod2 (file:///diagnostics_test.dart:12:3)\n'
'#2 someMethod3 (file:///foo.dart:4:1)\n'
);
expect(
DiagnosticsStackTrace('Stack trace', stack).toStringDeep(),
equalsIgnoringHashCodes(
'Stack trace:\n'
'#0 someMethod() file:///diagnostics_test.dart:42:19\n'
'#1 someMethod2() file:///diagnostics_test.dart:12:3\n'
'#2 someMethod3() file:///foo.dart:4:1\n'
'#0 someMethod (file:///diagnostics_test.dart:42:19)\n'
'#1 someMethod2 (file:///diagnostics_test.dart:12:3)\n'
'#2 someMethod3 (file:///foo.dart:4:1)\n'
),
);
......@@ -1868,9 +1868,9 @@ void main() {
DiagnosticsStackTrace('-- callback 2 --', stack, showSeparator: false).toStringDeep(),
equalsIgnoringHashCodes(
'-- callback 2 --\n'
'#0 someMethod() file:///diagnostics_test.dart:42:19\n'
'#1 someMethod2() file:///diagnostics_test.dart:12:3\n'
'#2 someMethod3() file:///foo.dart:4:1\n'
'#0 someMethod (file:///diagnostics_test.dart:42:19)\n'
'#1 someMethod2 (file:///diagnostics_test.dart:12:3)\n'
'#2 someMethod3 (file:///foo.dart:4:1)\n'
),
);
});
......
......@@ -45,13 +45,18 @@ Future<void> main() async {
final StackTrace sampleStack = await getSampleStack();
test('Error reporting - pretest', () async {
setUp(() async {
expect(debugPrint, equals(debugPrintThrottled));
debugPrint = (String message, { int wrapWidth }) {
console.add(message);
};
});
tearDown(() async {
expect(console, isEmpty);
debugPrint = debugPrintThrottled;
});
test('Error reporting - assert with message', () async {
expect(console, isEmpty);
FlutterError.dumpErrorToConsole(FlutterErrorDetails(
......@@ -71,11 +76,6 @@ Future<void> main() async {
'\'[^\']+flutter/test/foundation/error_reporting_test\\.dart\':\n'
'Failed assertion: line [0-9]+ pos [0-9]+: \'false\'\n'
'\n'
'Either the assertion indicates an error in the framework itself, or we should provide substantially\n'
'more information in this error message to help you determine and fix the underlying cause\\.\n'
'In either case, please report this assertion by filing a bug on GitHub:\n'
' https://github\\.com/flutter/flutter/issues/new\\?template=BUG\\.md\n'
'\n'
'When the exception was thrown, this was the stack:\n'
'#0 getSampleStack\\.<anonymous closure> \\([^)]+flutter/test/foundation/error_reporting_test\\.dart:[0-9]+:[0-9]+\\)\n'
'#2 getSampleStack \\([^)]+flutter/test/foundation/error_reporting_test\\.dart:[0-9]+:[0-9]+\\)\n'
......@@ -111,11 +111,6 @@ Future<void> main() async {
'word word word word word word word word word word word word word word word word word word word word\n'
'\'[^\']+flutter/test/foundation/error_reporting_test\\.dart\':\n'
'Failed assertion: line [0-9]+ pos [0-9]+: \'false\'\n'
'\n'
'Either the assertion indicates an error in the framework itself, or we should provide substantially\n'
'more information in this error message to help you determine and fix the underlying cause\\.\n'
'In either case, please report this assertion by filing a bug on GitHub:\n'
' https://github\\.com/flutter/flutter/issues/new\\?template=BUG\\.md\n'
'════════════════════════════════════════════════════════════════════════════════════════════════════\$',
));
console.clear();
......@@ -153,11 +148,6 @@ Future<void> main() async {
'\'[^\']+flutter/test/foundation/error_reporting_test\\.dart\':[\n ]'
'Failed[\n ]assertion:[\n ]line[\n ][0-9]+[\n ]pos[\n ][0-9]+:[\n ]\'false\':[\n ]is[\n ]not[\n ]true\\.\n'
'\n'
'Either the assertion indicates an error in the framework itself, or we should provide substantially\n'
'more information in this error message to help you determine and fix the underlying cause\\.\n'
'In either case, please report this assertion by filing a bug on GitHub:\n'
' https://github\\.com/flutter/flutter/issues/new\\?template=BUG\\.md\n'
'\n'
'When the exception was thrown, this was the stack:\n'
'#0 getSampleStack\\.<anonymous closure> \\([^)]+flutter/test/foundation/error_reporting_test\\.dart:[0-9]+:[0-9]+\\)\n'
'#2 getSampleStack \\([^)]+flutter/test/foundation/error_reporting_test\\.dart:[0-9]+:[0-9]+\\)\n'
......@@ -219,9 +209,4 @@ Future<void> main() async {
console.clear();
FlutterError.resetErrorCount();
});
test('Error reporting - posttest', () async {
expect(console, isEmpty);
debugPrint = debugPrintThrottled;
});
}
This diff is collapsed.
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