Incremental changes in error reporting

chrome/browser/resources/google_now/utility.js

Index: chrome/browser/resources/google_now/utility.js
diff --git a/chrome/browser/resources/google_now/utility.js b/chrome/browser/resources/google_now/utility.js
index e242739d5c0daafce80a2f077975e074d015a745..50cf2a7b3b3847aae3bee581048003d6ccd3f64e 100644
--- a/chrome/browser/resources/google_now/utility.js
+++ b/chrome/browser/resources/google_now/utility.js
@@ -19,15 +19,6 @@ var NOTIFICATION_CARDS_URL = localStorage['server_url'];
 
 var DEBUG_MODE = localStorage['debug_mode'];
 
-/**
- * Shows a message popup in debug mode.
- * @param {string} message Diagnostic message.
- */
-function debugAlert(message) {
-  if (DEBUG_MODE)
-    alert(message);
-}
-
 /**
  * Checks for internal errors.
  * @param {boolean} condition Condition that must be true.
@@ -101,14 +92,8 @@ function buildTaskManager(areConflicting) {
    * Checks that we run in an instrumented callback.
    */
   function checkInInstrumentedCallback() {
-    if (!isInInstrumentedCallback) {
-      // Cannot use verify() since no one will catch the exception.
-      // This check will detect bugs at the development stage, and is very
-      // unlikely to be seen by users.
-      var error = 'Not in instrumented callback: ' + new Error().stack;
-      console.error(error);
-      debugAlert(error);
-    }
+    if (!isInInstrumentedCallback)
+      reportError(new Error('\nNot in instrumented callback'));
   }
 
   /**
@@ -193,29 +178,44 @@ function buildTaskManager(areConflicting) {
 
   /**
    * Sends an error report to the server.
-   * @param {Error} error Error to report.
+   * @param {Error} error Error to send.
    */
   function sendErrorReport(error) {
     var filteredStack = error.stack.replace(/.*\n/, '\n');
     var file;
     var line;
-    var topFrameMatches = filteredStack.match(/\(.*\)/);
-    // topFrameMatches's example:
-    // (chrome-extension://pmofbkohncoogjjhahejjfbppikbjigm/utility.js:308:19)
-    var crashLocation = topFrameMatches && topFrameMatches[0];
-    if (crashLocation) {
-      var topFrameElements =
-          crashLocation.substring(1, crashLocation.length - 1).split(':');
-      // topFrameElements for the above example will look like:
-      // [0] chrome-extension
-      // [1] //pmofbkohncoogjjhahejjfbppikbjigm/utility.js
-      // [2] 308
-      // [3] 19
+    var topFrameLineMatch = filteredStack.match(/\n    at .*\n/);

[skare_, 2013/08/07 18:40:38]

seems like a good place for a test :D

+    var topFrame = topFrameLineMatch && topFrameLineMatch[0];
+    if (topFrame) {
+      // Examples of a frame:
+      // 1. '\n    at someFunction (chrome-extension://
+      //     pmofbkohncoogjjhahejjfbppikbjigm/background.js:915:15)\n'
+      // 2. '\n    at chrome-extension://pmofbkohncoogjjhahejjfbppikbjigm/
+      //     utility.js:269:18\n'
+      // 3. '\n    at Function.target.(anonymous function) (extensions::
+      //     SafeBuiltins:19:14)\n'
+      // 4. '\n    at Event.dispatchToListener (event_bindings:382:22)\n'
+      var errorLocation;
+      // Find the the parentheses at the end of the line, if any.
+      var parenthesesMatch = topFrame.match(/\(.*\)\n/);
+      if (parenthesesMatch && parenthesesMatch[0]) {
+        errorLocation =
+            parenthesesMatch[0].substring(1, parenthesesMatch[0].length - 2);
+      } else {
+        errorLocation = topFrame;
+      }
+
+      var topFrameElements = errorLocation.split(':');
+      // topFrameElements is an array that ends like:
+      // [N-3] //pmofbkohncoogjjhahejjfbppikbjigm/utility.js
+      // [N-2] 308
+      // [N-1] 19
       if (topFrameElements.length >= 3) {
-        file = topFrameElements[0] + ':' + topFrameElements[1];
-        line = topFrameElements[2];
+        file = topFrameElements[topFrameElements.length - 3];
+        line = topFrameElements[topFrameElements.length - 2];
       }
     }
+
     var requestParameters =
         'error=' + encodeURIComponent(error.name) +
         '&script=' + encodeURIComponent(file) +
@@ -230,6 +230,24 @@ function buildTaskManager(areConflicting) {
   }
 
   /**
+   * Reports an error to the server and the user, as appropriate.
+   * @param {Error} error Error to report.
+   */
+  function reportError(error) {
+    var message = 'Critical error:\n' + error.stack;
+    console.error(message);
+    if (!errorReported) {
+      errorReported = true;
+      chrome.metricsPrivate.getIsCrashReportingEnabled(function(isEnabled) {
+        if (isEnabled)
+          sendErrorReport(error);
+        if (DEBUG_MODE)
+          alert(message);
+      });
+    }
+  }
+
+  /**
    * Unique ID of the next callback.
    * @type {number}
    */
@@ -279,16 +297,7 @@ function buildTaskManager(areConflicting) {
             finish();
         }
       } catch (error) {
-        var message = 'Uncaught exception:\n' + error.stack;
-        console.error(message);
-        if (!errorReported) {
-          errorReported = true;
-          chrome.metricsPrivate.getIsCrashReportingEnabled(function(isEnabled) {
-            if (isEnabled)
-              sendErrorReport(error);
-          });
-          debugAlert(message);
-        }
+        reportError(error);
       }
     };
   }
@@ -305,15 +314,15 @@ function buildTaskManager(areConflicting) {
     var originalFunction = namespace[functionName];
 
     if (!originalFunction)
-      debugAlert('Cannot instrument ' + functionName);
+      reportError(new Error('\nCannot instrument ' + functionName));
 
     namespace[functionName] = function() {
       // This is the wrapper for the API function. Pass the wrapped callback to
       // the original function.
       var callback = arguments[callbackParameter];
       if (typeof callback != 'function') {
-        debugAlert('Argument ' + callbackParameter + ' of ' + functionName +
-              ' is not a function');
+        reportError(new Error('\nArgument ' + callbackParameter + ' of ' +
+            functionName + ' is not a function'));
       }
       arguments[callbackParameter] = wrapCallback(
           callback, functionName == 'addListener');