Add the ability to disable chain-tracking. (flutter#17)

nex3 · web-flow · commit e75279d3cd8d · 2016-11-15T16:33:14.000-08:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,10 @@
+## 1.7.0
+
+* Add a `Chain.disable()` function that disables stack-chain tracking.
+
+* Fix a bug where `Chain.capture(..., when: false)` would throw if an error was
+  emitted without a stack trace.
+
 ## 1.6.8
 
 * Add a note to the documentation of `Chain.terse` and `Trace.terse`.
diff --git a/lib/src/chain.dart b/lib/src/chain.dart
@@ -14,6 +14,9 @@ import 'utils.dart';
 @Deprecated("Will be removed in stack_trace 2.0.0.")
 typedef void ChainHandler(error, Chain chain);
 
+/// An opaque key used to track the current [StackZoneSpecification].
+final _specKey = new Object();
+
 /// A chain of stack traces.
 ///
 /// A stack chain is a collection of one or more stack traces that collectively
@@ -43,8 +46,7 @@ class Chain implements StackTrace {
   final List<Trace> traces;
 
   /// The [StackZoneSpecification] for the current zone.
-  static StackZoneSpecification get _currentSpec =>
-    Zone.current[#stack_trace.stack_zone.spec];
+  static StackZoneSpecification get _currentSpec => Zone.current[_specKey];
 
   /// If [when] is `true`, runs [callback] in a [Zone] in which the current
   /// stack chain is tracked and automatically associated with (most) errors.
@@ -73,7 +75,11 @@ class Chain implements StackTrace {
       var newOnError;
       if (onError != null) {
         newOnError = (error, stackTrace) {
-          onError(error, new Chain.forTrace(stackTrace));
+          onError(
+              error,
+              stackTrace == null
+                  ? new Chain.current()
+                  : new Chain.forTrace(stackTrace));
         };
       }
 
@@ -89,11 +95,27 @@ class Chain implements StackTrace {
         return Zone.current.handleUncaughtError(error, stackTrace);
       }
     }, zoneSpecification: spec.toSpec(), zoneValues: {
-      #stack_trace.stack_zone.spec: spec
+      _specKey: spec,
+      StackZoneSpecification.disableKey: false
     }) as dynamic/*=T*/;
     // TODO(rnystrom): Remove this cast if runZoned() gets a generic type.
   }
 
+  /// If [when] is `true` and this is called within a [Chain.capture] zone, runs
+  /// [callback] in a [Zone] in which chain capturing is disabled.
+  ///
+  /// If [callback] returns a value, it will be returned by [disable] as well.
+  static /*=T*/ disable/*<T>*/(/*=T*/ callback(), {bool when: true}) {
+    var zoneValues = when
+        ? {
+            _specKey: null,
+            StackZoneSpecification.disableKey: true
+          }
+        : null;
+
+    return runZoned(callback, zoneValues: zoneValues);
+  }
+
   /// Returns [futureOrStream] unmodified.
   ///
   /// Prior to Dart 1.7, this was necessary to ensure that stack traces for
diff --git a/lib/src/stack_zone_specification.dart b/lib/src/stack_zone_specification.dart
@@ -30,6 +30,15 @@ typedef void _ChainHandler(error, Chain chain);
 /// Since [ZoneSpecification] can't be extended or even implemented, in order to
 /// get a real [ZoneSpecification] instance it's necessary to call [toSpec].
 class StackZoneSpecification {
+  /// An opaque object used as a zone value to disable chain tracking in a given
+  /// zone.
+  ///
+  /// If `Zone.current[disableKey]` is `true`, no stack chains will be tracked.
+  static final disableKey = new Object();
+
+  /// Whether chain-tracking is disabled in the current zone.
+  bool get _disabled => Zone.current[disableKey] == true;
+
   /// The expando that associates stack chains with [StackTrace]s.
   ///
   /// The chains are associated with stack traces rather than errors themselves
@@ -54,11 +63,11 @@ class StackZoneSpecification {
   /// Converts [this] to a real [ZoneSpecification].
   ZoneSpecification toSpec() {
     return new ZoneSpecification(
-        handleUncaughtError: handleUncaughtError,
-        registerCallback: registerCallback,
-        registerUnaryCallback: registerUnaryCallback,
-        registerBinaryCallback: registerBinaryCallback,
-        errorCallback: errorCallback);
+        handleUncaughtError: _handleUncaughtError,
+        registerCallback: _registerCallback,
+        registerUnaryCallback: _registerUnaryCallback,
+        registerBinaryCallback: _registerBinaryCallback,
+        errorCallback: _errorCallback);
   }
 
   /// Returns the current stack chain.
@@ -79,57 +88,20 @@ class StackZoneSpecification {
     return new _Node(trace, previous).toChain();
   }
 
-  /// Ensures that an error emitted by [future] has the correct stack
-  /// information associated with it.
-  ///
-  /// By default, the first frame of the first trace will be the line where
-  /// [trackFuture] is called. If [level] is passed, the first trace will start
-  /// that many frames up instead.
-  Future trackFuture(Future future, [int level=0]) {
-    var completer = new Completer.sync();
-    var node = _createNode(level + 1);
-    future.then(completer.complete).catchError((e, stackTrace) {
-      if (stackTrace == null) stackTrace = new Trace.current();
-      if (stackTrace is! Chain && _chains[stackTrace] == null) {
-        _chains[stackTrace] = node;
-      }
-      completer.completeError(e, stackTrace);
-    });
-    return completer.future;
-  }
-
-  /// Ensures that any errors emitted by [stream] have the correct stack
-  /// information associated with them.
-  ///
-  /// By default, the first frame of the first trace will be the line where
-  /// [trackStream] is called. If [level] is passed, the first trace will start
-  /// that many frames up instead.
-  Stream trackStream(Stream stream, [int level=0]) {
-    var node = _createNode(level + 1);
-    return stream.transform(new StreamTransformer.fromHandlers(
-        handleError: (error, stackTrace, sink) {
-      if (stackTrace == null) stackTrace = new Trace.current();
-      if (stackTrace is! Chain && _chains[stackTrace] == null) {
-        _chains[stackTrace] = node;
-      }
-      sink.addError(error, stackTrace);
-    }));
-  }
-
   /// Tracks the current stack chain so it can be set to [_currentChain] when
   /// [f] is run.
-  ZoneCallback registerCallback(Zone self, ZoneDelegate parent, Zone zone,
+  ZoneCallback _registerCallback(Zone self, ZoneDelegate parent, Zone zone,
       Function f) {
-    if (f == null) return parent.registerCallback(zone, null);
+    if (f == null || _disabled) return parent.registerCallback(zone, f);
     var node = _createNode(1);
     return parent.registerCallback(zone, () => _run(f, node));
   }
 
   /// Tracks the current stack chain so it can be set to [_currentChain] when
   /// [f] is run.
-  ZoneUnaryCallback registerUnaryCallback(Zone self, ZoneDelegate parent,
+  ZoneUnaryCallback _registerUnaryCallback(Zone self, ZoneDelegate parent,
       Zone zone, Function f) {
-    if (f == null) return parent.registerUnaryCallback(zone, null);
+    if (f == null || _disabled) return parent.registerUnaryCallback(zone, f);
     var node = _createNode(1);
     return parent.registerUnaryCallback(zone, (arg) {
       return _run(() => f(arg), node);
@@ -138,9 +110,10 @@ class StackZoneSpecification {
 
   /// Tracks the current stack chain so it can be set to [_currentChain] when
   /// [f] is run.
-  ZoneBinaryCallback registerBinaryCallback(Zone self, ZoneDelegate parent,
+  ZoneBinaryCallback _registerBinaryCallback(Zone self, ZoneDelegate parent,
       Zone zone, Function f) {
-    if (f == null) return parent.registerBinaryCallback(zone, null);
+    if (f == null || _disabled) return parent.registerBinaryCallback(zone, f);
+
     var node = _createNode(1);
     return parent.registerBinaryCallback(zone, (arg1, arg2) {
       return _run(() => f(arg1, arg2), node);
@@ -149,8 +122,12 @@ class StackZoneSpecification {
 
   /// Looks up the chain associated with [stackTrace] and passes it either to
   /// [_onError] or [parent]'s error handler.
-  handleUncaughtError(Zone self, ZoneDelegate parent, Zone zone, error,
+  _handleUncaughtError(Zone self, ZoneDelegate parent, Zone zone, error,
       StackTrace stackTrace) {
+    if (_disabled) {
+      return parent.handleUncaughtError(zone, error, stackTrace);
+    }
+
     var stackChain = chainFor(stackTrace);
     if (_onError == null) {
       return parent.handleUncaughtError(zone, error, stackChain);
@@ -171,8 +148,10 @@ class StackZoneSpecification {
 
   /// Attaches the current stack chain to [stackTrace], replacing it if
   /// necessary.
-  AsyncError errorCallback(Zone self, ZoneDelegate parent, Zone zone,
+  AsyncError _errorCallback(Zone self, ZoneDelegate parent, Zone zone,
       Object error, StackTrace stackTrace) {
+    if (_disabled) return parent.errorCallback(zone, error, stackTrace);
+
     // Go up two levels to get through [_CustomZone.errorCallback].
     if (stackTrace == null) {
       stackTrace = _createNode(2).toChain();
diff --git a/pubspec.yaml b/pubspec.yaml
@@ -7,7 +7,7 @@ name: stack_trace
 #
 # When the major version is upgraded, you *must* update that version constraint
 # in pub to stay in sync with this.
-version: 1.6.8
+version: 1.7.0
 author: "Dart Team <misc@dartlang.org>"
 homepage: https://github.com/dart-lang/stack_trace
 description: >
diff --git a/test/chain/chain_test.dart b/test/chain/chain_test.dart
@@ -8,6 +8,7 @@ import 'package:path/path.dart' as p;
 import 'package:stack_trace/stack_trace.dart';
 import 'package:test/test.dart';
 
+import '../utils.dart';
 import 'utils.dart';
 
 typedef void ChainErrorCallback(stack, Chain chain);
@@ -53,6 +54,82 @@ void main() {
       }) as ChainErrorCallback, when: false);
       // TODO(rnystrom): Remove this cast if expectAsync() gets a better type.
     });
+
+    test("doesn't enable chain-tracking", () {
+      return Chain.disable(() {
+        return Chain.capture(() {
+          var completer = new Completer();
+          inMicrotask(() {
+            completer.complete(new Chain.current());
+          });
+
+          return completer.future.then((chain) {
+            expect(chain.traces, hasLength(1));
+          });
+        }, when: false);
+      });
+    });
+  });
+
+  group("Chain.disable()", () {
+    test("disables chain-tracking", () {
+      return Chain.disable(() {
+        var completer = new Completer();
+        inMicrotask(() => completer.complete(new Chain.current()));
+
+        return completer.future.then((chain) {
+          expect(chain.traces, hasLength(1));
+        });
+      });
+    });
+
+    test("Chain.capture() re-enables chain-tracking", () {
+      return Chain.disable(() {
+        return Chain.capture(() {
+          var completer = new Completer();
+          inMicrotask(() => completer.complete(new Chain.current()));
+
+          return completer.future.then((chain) {
+            expect(chain.traces, hasLength(2));
+          });
+        });
+      });
+    });
+
+    test("preserves parent zones of the capture zone", () {
+      // The outer disable call turns off the test package's chain-tracking.
+      return Chain.disable(() {
+        return runZoned(() {
+          return Chain.capture(() {
+            expect(Chain.disable(() => Zone.current[#enabled]), isTrue);
+          });
+        }, zoneValues: {#enabled: true});
+      });
+    });
+
+    test("preserves child zones of the capture zone", () {
+      // The outer disable call turns off the test package's chain-tracking.
+      return Chain.disable(() {
+        return Chain.capture(() {
+          return runZoned(() {
+            expect(Chain.disable(() => Zone.current[#enabled]), isTrue);
+          }, zoneValues: {#enabled: true});
+        });
+      });
+    });
+
+    test("with when: false doesn't disable", () {
+      return Chain.capture(() {
+        return Chain.disable(() {
+          var completer = new Completer();
+          inMicrotask(() => completer.complete(new Chain.current()));
+
+          return completer.future.then((chain) {
+            expect(chain.traces, hasLength(2));
+          });
+        }, when: false);
+      });
+    });
   });
 
   test("toString() ensures that all traces are aligned", () {
@@ -248,68 +325,4 @@ void main() {
         '$userSlashCode 10:11  Foo.bar\n'
         'dart:core 10:11       Bar.baz\n'));
   });
-
-  group('Chain.track(Future)', () {
-    test('forwards the future value within Chain.capture()', () {
-      Chain.capture(() {
-        expect(Chain.track(new Future.value('value')),
-            completion(equals('value')));
-
-        var trace = new Trace.current();
-        expect(Chain.track(new Future.error('error', trace))
-            .catchError((e, stackTrace) {
-          expect(e, equals('error'));
-          expect(stackTrace.toString(), equals(trace.toString()));
-        }), completes);
-      });
-    });
-
-    test('forwards the future value outside of Chain.capture()', () {
-      expect(Chain.track(new Future.value('value')),
-          completion(equals('value')));
-
-      var trace = new Trace.current();
-      expect(Chain.track(new Future.error('error', trace))
-          .catchError((e, stackTrace) {
-        expect(e, equals('error'));
-        expect(stackTrace.toString(), equals(trace.toString()));
-      }), completes);
-    });
-  });
-
-  group('Chain.track(Stream)', () {
-    test('forwards stream values within Chain.capture()', () {
-      Chain.capture(() {
-        var controller = new StreamController()
-            ..add(1)..add(2)..add(3)..close();
-        expect(Chain.track(controller.stream).toList(),
-            completion(equals([1, 2, 3])));
-
-        var trace = new Trace.current();
-        controller = new StreamController()..addError('error', trace);
-        expect(Chain.track(controller.stream).toList()
-            .catchError((e, stackTrace) {
-          expect(e, equals('error'));
-          expect(stackTrace.toString(), equals(trace.toString()));
-        }), completes);
-      });
-    });
-
-    test('forwards stream values outside of Chain.capture()', () {
-      Chain.capture(() {
-        var controller = new StreamController()
-            ..add(1)..add(2)..add(3)..close();
-        expect(Chain.track(controller.stream).toList(),
-            completion(equals([1, 2, 3])));
-
-        var trace = new Trace.current();
-        controller = new StreamController()..addError('error', trace);
-        expect(Chain.track(controller.stream).toList()
-            .catchError((e, stackTrace) {
-          expect(e, equals('error'));
-          expect(stackTrace.toString(), equals(trace.toString()));
-        }), completes);
-      });
-    });
-  });
 }
diff --git a/test/chain/dart2js_test.dart b/test/chain/dart2js_test.dart
@@ -240,9 +240,8 @@ void main() {
 
   test('current() outside of capture() returns a chain wrapping the current '
       'trace', () {
-    // The test runner runs all tests with chains enabled, so to test without we
-    // have to do some zone munging.
-    return runZoned(() async {
+    // The test runner runs all tests with chains enabled.
+    return Chain.disable(() async {
       var completer = new Completer();
       inMicrotask(() => completer.complete(new Chain.current()));
 
@@ -251,7 +250,7 @@ void main() {
       // chain isn't available and it just returns the current stack when
       // called.
       expect(chain.traces, hasLength(1));
-    }, zoneValues: {#stack_trace.stack_zone.spec: null});
+    });
   });
 
   group('forTrace() within capture()', () {
diff --git a/test/chain/vm_test.dart b/test/chain/vm_test.dart

Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@ name: stack_trace`
`7`	`7`	`#`
`8`	`8`	`# When the major version is upgraded, you must update that version constraint`
`9`	`9`	`# in pub to stay in sync with this.`
`10`		`-version: 1.6.8`
	`10`	`+version: 1.7.0`
`11`	`11`	`author: "Dart Team <[email protected]>"`
`12`	`12`	`homepage: https://github.com/dart-lang/stack_trace`
`13`	`13`	`description: >`