Fix behavior causing frames lacking a source map to always be omitted. Add `includeUnmappedFrames` …

lib/source_map_stack_trace.dart

Index: lib/source_map_stack_trace.dart
diff --git a/lib/source_map_stack_trace.dart b/lib/source_map_stack_trace.dart
index ae2f124d37219bde279cf5fb6d49e8ae8327627b..1229d8aa789fc644e0b1cadeafb7183366c90607 100644
--- a/lib/source_map_stack_trace.dart
+++ b/lib/source_map_stack_trace.dart
@@ -20,11 +20,17 @@ import 'package:stack_trace/stack_trace.dart';
 /// It can be a [String] or a [Uri]. If it's passed, stack frames from the SDK
 /// will have `dart:` URLs.
 ///
+/// [includeUnmappedFrames] indicates whether frames that do not match
+/// [sourceMap] should be included in the mapped stack trace.

[nweiz, 2016/12/08 23:34:31]

What happens to those frames if it's `false`?

[Jacob, 2016/12/09 02:18:28]

if it is false they are not included.

+///
 /// [packageRoot] is deprecated and shouldn't be used in new code. This throws
 /// an [ArgumentError] if [packageRoot] and [packageResolver] are both passed.
 StackTrace mapStackTrace(Mapping sourceMap, StackTrace stackTrace,
-    {bool minified: false, SyncPackageResolver packageResolver, sdkRoot,
-    @Deprecated("Use the packageResolver parameter instead.") packageRoot}) {
+    {bool minified: false,
+    SyncPackageResolver packageResolver,
+    sdkRoot,
+    @Deprecated("Use the packageResolver parameter instead.") packageRoot,
+    bool includeUnmappedFrames: false}) {
   if (packageRoot != null) {
     if (packageResolver != null) {
       throw new ArgumentError(
@@ -36,11 +42,11 @@ StackTrace mapStackTrace(Mapping sourceMap, StackTrace stackTrace,
 
   if (stackTrace is Chain) {
     return new Chain(stackTrace.traces.map((trace) {
-      return new Trace.from(mapStackTrace(
-          sourceMap, trace,
+      return new Trace.from(mapStackTrace(sourceMap, trace,
           minified: minified,
           packageResolver: packageResolver,
-          sdkRoot: sdkRoot));
+          sdkRoot: sdkRoot,
+          includeUnmappedFrames: includeUnmappedFrames));
     }));
   }
 
@@ -59,14 +65,16 @@ StackTrace mapStackTrace(Mapping sourceMap, StackTrace stackTrace,
 
     // If there's no column, try using the first column of the line.
     var column = frame.column == null ? 0 : frame.column;
+    var uri = frame.uri;
 
     // Subtract 1 because stack traces use 1-indexed lines and columns and
     // source maps uses 0-indexed.
-    var span = sourceMap.spanFor(frame.line - 1, column - 1);
+    var span =
+        sourceMap.spanFor(frame.line - 1, column - 1, uri: uri?.toString());
 
-    // If we can't find a source span, ignore the frame. It's probably something
-    // internal that the user doesn't care about.

[nweiz, 2016/12/08 23:34:31]

This comment should still exist, maybe reworded a bit to fit the new code.

[Jacob, 2016/12/09 20:21:43]

obsolete.

-    if (span == null) return null;
+    if (span == null) {
+      return includeUnmappedFrames ? frame : null;
+    }
 
     var sourceUrl = span.sourceUrl.toString();
     if (sdkRoot != null && p.url.isWithin(sdkLib, sourceUrl)) {
@@ -74,15 +82,16 @@ StackTrace mapStackTrace(Mapping sourceMap, StackTrace stackTrace,
     } else if (packageResolver != null) {
       if (packageResolver.packageRoot != null &&
           p.url.isWithin(packageResolver.packageRoot.toString(), sourceUrl)) {
-        sourceUrl = "package:" + p.url.relative(sourceUrl,
-            from: packageResolver.packageRoot.toString());
+        sourceUrl = "package:" +
+            p.url.relative(sourceUrl,
+                from: packageResolver.packageRoot.toString());

[nweiz, 2016/12/08 23:34:31]

Please don't include unrelated formatting changes.

[Jacob, 2016/12/09 02:18:28]

I'll send a followup cl that runs dartfm. would be nice if using a dart editor
that automatically runs dartfmt didn't mess up this file.

       } else if (packageResolver.packageConfigMap != null) {
         for (var package in packageResolver.packageConfigMap.keys) {
           var packageUrl = packageResolver.packageConfigMap[package].toString();
           if (!p.url.isWithin(packageUrl, sourceUrl)) continue;
 
-          sourceUrl = "package:$package/" +
-              p.url.relative(sourceUrl, from: packageUrl);
+          sourceUrl =
+              "package:$package/" + p.url.relative(sourceUrl, from: packageUrl);
           break;
         }
       }
@@ -109,7 +118,8 @@ String _prettifyMember(String member) {
       // Get rid of arity indicators and named arguments.
       .replaceAll(new RegExp(r"\$\d+(\$[a-zA-Z_0-9]+)*$"), "")
       // Convert closures to <fn>.
-      .replaceAllMapped(new RegExp(r"(_+)closure\d*\.call$"),
+      .replaceAllMapped(
+          new RegExp(r"(_+)closure\d*\.call$"),
           // The number of underscores before "closure" indicates how nested it
           // is.
           (match) => ".<fn>" * match[1].length)
@@ -125,6 +135,6 @@ String _prettifyMember(String member) {
       // Convert underscores after identifiers to dots. This runs the risk of
       // incorrectly converting members that contain underscores, but those are
       // contrary to the style guide anyway.
-      .replaceAllMapped(new RegExp(r"([a-zA-Z0-9]+)_"),
-          (match) => match[1] + ".");
+      .replaceAllMapped(
+          new RegExp(r"([a-zA-Z0-9]+)_"), (match) => match[1] + ".");
 }