Remove the outermost folded frame for terse stack traces. R=rnystrom@google.com Review URL: https://codereview.chromium.org//962913002
diff --git a/pkgs/stack_trace/CHANGELOG.md b/pkgs/stack_trace/CHANGELOG.md index ddaf228..403cd65 100644 --- a/pkgs/stack_trace/CHANGELOG.md +++ b/pkgs/stack_trace/CHANGELOG.md
@@ -1,3 +1,9 @@ +## 1.2.2 + +* Don't print the first folded frame of terse stack traces. This frame + is always just an internal isolate message handler anyway. This + improves the readability of stack traces, especially in stack chains. + ## 1.2.1 * Add `terse` to `LazyTrace.foldFrames()`.
diff --git a/pkgs/stack_trace/lib/src/chain.dart b/pkgs/stack_trace/lib/src/chain.dart index 99726f3..9555e76 100644 --- a/pkgs/stack_trace/lib/src/chain.dart +++ b/pkgs/stack_trace/lib/src/chain.dart
@@ -155,9 +155,14 @@ var foldedTraces = traces.map( (trace) => trace.foldFrames(predicate, terse: terse)); var nonEmptyTraces = foldedTraces.where((trace) { - // Ignore traces that contain only folded frames. These traces will be - // folded into a single frame each. - return trace.frames.length > 1; + // Ignore traces that contain only folded frames. + if (trace.frames.length > 1) return true; + + // In terse mode, the trace may have removed an outer folded frame, + // leaving a single non-folded frame. We can detect a folded frame because + // it has no line information. + if (!terse) return false; + return trace.frames.single.line != null; }); // If all the traces contain only internal processing, preserve the last
diff --git a/pkgs/stack_trace/lib/src/trace.dart b/pkgs/stack_trace/lib/src/trace.dart index 53dbe33..e0cb21b 100644 --- a/pkgs/stack_trace/lib/src/trace.dart +++ b/pkgs/stack_trace/lib/src/trace.dart
@@ -202,7 +202,8 @@ /// This is accomplished by folding together multiple stack frames from the /// core library or from this package, as in [foldFrames]. Remaining core /// library frames have their libraries, "-patch" suffixes, and line numbers - /// removed. + /// removed. If the outermost frame of the stack trace is a core library + /// frame, it's removed entirely. /// /// For custom folding, see [foldFrames]. Trace get terse => foldFrames((_) => false, terse: true); @@ -216,8 +217,8 @@ /// code and code that's called by user code. /// /// If [terse] is true, this will also fold together frames from the core - /// library or from this package, and simplify core library frames as in - /// [Trace.terse]. + /// library or from this package, simplify core library frames, and + /// potentially remove the outermost frame as in [Trace.terse]. Trace foldFrames(bool predicate(Frame frame), {bool terse: false}) { if (terse) { var oldPredicate = predicate; @@ -255,6 +256,7 @@ var library = frame.library.replaceAll(_terseRegExp, ''); return new Frame(Uri.parse(library), null, null, frame.member); }).toList(); + if (newFrames.first.isCore && newFrames.length > 1) newFrames.removeAt(0); } return new Trace(newFrames.reversed);
diff --git a/pkgs/stack_trace/pubspec.yaml b/pkgs/stack_trace/pubspec.yaml index 1134a70..24acfbc 100644 --- a/pkgs/stack_trace/pubspec.yaml +++ b/pkgs/stack_trace/pubspec.yaml
@@ -7,7 +7,7 @@ # # When the major version is upgraded, you *must* update that version constraint # in pub to stay in sync with this. -version: 1.2.1 +version: 1.2.2-dev author: "Dart Team <misc@dartlang.org>" homepage: http://github.com/dart-lang/stack_trace description: >
diff --git a/pkgs/stack_trace/test/chain_test.dart b/pkgs/stack_trace/test/chain_test.dart index 54bc5f5..c5f8efb 100644 --- a/pkgs/stack_trace/test/chain_test.dart +++ b/pkgs/stack_trace/test/chain_test.dart
@@ -500,7 +500,6 @@ expect(chain.terse.toString(), equals( 'dart:core Bar.baz\n' '$userSlashCode 10:11 Bang.qux\n' - 'dart:core Zop.zoop\n' '===== asynchronous gap ===========================\n' '$userSlashCode 10:11 Bang.qux\n' 'dart:core Zip.zap\n' @@ -523,10 +522,8 @@ expect(chain.terse.toString(), equals( '$userSlashCode 10:11 Foo.bar\n' - 'dart:core Bar.baz\n' '===== asynchronous gap ===========================\n' - '$userSlashCode 10:11 Foo.bar\n' - 'dart:core Bar.baz\n')); + '$userSlashCode 10:11 Foo.bar\n')); }); test("doesn't return an empty chain", () {
diff --git a/pkgs/stack_trace/test/trace_test.dart b/pkgs/stack_trace/test/trace_test.dart index 125ba95..10c931c 100644 --- a/pkgs/stack_trace/test/trace_test.dart +++ b/pkgs/stack_trace/test/trace_test.dart
@@ -267,33 +267,58 @@ test('.terse folds core frames together bottom-up', () { var trace = new Trace.parse(''' -#0 notCore (foo.dart:42:21) #1 top (dart:async/future.dart:0:2) #2 bottom (dart:core/uri.dart:1:100) -#3 alsoNotCore (bar.dart:10:20) -#4 top (dart:io:5:10) -#5 bottom (dart:async-patch/future.dart:9:11) +#0 notCore (foo.dart:42:21) +#3 top (dart:io:5:10) +#4 bottom (dart:async-patch/future.dart:9:11) +#5 alsoNotCore (bar.dart:10:20) '''); expect(trace.terse.toString(), equals(''' -foo.dart 42:21 notCore dart:core bottom -bar.dart 10:20 alsoNotCore +foo.dart 42:21 notCore dart:async bottom +bar.dart 10:20 alsoNotCore ''')); }); test('.terse folds empty async frames', () { var trace = new Trace.parse(''' +#0 top (dart:async/future.dart:0:2) +#1 empty.<<anonymous closure>_async_body> (bar.dart) +#2 bottom (dart:async-patch/future.dart:9:11) +#3 notCore (foo.dart:42:21) +'''); + + expect(trace.terse.toString(), equals(''' +dart:async bottom +foo.dart 42:21 notCore +''')); + }); + + test('.terse removes the bottom-most async frame', () { + var trace = new Trace.parse(''' #0 notCore (foo.dart:42:21) #1 top (dart:async/future.dart:0:2) -#2 empty.<<anonymous closure>_async_body> (bar.dart) -#3 bottom (dart:async-patch/future.dart:9:11) +#2 bottom (dart:core/uri.dart:1:100) +#3 top (dart:io:5:10) +#4 bottom (dart:async-patch/future.dart:9:11) '''); expect(trace.terse.toString(), equals(''' foo.dart 42:21 notCore -dart:async bottom +''')); + }); + + test(".terse won't make a trace empty", () { + var trace = new Trace.parse(''' +#1 top (dart:async/future.dart:0:2) +#2 bottom (dart:core/uri.dart:1:100) +'''); + + expect(trace.terse.toString(), equals(''' +dart:core bottom ''')); }); @@ -316,7 +341,7 @@ ''')); }); - test('.foldFrames with terse: true, folds core frames as well', () { + test('.foldFrames with terse: true folds core frames as well', () { var trace = new Trace.parse(''' #0 notFoo (foo.dart:42:21) #1 fooTop (bar.dart:0:2) @@ -332,7 +357,6 @@ foo.dart 42:21 notFoo dart:async coreBottom bar.dart 10:20 alsoNotFoo -dart:async coreBottom ''')); }); }