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~ +''')); + }); }); }