Don't throw for unparseable frames.
If the stack trace parser throws when it encounters a frame it doesn't
understand, that tends to obscure other errors and generally make life
difficult for users.
R=rnystrom@google.com
Review URL: https://codereview.chromium.org//1283113002 .
diff --git a/pkgs/stack_trace/CHANGELOG.md b/pkgs/stack_trace/CHANGELOG.md
index 1d4eaae..ed1a6c7 100644
--- a/pkgs/stack_trace/CHANGELOG.md
+++ b/pkgs/stack_trace/CHANGELOG.md
@@ -1,4 +1,9 @@
-## 1.3.6
+## 1.4.0
+
+* `new Trace.parse()` and related constructors will no longer throw an exception
+ if they encounter an unparseable stack frame. Instead, they will generate an
+ `UnparsedFrame`, which exposes no metadata but preserves the frame's original
+ text.
* Properly parse native-code V8 frames.
diff --git a/pkgs/stack_trace/lib/src/frame.dart b/pkgs/stack_trace/lib/src/frame.dart
index a5fd6ae..072983a 100644
--- a/pkgs/stack_trace/lib/src/frame.dart
+++ b/pkgs/stack_trace/lib/src/frame.dart
@@ -7,6 +7,7 @@
import 'package:path/path.dart' as path;
import 'trace.dart';
+import 'unparsed_frame.dart';
// #1 Foo._bar (file:///home/nweiz/code/stuff.dart:42:21)
// #1 Foo._bar (file:///home/nweiz/code/stuff.dart:42)
@@ -126,7 +127,7 @@
}
/// Parses a string representation of a Dart VM stack frame.
- factory Frame.parseVM(String frame) {
+ factory Frame.parseVM(String frame) => _catchFormatException(frame, () {
// The VM sometimes folds multiple stack frames together and replaces them
// with "...".
if (frame == '...') {
@@ -134,9 +135,7 @@
}
var match = _vmFrame.firstMatch(frame);
- if (match == null) {
- throw new FormatException("Couldn't parse VM stack trace line '$frame'.");
- }
+ if (match == null) return new UnparsedFrame(frame);
// Get the pieces out of the regexp match. Function, URI and line should
// always be found. The column is optional.
@@ -149,14 +148,12 @@
var line = lineAndColumn.length > 1 ? int.parse(lineAndColumn[1]) : null;
var column = lineAndColumn.length > 2 ? int.parse(lineAndColumn[2]) : null;
return new Frame(uri, line, column, member);
- }
+ });
/// Parses a string representation of a Chrome/V8 stack frame.
- factory Frame.parseV8(String frame) {
+ factory Frame.parseV8(String frame) => _catchFormatException(frame, () {
var match = _v8Frame.firstMatch(frame);
- if (match == null) {
- throw new FormatException("Couldn't parse V8 stack trace line '$frame'.");
- }
+ if (match == null) return new UnparsedFrame(frame);
// v8 location strings can be arbitrarily-nested, since it adds a layer of
// nesting for each eval performed on that line.
@@ -172,10 +169,7 @@
}
var urlMatch = _v8UrlLocation.firstMatch(location);
- if (urlMatch == null) {
- throw new FormatException(
- "Couldn't parse V8 stack trace line '$frame'.");
- }
+ if (urlMatch == null) return new UnparsedFrame(frame);
return new Frame(
_uriOrPathToUri(urlMatch[1]),
@@ -197,7 +191,7 @@
// functions.
return parseLocation(match[3], "<fn>");
}
- }
+ });
/// Parses a string representation of a JavaScriptCore stack trace.
factory Frame.parseJSCore(String frame) => new Frame.parseV8(frame);
@@ -209,12 +203,9 @@
factory Frame.parseIE(String frame) => new Frame.parseV8(frame);
/// Parses a string representation of a Firefox stack frame.
- factory Frame.parseFirefox(String frame) {
+ factory Frame.parseFirefox(String frame) => _catchFormatException(frame, () {
var match = _firefoxSafariFrame.firstMatch(frame);
- if (match == null) {
- throw new FormatException(
- "Couldn't parse Firefox/Safari stack trace line '$frame'.");
- }
+ if (match == null) return new UnparsedFrame(frame);
// Normally this is a URI, but in a jsshell trace it can be a path.
var uri = _uriOrPathToUri(match[3]);
@@ -237,7 +228,7 @@
var column = match[5] == null || match[5] == '' ?
null : int.parse(match[5]);
return new Frame(uri, line, column, member);
- }
+ });
/// Parses a string representation of a Safari 6.0 stack frame.
@Deprecated("Use Frame.parseSafari instead.")
@@ -251,7 +242,7 @@
factory Frame.parseSafari(String frame) => new Frame.parseFirefox(frame);
/// Parses this package's string representation of a stack frame.
- factory Frame.parseFriendly(String frame) {
+ factory Frame.parseFriendly(String frame) => _catchFormatException(frame, () {
var match = _friendlyFrame.firstMatch(frame);
if (match == null) {
throw new FormatException(
@@ -268,7 +259,7 @@
var line = match[2] == null ? null : int.parse(match[2]);
var column = match[3] == null ? null : int.parse(match[3]);
return new Frame(uri, line, column, match[4]);
- }
+ });
/// A regular expression matching an absolute URI.
static final _uriRegExp = new RegExp(r'^[a-zA-Z][-+.a-zA-Z\d]*://');
@@ -294,6 +285,18 @@
return Uri.parse(uriOrPath);
}
+ /// Runs [body] and returns its result.
+ ///
+ /// If [body] throws a [FormatException], returns an [UnparsedFrame] with
+ /// [text] instead.
+ static Frame _catchFormatException(String text, Frame body()) {
+ try {
+ return body();
+ } on FormatException catch (_) {
+ return new UnparsedFrame(text);
+ }
+ }
+
Frame(this.uri, this.line, this.column, this.member);
String toString() => '$location in $member';
diff --git a/pkgs/stack_trace/lib/src/trace.dart b/pkgs/stack_trace/lib/src/trace.dart
index a1691ed..f615cd2 100644
--- a/pkgs/stack_trace/lib/src/trace.dart
+++ b/pkgs/stack_trace/lib/src/trace.dart
@@ -10,6 +10,7 @@
import 'chain.dart';
import 'frame.dart';
import 'lazy_trace.dart';
+import 'unparsed_frame.dart';
import 'utils.dart';
import 'vm_trace.dart';
@@ -115,7 +116,7 @@
try {
if (trace.isEmpty) return new Trace(<Frame>[]);
if (trace.contains(_v8Trace)) return new Trace.parseV8(trace);
- if (trace.startsWith("\tat ")) return new Trace.parseJSCore(trace);
+ if (trace.contains("\tat ")) return new Trace.parseJSCore(trace);
if (trace.contains(_firefoxSafariTrace)) {
return new Trace.parseFirefox(trace);
}
@@ -262,7 +263,7 @@
var newFrames = [];
for (var frame in frames.reversed) {
- if (!predicate(frame)) {
+ if (frame is UnparsedFrame || !predicate(frame)) {
newFrames.add(frame);
} else if (newFrames.isEmpty || !predicate(newFrames.last)) {
newFrames.add(new Frame(
@@ -272,7 +273,7 @@
if (terse) {
newFrames = newFrames.map((frame) {
- if (!predicate(frame)) return frame;
+ if (frame is UnparsedFrame || !predicate(frame)) return frame;
var library = frame.library.replaceAll(_terseRegExp, '');
return new Frame(Uri.parse(library), null, null, frame.member);
}).toList();
@@ -290,6 +291,7 @@
// Print out the stack trace nicely formatted.
return frames.map((frame) {
+ if (frame is UnparsedFrame) return "$frame\n";
return '${padRight(frame.location, longest)} ${frame.member}\n';
}).join();
}
diff --git a/pkgs/stack_trace/lib/src/unparsed_frame.dart b/pkgs/stack_trace/lib/src/unparsed_frame.dart
new file mode 100644
index 0000000..19d09c6
--- /dev/null
+++ b/pkgs/stack_trace/lib/src/unparsed_frame.dart
@@ -0,0 +1,26 @@
+// Copyright (c) 2015, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+library stack_trace.unparsed_frame;
+
+import 'frame.dart';
+
+/// A frame that failed to parse.
+///
+/// The [member] property contains the original frame's contents.
+class UnparsedFrame implements Frame {
+ final Uri uri = new Uri(path: "unparsed");
+ final int line = null;
+ final int column = null;
+ final bool isCore = false;
+ final String library = "unparsed";
+ final String package = null;
+ final String location = "unparsed";
+
+ final String member;
+
+ UnparsedFrame(this.member);
+
+ String toString() => _original;
+}
\ No newline at end of file
diff --git a/pkgs/stack_trace/lib/stack_trace.dart b/pkgs/stack_trace/lib/stack_trace.dart
index ac875a9..6bba635 100644
--- a/pkgs/stack_trace/lib/stack_trace.dart
+++ b/pkgs/stack_trace/lib/stack_trace.dart
@@ -23,6 +23,7 @@
*/
library stack_trace;
-export 'src/trace.dart';
-export 'src/frame.dart';
export 'src/chain.dart';
+export 'src/frame.dart';
+export 'src/trace.dart';
+export 'src/unparsed_frame.dart';
diff --git a/pkgs/stack_trace/test/frame_test.dart b/pkgs/stack_trace/test/frame_test.dart
index 809822a..1233cbf 100644
--- a/pkgs/stack_trace/test/frame_test.dart
+++ b/pkgs/stack_trace/test/frame_test.dart
@@ -71,14 +71,14 @@
expect(frame.column, isNull);
});
- test('throws a FormatException for malformed frames', () {
- expect(() => new Frame.parseVM(''), throwsFormatException);
- expect(() => new Frame.parseVM('#1'), throwsFormatException);
- expect(() => new Frame.parseVM('#1 Foo'), throwsFormatException);
- expect(() => new Frame.parseVM('#1 (dart:async/future.dart:10:15)'),
- throwsFormatException);
- expect(() => new Frame.parseVM('Foo (dart:async/future.dart:10:15)'),
- throwsFormatException);
+ test('returns an UnparsedFrame for malformed frames', () {
+ expectIsUnparsed((text) => new Frame.parseV8(text), '');
+ expectIsUnparsed((text) => new Frame.parseV8(text), '#1');
+ expectIsUnparsed((text) => new Frame.parseV8(text), '#1 Foo');
+ expectIsUnparsed((text) => new Frame.parseV8(text),
+ '#1 (dart:async/future.dart:10:15)');
+ expectIsUnparsed((text) => new Frame.parseV8(text),
+ 'Foo (dart:async/future.dart:10:15)');
});
});
@@ -219,24 +219,22 @@
equals('<fn>.<fn>.bar'));
});
- test('throws a FormatException for malformed frames', () {
- expect(() => new Frame.parseV8(''), throwsFormatException);
- expect(() => new Frame.parseV8(' at'), throwsFormatException);
- expect(() => new Frame.parseV8(' at Foo'), throwsFormatException);
- expect(() => new Frame.parseV8(' at Foo (dart:async/future.dart)'),
- throwsFormatException);
- expect(() => new Frame.parseV8(' at Foo (dart:async/future.dart:10)'),
- throwsFormatException);
- expect(() => new Frame.parseV8(' at (dart:async/future.dart:10:15)'),
- throwsFormatException);
- expect(() => new Frame.parseV8('Foo (dart:async/future.dart:10:15)'),
- throwsFormatException);
- expect(() => new Frame.parseV8(' at dart:async/future.dart'),
- throwsFormatException);
- expect(() => new Frame.parseV8(' at dart:async/future.dart:10'),
- throwsFormatException);
- expect(() => new Frame.parseV8('dart:async/future.dart:10:15'),
- throwsFormatException);
+ test('returns an UnparsedFrame for malformed frames', () {
+ expectIsUnparsed((text) => new Frame.parseV8(text), '');
+ expectIsUnparsed((text) => new Frame.parseV8(text), ' at');
+ expectIsUnparsed((text) => new Frame.parseV8(text), ' at Foo');
+ expectIsUnparsed((text) => new Frame.parseV8(text),
+ ' at Foo (dart:async/future.dart)');
+ expectIsUnparsed((text) => new Frame.parseV8(text),
+ ' at (dart:async/future.dart:10:15)');
+ expectIsUnparsed((text) => new Frame.parseV8(text),
+ 'Foo (dart:async/future.dart:10:15)');
+ expectIsUnparsed((text) => new Frame.parseV8(text),
+ ' at dart:async/future.dart');
+ expectIsUnparsed((text) => new Frame.parseV8(text),
+ ' at dart:async/future.dart:10');
+ expectIsUnparsed((text) => new Frame.parseV8(text),
+ 'dart:async/future.dart:10:15');
});
});
@@ -376,15 +374,15 @@
expect(frame.member, equals("convertDartClosureToJS.<fn>.<fn>"));
});
- test('throws a FormatException for malformed frames', () {
- expect(() => new Frame.parseFirefox(''), throwsFormatException);
- expect(() => new Frame.parseFirefox('.foo'), throwsFormatException);
- expect(() => new Frame.parseFirefox('.foo@dart:async/future.dart'),
- throwsFormatException);
- expect(() => new Frame.parseFirefox('.foo(@dart:async/future.dart:10'),
- throwsFormatException);
- expect(() => new Frame.parseFirefox('@dart:async/future.dart'),
- throwsFormatException);
+ test('returns an UnparsedFrame for malformed frames', () {
+ expectIsUnparsed((text) => new Frame.parseFirefox(text), '');
+ expectIsUnparsed((text) => new Frame.parseFirefox(text), '.foo');
+ expectIsUnparsed((text) => new Frame.parseFirefox(text),
+ '.foo@dart:async/future.dart');
+ expectIsUnparsed((text) => new Frame.parseFirefox(text),
+ '.foo(@dart:async/future.dart:10');
+ expectIsUnparsed((text) => new Frame.parseFirefox(text),
+ '@dart:async/future.dart');
});
test('parses a simple stack frame correctly', () {
@@ -470,12 +468,11 @@
expect(frame.member, equals('Foo.<fn>.bar'));
});
- test('throws a FormatException for malformed frames', () {
- expect(() => new Frame.parseFriendly(''), throwsFormatException);
- expect(() => new Frame.parseFriendly('foo/bar.dart'),
- throwsFormatException);
- expect(() => new Frame.parseFriendly('foo/bar.dart 10:11'),
- throwsFormatException);
+ test('returns an UnparsedFrame for malformed frames', () {
+ expectIsUnparsed((text) => new Frame.parseFriendly(text), '');
+ expectIsUnparsed((text) => new Frame.parseFriendly(text), 'foo/bar.dart');
+ expectIsUnparsed((text) => new Frame.parseFriendly(text),
+ 'foo/bar.dart 10:11');
});
});
@@ -562,3 +559,9 @@
});
});
}
+
+void expectIsUnparsed(Frame constructor(String text), String text) {
+ var frame = constructor(text);
+ expect(frame, new isInstanceOf<UnparsedFrame>());
+ expect(frame.toString(), equals(text));
+}
diff --git a/pkgs/stack_trace/test/trace_test.dart b/pkgs/stack_trace/test/trace_test.dart
index 14e1e2b..1cc32f7 100644
--- a/pkgs/stack_trace/test/trace_test.dart
+++ b/pkgs/stack_trace/test/trace_test.dart
@@ -293,8 +293,9 @@
'#3 baz (dart:async:15:0)\n'));
});
- test('.terse folds core frames together bottom-up', () {
- var trace = new Trace.parse('''
+ group("folding", () {
+ test('.terse folds core frames together bottom-up', () {
+ var trace = new Trace.parse('''
#1 top (dart:async/future.dart:0:2)
#2 bottom (dart:core/uri.dart:1:100)
#0 notCore (foo.dart:42:21)
@@ -303,30 +304,30 @@
#5 alsoNotCore (bar.dart:10:20)
''');
- expect(trace.terse.toString(), equals('''
+ expect(trace.terse.toString(), equals('''
dart:core bottom
foo.dart 42:21 notCore
dart:async bottom
bar.dart 10:20 alsoNotCore
'''));
- });
+ });
- test('.terse folds empty async frames', () {
- var trace = new Trace.parse('''
+ 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('''
+ 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('''
+ 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 bottom (dart:core/uri.dart:1:100)
@@ -334,32 +335,28 @@
#4 bottom (dart:async-patch/future.dart:9:11)
''');
- expect(trace.terse.toString(), equals('''
+ expect(trace.terse.toString(), equals('''
foo.dart 42:21 notCore
'''));
- });
+ });
- test(".terse won't make a trace empty", () {
- var trace = new Trace.parse('''
+ 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('''
+ expect(trace.terse.toString(), equals('''
dart:core bottom
'''));
- });
+ });
- test(".terse won't panic on an empty trace", () {
- var trace = new Trace.parse('''
-''');
+ test(".terse won't panic on an empty trace", () {
+ expect(Trace.parse("").terse.toString(), equals(""));
+ });
- expect(trace.terse.toString(), equals('''
-'''));
- });
-
- test('.foldFrames folds frames together bottom-up', () {
- var trace = new Trace.parse('''
+ test('.foldFrames folds frames together bottom-up', () {
+ var trace = new Trace.parse('''
#0 notFoo (foo.dart:42:21)
#1 fooTop (bar.dart:0:2)
#2 fooBottom (foo.dart:1:100)
@@ -368,17 +365,17 @@
#5 fooBottom (dart:async-patch/future.dart:9:11)
''');
- var folded = trace.foldFrames((frame) => frame.member.startsWith('foo'));
- expect(folded.toString(), equals('''
+ var folded = trace.foldFrames((frame) => frame.member.startsWith('foo'));
+ expect(folded.toString(), equals('''
foo.dart 42:21 notFoo
foo.dart 1:100 fooBottom
bar.dart 10:20 alsoNotFoo
dart:async-patch/future.dart 9:11 fooBottom
'''));
- });
+ });
- test('.foldFrames with terse: true folds core frames as well', () {
- var trace = new Trace.parse('''
+ 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)
#2 coreBottom (dart:async/future.dart:0:2)
@@ -387,17 +384,17 @@
#5 coreBottom (dart:async-patch/future.dart:9:11)
''');
- var folded = trace.foldFrames((frame) => frame.member.startsWith('foo'),
- terse: true);
- expect(folded.toString(), equals('''
+ var folded = trace.foldFrames((frame) => frame.member.startsWith('foo'),
+ terse: true);
+ expect(folded.toString(), equals('''
foo.dart 42:21 notFoo
dart:async coreBottom
bar.dart 10:20 alsoNotFoo
'''));
- });
+ });
- test('.foldFrames with terse: true shortens folded frames', () {
- var trace = new Trace.parse('''
+ test('.foldFrames with terse: true shortens folded frames', () {
+ var trace = new Trace.parse('''
#0 notFoo (foo.dart:42:21)
#1 fooTop (bar.dart:0:2)
#2 fooBottom (package:foo/bar.dart:0:2)
@@ -406,13 +403,28 @@
#5 fooBottom (foo/bar.dart:9:11)
''');
- var folded = trace.foldFrames((frame) => frame.member.startsWith('foo'),
- terse: true);
- expect(folded.toString(), equals('''
+ var folded = trace.foldFrames((frame) => frame.member.startsWith('foo'),
+ terse: true);
+ expect(folded.toString(), equals('''
foo.dart 42:21 notFoo
package:foo fooBottom
bar.dart 10:20 alsoNotFoo
foo fooBottom
'''));
+ });
+
+ test('.foldFrames will never fold unparsed frames', () {
+ var trace = new Trace.parse(r'''
+.g"cs$#:b";a#>sw{*{ul$"$xqwr`p
+%+j-?uppx<([j@#nu{{>*+$%x-={`{
+!e($b{nj)zs?cgr%!;bmw.+$j+pfj~
+''');
+
+ expect(trace.foldFrames((frame) => true).toString(), equals(r'''
+.g"cs$#:b";a#>sw{*{ul$"$xqwr`p
+%+j-?uppx<([j@#nu{{>*+$%x-={`{
+!e($b{nj)zs?cgr%!;bmw.+$j+pfj~
+'''));
+ });
});
}