Merge pull request #35 from dart-lang/trim-vm-chains
Improve handling of VM stack chains
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 19fb82b..b0c972a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,18 @@
+## 1.8.3
+
+* `Chain.forTrace()` now returns a full stack chain for *all* `StackTrace`s
+ within `Chain.capture()`, even those that haven't been processed by
+ `dart:async` yet.
+
+* `Chain.forTrace()` now uses the Dart VM's stack chain information when called
+ synchronously within `Chain.capture()`. This matches the existing behavior
+ outside `Chain.capture()`.
+
+* `Chain.forTrace()` now trims the VM's stack chains for the innermost stack
+ trace within `Chain.capture()` (unless it's called synchronously, as above).
+ This avoids duplicated frames and makes the format of the innermost traces
+ consistent with the other traces in the chain.
+
## 1.8.2
* Update to use strong-mode clean Zone API.
diff --git a/lib/src/chain.dart b/lib/src/chain.dart
index 01bf06f..56c5333 100644
--- a/lib/src/chain.dart
+++ b/lib/src/chain.dart
@@ -153,6 +153,7 @@
factory Chain.forTrace(StackTrace trace) {
if (trace is Chain) return trace;
if (_currentSpec != null) return _currentSpec.chainFor(trace);
+ if (trace is Trace) return new Chain([trace]);
return new LazyChain(() => new Chain.parse(trace.toString()));
}
diff --git a/lib/src/stack_zone_specification.dart b/lib/src/stack_zone_specification.dart
index 6749d56..cd5d068 100644
--- a/lib/src/stack_zone_specification.dart
+++ b/lib/src/stack_zone_specification.dart
@@ -5,6 +5,7 @@
import 'dart:async';
import 'chain.dart';
+import 'lazy_chain.dart';
import 'lazy_trace.dart';
import 'trace.dart';
import 'utils.dart';
@@ -86,8 +87,23 @@
/// with [trace], this just returns a single-trace chain containing [trace].
Chain chainFor(StackTrace trace) {
if (trace is Chain) return trace;
- var previous = trace == null ? null : _chains[trace];
- return new _Node(trace, previous).toChain();
+ trace ??= StackTrace.current;
+
+ var previous = _chains[trace] ?? _currentNode;
+ if (previous == null) {
+ // If there's no [_currentNode], we're running synchronously beneath
+ // [Chain.capture] and we should fall back to the VM's stack chaining. We
+ // can't use [Chain.from] here because it'll just call [chainFor] again.
+ if (trace is Trace) return new Chain([trace]);
+ return new LazyChain(() => new Chain.parse(trace.toString()));
+ } else {
+ if (trace is! Trace) {
+ var original = trace;
+ trace = new LazyTrace(() => new Trace.parse(_trimVMChain(original)));
+ }
+
+ return new _Node(trace, previous).toChain();
+ }
}
/// Tracks the current stack chain so it can be set to [_currentChain] when
@@ -194,6 +210,29 @@
_currentNode = previousNode;
}
}
+
+ /// Like [new Trace.current], but if the current stack trace has VM chaining
+ /// enabled, this only returns the innermost sub-trace.
+ Trace _currentTrace([int level]) {
+ level ??= 0;
+ var stackTrace = StackTrace.current;
+ return new LazyTrace(() {
+ var text = _trimVMChain(stackTrace);
+ var trace = new Trace.parse(text);
+ // JS includes a frame for the call to StackTrace.current, but the VM
+ // doesn't, so we skip an extra frame in a JS context.
+ return new Trace(trace.frames.skip(level + (inJS ? 2 : 1)),
+ original: text);
+ });
+ }
+
+ /// Removes the VM's stack chains from the native [trace], since we're
+ /// generating our own and we don't want duplicate frames.
+ String _trimVMChain(StackTrace trace) {
+ var text = trace.toString();
+ var index = text.indexOf(vmChainGap);
+ return index == -1 ? text : text.substring(0, index);
+ }
}
/// A linked list node representing a single entry in a stack chain.
@@ -204,8 +243,7 @@
/// The previous node in the chain.
final _Node previous;
- _Node(StackTrace trace, [this.previous])
- : trace = trace == null ? _currentTrace() : new Trace.from(trace);
+ _Node(StackTrace trace, [this.previous]) : trace = new Trace.from(trace);
/// Converts this to a [Chain].
Chain toChain() {
@@ -218,22 +256,3 @@
return new Chain(nodes);
}
}
-
-/// Like [new Trace.current], but if the current stack trace has VM chaining
-/// enabled, this only returns the innermost sub-trace.
-Trace _currentTrace([int level]) {
- level ??= 0;
- var stackTrace = StackTrace.current;
- return new LazyTrace(() {
- // Ignore the VM's stack chains when we generate our own. Otherwise we'll
- // end up with duplicate frames all over the place.
- var text = stackTrace.toString();
- var index = text.indexOf(vmChainGap);
- if (index != -1) text = text.substring(0, index);
-
- var trace = new Trace.parse(text);
- // JS includes a frame for the call to StackTrace.current, but the VM
- // doesn't, so we skip an extra frame in a JS context.
- return new Trace(trace.frames.skip(level + (inJS ? 2 : 1)), original: text);
- });
-}
diff --git a/pubspec.yaml b/pubspec.yaml
index a6e5b92..65d3ef6 100644
--- a/pubspec.yaml
+++ b/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.8.2
+version: 1.8.3
author: "Dart Team <misc@dartlang.org>"
homepage: https://github.com/dart-lang/stack_trace
description: A package for manipulating stack traces and printing them readably.
diff --git a/test/chain/vm_test.dart b/test/chain/vm_test.dart
index daf7f8f..a47cbf7 100644
--- a/test/chain/vm_test.dart
+++ b/test/chain/vm_test.dart
@@ -8,20 +8,31 @@
import 'dart:async';
-import 'package:stack_trace/stack_trace.dart';
import 'package:test/test.dart';
+import 'package:stack_trace/stack_trace.dart';
+import 'package:stack_trace/src/utils.dart';
+
import '../utils.dart';
import 'utils.dart';
void main() {
group('capture() with onError catches exceptions', () {
- test('thrown synchronously', () {
- return captureFuture(() => throw 'error').then((chain) {
- expect(chain.traces, hasLength(1));
- expect(
- chain.traces.single.frames.first, frameMember(startsWith('main')));
+ test('thrown synchronously', () async {
+ StackTrace vmTrace;
+ var chain = await captureFuture(() {
+ try {
+ throw 'error';
+ } catch (_, stackTrace) {
+ vmTrace = stackTrace;
+ rethrow;
+ }
});
+
+ // Because there's no chain context for a synchronous error, we fall back
+ // on the VM's stack chain tracking.
+ expect(chain.toString(),
+ equals(new Chain.parse(vmTrace.toString()).toString()));
});
test('thrown in a microtask', () {
@@ -444,11 +455,10 @@
});
});
- test(
- 'called for an unregistered stack trace returns a chain wrapping that '
- 'trace', () {
+ test('called for an unregistered stack trace uses the current chain',
+ () async {
var trace;
- var chain = Chain.capture(() {
+ var chain = await Chain.capture(() async {
try {
throw 'error';
} catch (_, stackTrace) {
@@ -457,27 +467,36 @@
}
});
- expect(chain.traces, hasLength(1));
+ expect(chain.traces, hasLength(2));
+
+ // Assert that we've trimmed the VM's stack chains here to avoid
+ // duplication.
expect(chain.traces.first.toString(),
- equals(new Trace.from(trace).toString()));
+ equals(new Chain.parse(trace.toString()).traces.first.toString()));
+ expect(
+ chain.traces.last.frames, contains(frameMember(startsWith('main'))));
});
});
test(
- 'forTrace() outside of capture() returns a chain wrapping the given '
- 'trace', () {
- var trace;
- var chain = Chain.capture(() {
- try {
- throw 'error';
- } catch (_, stackTrace) {
- trace = stackTrace;
- return new Chain.forTrace(stackTrace);
- }
- });
+ 'forTrace() outside of capture() returns a chain describing the VM stack '
+ 'chain', () {
+ // Disable the test package's chain-tracking.
+ return Chain.disable(() async {
+ var trace;
+ await Chain.capture(() async {
+ try {
+ throw 'error';
+ } catch (_, stackTrace) {
+ trace = stackTrace;
+ }
+ });
- expect(chain.traces, hasLength(1));
- expect(chain.traces.first.toString(),
- equals(new Trace.from(trace).toString()));
+ var chain = new Chain.forTrace(trace);
+ expect(chain.traces,
+ hasLength(vmChainGap.allMatches(trace.toString()).length + 1));
+ expect(
+ chain.traces.first.frames, contains(frameMember(startsWith('main'))));
+ });
});
}
