Skip to content

Commit

Permalink
Revert "Fix remaining holes in stack trace demangling (flutter#60478)" (
Browse files Browse the repository at this point in the history 
flutter#60916)

This reverts commit d986fdc.
  • Loading branch information
@dnfield
dnfield authored Jul 6, 2020
1 parent d986fdc commit 2c7e5dd
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 176 deletions.
58 changes: 7 additions & 51 deletions packages/flutter/lib/src/foundation/assertions.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,6 @@ typedef DiagnosticPropertiesTransformer = Iterable<DiagnosticsNode> Function(Ite
/// and other callbacks that collect information describing an error.
typedef InformationCollector = Iterable<DiagnosticsNode> Function();

/// Signature for a function that demangles [StackTrace] objects into a format
/// that can be parsed by [StackFrame].
///
/// See also:
///
/// * [FlutterError.demangleStackTrace], which shows an example implementation.
typedef StackTraceDemangler = StackTrace Function(StackTrace details);

/// Partial information from a stack frame for stack filtering purposes.
///
/// See also:
Expand Down Expand Up @@ -659,7 +651,7 @@ class FlutterErrorDetails with Diagnosticable {
// 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(FlutterError.demangleStackTrace(stack))
final List<StackFrame> stackFrames = StackFrame.fromStackTrace(stack)
.skipWhile((StackFrame frame) => frame.packageScheme == 'dart')
.toList();
final bool ourFault = stackFrames.length >= 2
Expand Down Expand Up @@ -871,31 +863,6 @@ class FlutterError extends Error with DiagnosticableTreeMixin implements Asserti
/// recommended.
static FlutterExceptionHandler onError = (FlutterErrorDetails details) => presentError(details);

/// Called by the Flutter framework before attempting to parse a [StackTrace].
///
/// Some [StackTrace] implementations have a different toString format from
/// what the framework expects, like ones from package:stack_trace. To make
/// sure we can still parse and filter mangled [StackTrace]s, the framework
/// first calls this function to demangle them.
///
/// This should be set in any environment that could propagate a non-standard
/// stack trace to the framework. Otherwise, the default behavior is to assume
/// all stack traces are in a standard format.
///
/// The following example demangles package:stack_trace traces by converting
/// them into vm traces, which the framework is able to parse:
///
/// ```dart
/// FlutterError.demangleStackTrace = (StackTrace stackTrace) {
/// if (stack is stack_trace.Trace)
// return stack.vmTrace;
// if (stack is stack_trace.Chain)
// return stack.toTrace().vmTrace;
// return stack;
/// };
/// ```
static StackTraceDemangler demangleStackTrace = (StackTrace stackTrace) => stackTrace;

/// Called whenever the Flutter framework wants to present an error to the
/// users.
///
Expand Down Expand Up @@ -1102,11 +1069,7 @@ class FlutterError extends Error with DiagnosticableTreeMixin implements Asserti
void debugPrintStack({StackTrace stackTrace, String label, int maxFrames}) {
if (label != null)
debugPrint(label);
if (stackTrace == null) {
stackTrace = StackTrace.current;
} else {
stackTrace = FlutterError.demangleStackTrace(stackTrace);
}
stackTrace ??= StackTrace.current;
Iterable<String> lines = stackTrace.toString().trimRight().split('\n');
if (kIsWeb && lines.isNotEmpty) {
// Remove extra call to StackTrace.current for web platform.
Expand Down Expand Up @@ -1142,7 +1105,11 @@ class DiagnosticsStackTrace extends DiagnosticsBlock {
}) : super(
name: name,
value: stack,
properties: _applyStackFilter(stack, stackFilter),
properties: stack == null
? <DiagnosticsNode>[]
: (stackFilter ?? FlutterError.defaultStackFilter)(stack.toString().trimRight().split('\n'))
.map<DiagnosticsNode>(_createStackFrame)
.toList(),
style: DiagnosticsTreeStyle.flat,
showSeparator: showSeparator,
allowTruncate: true,
Expand All @@ -1160,17 +1127,6 @@ class DiagnosticsStackTrace extends DiagnosticsBlock {
showSeparator: showSeparator,
);

static List<DiagnosticsNode> _applyStackFilter(
StackTrace stack,
IterableFilter<String> stackFilter,
) {
if (stack == null)
return <DiagnosticsNode>[];
final IterableFilter<String> filter = stackFilter ?? FlutterError.defaultStackFilter;
final Iterable<String> frames = filter('${FlutterError.demangleStackTrace(stack)}'.trimRight().split('\n'));
return frames.map<DiagnosticsNode>(_createStackFrame).toList();
}

static DiagnosticsNode _createStackFrame(String frame) {
return DiagnosticsNode.message(frame, allowWrap: false);
}
Expand Down
7 changes: 0 additions & 7 deletions packages/flutter/lib/src/foundation/stack_frame.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,6 @@ class StackFrame {
return stackOverFlowElision;
}

assert(
line != '===== asynchronous gap ===========================',
'Got a stack frame from package:stack_trace, where a vm or web frame was expected. '
'This can happen if FlutterError.demangleStackTrace was not set in an environment '
'that propagates non-standard stack traces to the framework, such as during tests.'
);

// Web frames.
if (!line.startsWith('#')) {
return _parseWebFrame(line);
Expand Down
4 changes: 1 addition & 3 deletions packages/flutter/lib/src/scheduler/binding.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -621,9 +621,7 @@ mixin SchedulerBinding on BindingBase {
debugPrint('When the current transient callback was registered, this was the stack:');
debugPrint(
FlutterError.defaultStackFilter(
FlutterError.demangleStackTrace(
_FrameCallbackEntry.debugCurrentCallbackStack,
).toString().trimRight().split('\n')
_FrameCallbackEntry.debugCurrentCallbackStack.toString().trimRight().split('\n')
).join('\n')
);
} else {
Expand Down
25 changes: 0 additions & 25 deletions packages/flutter/test/foundation/stack_frame_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,6 @@ void main() {
}, skip: isBrowser); // The VM test harness can handle a stack overflow, but
// the browser cannot - running this test in a browser will cause it to become
// unresponsive.

test('Traces from package:stack_trace throw assertion', () {
try {
StackFrame.fromStackString(mangledStackString);
assert(false, 'StackFrame.fromStackString did not throw on a mangled stack trace');
} catch (e) {
expect(e, isA<AssertionError>());
expect('$e', contains('Got a stack frame from package:stack_trace'));
}
});
}

const String stackString = '''
Expand Down Expand Up @@ -162,21 +152,6 @@ const String asyncStackString = '''
#37 _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:307:19)
#38 _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:174:12)''';

const String mangledStackString = '''
dart:async/future_impl.dart 23:44 _Completer.completeError
test\\bindings_async_gap_test.dart 42:17 main.<fn>.<fn>
package:flutter_test/src/binding.dart 744:19 TestWidgetsFlutterBinding._runTestBody
===== asynchronous gap ===========================
dart:async/zone.dart 1121:19 _CustomZone.registerUnaryCallback
dart:async-patch/async_patch.dart 83:23 _asyncThenWrapperHelper
dart:async/zone.dart 1222:13 _rootRunBinary
dart:async/zone.dart 1107:19 _CustomZone.runBinary
package:flutter_test/src/binding.dart 724:14 TestWidgetsFlutterBinding._runTest
package:flutter_test/src/binding.dart 1124:24 AutomatedTestWidgetsFlutterBinding.runTest.<fn>
package:fake_async/fake_async.dart 177:54 FakeAsync.run.<fn>.<fn>
dart:async/zone.dart 1190:13 _rootRun
''';

const List<StackFrame> asyncStackFrames = <StackFrame>[
StackFrame(number: 0, className: '', method: 'getSampleStack', packageScheme: 'file', package: '<unknown>', packagePath: '/path/to/flutter/packages/flutter/test/foundation/error_reporting_test.dart', line: 40, column: 57, source: '#0 getSampleStack.<anonymous closure> (file:///path/to/flutter/packages/flutter/test/foundation/error_reporting_test.dart:40:57)'),
StackFrame(number: 1, className: 'Future', method: 'sync', packageScheme: 'dart', package: 'async', packagePath: 'future.dart', line: 224, column: 31, isConstructor: true, source: '#1 new Future.sync (dart:async/future.dart:224:31)'),
Expand Down
31 changes: 15 additions & 16 deletions packages/flutter_test/lib/src/binding.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,6 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
return result;
}
FlutterExceptionHandler _oldExceptionHandler;
StackTraceDemangler _oldStackTraceDemangler;
FlutterErrorDetails _pendingExceptionDetails;

static const TextStyle _messageStyle = TextStyle(
Expand Down Expand Up @@ -580,6 +579,10 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
// our main future completing.
assert(Zone.current == _parentZone);
if (_pendingExceptionDetails != null) {
assert(
_unmangle(_pendingExceptionDetails.stack) == _pendingExceptionDetails.stack,
'The test binding presented an unmangled stack trace to the framework.',
);
debugPrint = debugPrintOverride; // just in case the test overrides it -- otherwise we won't see the error!
reportTestException(_pendingExceptionDetails, testDescription);
_pendingExceptionDetails = null;
Expand Down Expand Up @@ -610,9 +613,9 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
assert(description != null);
assert(inTest);
_oldExceptionHandler = FlutterError.onError;
_oldStackTraceDemangler = FlutterError.demangleStackTrace;
int _exceptionCount = 0; // number of un-taken exceptions
FlutterError.onError = (FlutterErrorDetails details) {
details = details.copyWith(stack: _unmangle(details.stack));
if (_pendingExceptionDetails != null) {
debugPrint = debugPrintOverride; // just in case the test overrides it -- otherwise we won't see the errors!
if (_exceptionCount == 0) {
Expand All @@ -631,17 +634,6 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
_pendingExceptionDetails = details;
}
};
FlutterError.demangleStackTrace = (StackTrace stack) {
// package:stack_trace uses ZoneSpecification.errorCallback to add useful
// information to stack traces, in this case the Trace and Chain classes
// can be present. Because these StackTrace implementations do not follow
// the format the framework expects, we covert them to a vm trace here.
if (stack is stack_trace.Trace)
return stack.vmTrace;
if (stack is stack_trace.Chain)
return stack.toTrace().vmTrace;
return stack;
};
final Completer<void> testCompleter = Completer<void>();
final VoidCallback testCompletionHandler = _createTestCompletionHandler(description, testCompleter);
void handleUncaughtError(dynamic exception, StackTrace stack) {
Expand All @@ -655,7 +647,7 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
debugPrint = debugPrintOverride; // just in case the test overrides it -- otherwise we won't see the error!
FlutterError.dumpErrorToConsole(FlutterErrorDetails(
exception: exception,
stack: stack,
stack: _unmangle(stack),
context: ErrorDescription('running a test (but after the test had completed)'),
library: 'Flutter test framework',
), forceReport: true);
Expand Down Expand Up @@ -702,7 +694,7 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
final int stackLinesToOmit = reportExpectCall(stack, omittedFrames);
FlutterError.reportError(FlutterErrorDetails(
exception: exception,
stack: stack,
stack: _unmangle(stack),
context: ErrorDescription('running a test'),
library: 'Flutter test framework',
stackFilter: (Iterable<String> frames) {
Expand Down Expand Up @@ -850,7 +842,6 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
void postTest() {
assert(inTest);
FlutterError.onError = _oldExceptionHandler;
FlutterError.demangleStackTrace = _oldStackTraceDemangler;
_pendingExceptionDetails = null;
_parentZone = null;
buildOwner.focusManager = FocusManager();
Expand Down Expand Up @@ -1722,3 +1713,11 @@ class _LiveTestRenderView extends RenderView {
_label?.paint(context.canvas, offset - const Offset(0.0, 10.0));
}
}

StackTrace _unmangle(StackTrace stack) {
if (stack is stack_trace.Trace)
return stack.vmTrace;
if (stack is stack_trace.Chain)
return stack.toTrace().vmTrace;
return stack;
}
2 changes: 1 addition & 1 deletion packages/flutter_test/test/bindings_async_gap_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,4 @@ Future<void> main() async {

class CustomException implements Exception {
const CustomException();
}
}
73 changes: 0 additions & 73 deletions packages/flutter_test/test/demangle_test.dart
View file

This file was deleted.

0 comments on commit 2c7e5dd

Please sign in to comment.