Report unchecked error in lastError.run

extensions/renderer/resources/last_error.js

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 var GetAvailability = requireNative('v8_context').GetAvailability;
 var GetGlobal = requireNative('sendRequest').GetGlobal;
 
 // Utility for setting chrome.*.lastError.
 //
 // A utility here is useful for two reasons:
 //  1. For backwards compatibility we need to set chrome.extension.lastError,
 //     but not all contexts actually have access to the extension namespace.
 //  2. When calling across contexts, the global object that gets lastError set
 //     needs to be that of the caller. We force callers to explicitly specify
 //     the chrome object to try to prevent bugs here.
 
 /**
  * Sets the last error for |name| on |targetChrome| to |message| with an
  * optional |stack|.
  */
 function set(name, message, stack, targetChrome) {
-  var errorMessage = name + ': ' + message;
-  if (stack != null && stack != '')
-    errorMessage += '\n' + stack;
-
-  if (!targetChrome)
+  if (!targetChrome) {
+    var errorMessage = name + ': ' + message;
+    if (stack != null && stack != '')
+      errorMessage += '\n' + stack;
     throw new Error('No chrome object to set error: ' + errorMessage);
+  }
   clear(targetChrome);  // in case somebody has set a sneaky getter/setter
 
   var errorObject = { message: message };
   if (GetAvailability('extension.lastError').is_available)
     targetChrome.extension.lastError = errorObject;
 
   assertRuntimeIsAvailable();
 
   // We check to see if developers access runtime.lastError in order to decide
   // whether or not to log it in the (error) console.
@@ skipping 68 matching lines @@
  *
  * The target chrome object is the global object's of the callback, so this
  * method won't work if the real callback has been wrapped (etc).
  */
 function run(name, message, stack, callback, args) {
   var targetChrome = GetGlobal(callback).chrome;
   set(name, message, stack, targetChrome);
   try {
     $Function.apply(callback, undefined, args);
   } finally {
+    reportIfUnchecked(name, targetChrome, stack);
     clear(targetChrome);
   }
 }
 
+/**
+ * Checks whether chrome.runtime.lastError has been accessed if set.
+ * If it was set but not accessed, the error is reported to the console.
+ *
+ * @param {string=} name - name of API.
+ * @param {Object} targetChrome - the Chrome object to check.
+ * @param {string=} stack - Stack trace of the call up to the error.
+ */
+function reportIfUnchecked(name, targetChrome, stack) {
+  var error = hasAccessed(targetChrome) && targetChrome.runtime.lastError;

[not at google - send to devlin, 2015/01/22 17:59:59]

targetChrome.runtime.lastError --> hasError(targetChrome)

I'd also call this variable "hasUncheckedError".

[robwu, 2015/01/22 18:07:22]

|error| is used below as |error.message| to get the error message out of it.

[not at google - send to devlin, 2015/01/22 18:12:53]

Alright. Well use that as a reason to not overload a type as both boolean +
string. Please write in a different way.

[robwu, 2015/01/22 18:19:33]

Done.

+  if (!error)
+    return;
+  console.error("Unchecked runtime.lastError while running " +
+      (name || "unknown") + ": " + error.message + (stack ? "\n" + stack : ""));
+}
+
 exports.clear = clear;
 exports.hasAccessed = hasAccessed;
 exports.hasError = hasError;
 exports.set = set;
 exports.run = run;
+exports.reportIfUnchecked = reportIfUnchecked;