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

lib/source_map_stack_trace.dart

 // 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.
 
 import 'package:package_resolver/package_resolver.dart';
 import 'package:path/path.dart' as p;
 import 'package:source_maps/source_maps.dart';
 import 'package:stack_trace/stack_trace.dart';
 
 /// Convert [stackTrace], a stack trace generated by dart2js-compiled
 /// JavaScript, to a native-looking stack trace using [sourceMap].
 ///
 /// [minified] indicates whether or not the dart2js code was minified. If it
 /// hasn't, this tries to clean up the stack frame member names.
 ///
 /// If [packageResolver] is passed, it's used to reconstruct `package:` URIs for
 /// stack frames that come from packages.
 ///
 /// [sdkRoot] is the URI (usually a `file:` URI) for the SDK containing dart2js.
 /// 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(
           "packageResolver and packageRoot may not both be passed.");
     }
 
     packageResolver = new SyncPackageResolver.root(packageRoot);
   }
 
   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));
     }));
   }
 
   if (sdkRoot != null && sdkRoot is! String && sdkRoot is! Uri) {
     throw new ArgumentError(
         'sdkRoot must be a String or a Uri, was "$sdkRoot".');
   }
 
   var sdkLib = sdkRoot == null ? null : "$sdkRoot/lib";
 
   var trace = new Trace.from(stackTrace);
   return new Trace(trace.frames.map((frame) {
     // If there's no line information, there's no way to translate this frame.
     // We could return it as-is, but these lines are usually not useful anyways.
     if (frame.line == null) return null;
 
     // 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)) {
       sourceUrl = "dart:" + p.url.relative(sourceUrl, from: sdkLib);
     } 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;
         }
       }
     }
 
     return new Frame(
         Uri.parse(sourceUrl),
         span.start.line + 1,
         span.start.column + 1,
         // If the dart2js output is minified, there's no use trying to prettify
         // its member names. Use the span's identifier if available, otherwise
         // use the minified member name.
         minified
             ? (span.isIdentifier ? span.text : frame.member)
             : _prettifyMember(frame.member));
   }).where((frame) => frame != null));
 }
 
 /// Reformats a JS member name to make it look more Dart-like.
 String _prettifyMember(String member) {
   return member
       // Get rid of the noise that Firefox sometimes adds.
       .replaceAll(new RegExp(r"/?<$"), "")
       // 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)
       // Get rid of explicitly-generated calls.
       .replaceAll(new RegExp(r"\.call$"), "")
       // Get rid of the top-level method prefix.
       .replaceAll(new RegExp(r"^dart\."), "")
       // Get rid of library namespaces.
       .replaceAll(new RegExp(r"[a-zA-Z_0-9]+\$"), "")
       // Get rid of the static method prefix. The class name also exists in the
       // invocation, so we're not getting rid of any information.
       .replaceAll(new RegExp(r"^[a-zA-Z_0-9]+.(static|dart)."), "")
       // 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] + ".");
 }