Unify logic of stack trace generation for extension errors

extensions/renderer/resources/event.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 exceptionHandler = require('uncaught_exception_handler');
   var eventNatives = requireNative('event_natives');
   var logging = requireNative('logging');
   var schemaRegistry = requireNative('schema_registry');
   var sendRequest = require('sendRequest').sendRequest;
   var utils = require('utils');
@@ skipping 84 matching lines @@
     this.event_ = event;
     this.listenerMap_ = {};
   };
 
   FilteredAttachmentStrategy.idToEventMap = {};
 
   FilteredAttachmentStrategy.prototype.onAddedListener = function(listener) {
     var id = eventNatives.AttachFilteredEvent(this.event_.eventName,
                                               listener.filters || {});
     if (id == -1)
-      throw new Error("Can't add listener");
+      throw new $Error.self("Can't add listener");

[not at google - send to devlin, 2014/08/19 16:45:55]

new Error()

     listener.id = id;
     this.listenerMap_[id] = listener;
     FilteredAttachmentStrategy.idToEventMap[id] = this.event_;
   };
 
   FilteredAttachmentStrategy.prototype.onRemovedListener = function(listener) {
     this.detachListener(listener, true);
   };
 
   FilteredAttachmentStrategy.prototype.detachListener =
       function(listener, manual) {
     if (listener.id == undefined)
-      throw new Error("listener.id undefined - '" + listener + "'");
+      throw new $Error.self("listener.id undefined - '" + listener + "'");
     var id = listener.id;
     delete this.listenerMap_[id];
     delete FilteredAttachmentStrategy.idToEventMap[id];
     eventNatives.DetachFilteredEvent(id, manual);
   };
 
   FilteredAttachmentStrategy.prototype.detach = function(manual) {
     for (var i in this.listenerMap_)
       this.detachListener(this.listenerMap_[i], manual);
   };
@@ skipping 64 matching lines @@
   // extension event rather than a <webview> event.
   var EventImpl = function(opt_eventName, opt_argSchemas, opt_eventOptions,
                            opt_webViewInstanceId) {
     this.eventName = opt_eventName;
     this.argSchemas = opt_argSchemas;
     this.listeners = [];
     this.eventOptions = parseEventOptions(opt_eventOptions);
     this.webViewInstanceId = opt_webViewInstanceId || 0;
 
     if (!this.eventName) {
       if (this.eventOptions.supportsRules)

[not at google - send to devlin, 2014/08/19 16:45:55]

Oh man some of these are so subtle. Errors that we expect the extension to
handle should still be Error. Internal errors should be $Error. Ugh.

I'm wondering whether a correct solution to this would be to have an assert()
function exposed to the custom bindings (similar to how privates() is exposed)
which basically throws an $Error, but continue to throw an Error for those that
are intended to be passed back to the extension. I can't expect future
generations to know the distinction between $Error and Error.

In the meantime let's submit this, and I'll point out the places where we could
continue to throw a new Error. Basically anywhere where the validation is in
response to the extension vs our own internal state.

(in this file it's all those "this event doesn't support blah" and *really* we
should be solving that by having different types for each Event. ah well)

[robwu, 2014/08/19 17:35:57]

Now I'm a bit confused. Why should $Error.self be changed to Error?
$Error.self === window.Error if the extension hasn't modified the global, right?

[not at google - send to devlin, 2014/08/19 17:54:38]

There are 2 scenarios here:

(1) The error is the extension's fault, for example passing in an invalid schema
for a function. That throws an Error.
In this case, we would want the extension to be able to do something like:

window.Error.prototype.debugMe() = function() {
  console.log(whatever);
};
...
try {
  chrome.runtime.connect(42);
} catch(e) {
  e.debugMe();
}

some non-contrived version of that. Let them monkey patch however much they
want.


(2) The error is our fault, and we want to deal with that without worrying about
the extension mucking things up. For example, the following should *not* break
us:

window.Error = function(message) {
  console.log('constructed error', message);
};

but it probably would.


My suggestion for a solution here is rather than relying on people that write
these bindings to remember to use $Error for internal errors and Error for
external errors, just provide an assert() method which implicitly throws the
equivalent of $Error. assert() would be more convenient anyway.

Honestly we already provide logging.[D]CHECK. Maybe we should just forget about
this assert() thing and just make those easier to use (e.g. insert them in every
context rather than requiring we requireNative('logging') to access them).

[robwu, 2014/08/19 20:51:37]

I have reverted every use of "throw new $Error.self" (patch set 5). The only
thing that uses $Error is $Error.captureStackTrace in
uncaught_exception_handler.js.
It is safe to use this potentially unsafe constructor for throwing, because the
code at that point is already throwing an error. Extension code that catches
errors should use the new safeErrorToString method that I added for getting an
error message (see patch set 5).

-        throw new Error("Events that support rules require an event name.");
+        throw new $Error.self(

[not at google - send to devlin, 2014/08/19 16:45:55]

new Error()

+            "Events that support rules require an event name.");
       // Events without names cannot be managed by the browser by definition
       // (the browser has no way of identifying them).
       this.eventOptions.unmanaged = true;
     }
 
     // Track whether the event has been destroyed to help track down the cause
     // of http://crbug.com/258526.
     // This variable will eventually hold the stack trace of the destroy call.
     // TODO(kalman): Delete this and replace with more sound logic that catches
     // when events are used without being *attached*.
     this.destroyed = null;
 
     if (this.eventOptions.unmanaged)
       this.attachmentStrategy = new NullAttachmentStrategy(this);
     else if (this.eventOptions.supportsFilters)
       this.attachmentStrategy = new FilteredAttachmentStrategy(this);
     else
       this.attachmentStrategy = new UnfilteredAttachmentStrategy(this);
   };
 
   // callback is a function(args, dispatch). args are the args we receive from
   // dispatchEvent(), and dispatch is a function(args) that dispatches args to
   // its listeners.
   function registerArgumentMassager(name, callback) {
     if (eventArgumentMassagers[name])
-      throw new Error("Massager already registered for event: " + name);
+      throw new $Error.self("Massager already registered for event: " + name);
     eventArgumentMassagers[name] = callback;
   }
 
   // Dispatches a named event with the given argument array. The args array is
   // the list of arguments that will be sent to the event callback.
   function dispatchEvent(name, args, filteringInfo) {
     var listenerIDs = [];
 
     if (filteringInfo)
       listenerIDs = eventNatives.MatchAgainstEventFilter(name, filteringInfo);
@@ skipping 11 matching lines @@
 
     if (eventArgumentMassagers[name])
       eventArgumentMassagers[name](args, dispatchArgs);
     else
       dispatchArgs(args);
   }
 
   // Registers a callback to be called when this event is dispatched.
   EventImpl.prototype.addListener = function(cb, filters) {
     if (!this.eventOptions.supportsListeners)
-      throw new Error("This event does not support listeners.");
+      throw new $Error.self("This event does not support listeners.");

[not at google - send to devlin, 2014/08/19 16:45:54]

new Error()

     if (this.eventOptions.maxListeners &&
         this.getListenerCount_() >= this.eventOptions.maxListeners) {
-      throw new Error("Too many listeners for " + this.eventName);
+      throw new $Error.self("Too many listeners for " + this.eventName);

[not at google - send to devlin, 2014/08/19 16:45:55]

new Error()

     }
     if (filters) {
       if (!this.eventOptions.supportsFilters)
-        throw new Error("This event does not support filters.");
+        throw new $Error.self("This event does not support filters.");
       if (filters.url && !(filters.url instanceof Array))
-        throw new Error("filters.url should be an array.");
+        throw new $Error.self("filters.url should be an array.");
       if (filters.serviceType &&
           !(typeof filters.serviceType === 'string')) {
-        throw new Error("filters.serviceType should be a string.")
+        throw new $Error.self("filters.serviceType should be a string.");

[not at google - send to devlin, 2014/08/19 16:45:55]

new Error() for all of these

       }
     }
     var listener = {callback: cb, filters: filters};
     this.attach_(listener);
     $Array.push(this.listeners, listener);
   };
 
   EventImpl.prototype.attach_ = function(listener) {
     this.attachmentStrategy.onAddedListener(listener);
 
     if (this.listeners.length == 0) {
       allAttachedEvents[allAttachedEvents.length] = this;
       if (this.eventName) {
         if (attachedNamedEvents[this.eventName]) {
-          throw new Error("Event '" + this.eventName +
+          throw new $Error.self("Event '" + this.eventName +
                           "' is already attached.");
         }
         attachedNamedEvents[this.eventName] = this;
       }
     }
   };
 
   // Unregisters a callback.
   EventImpl.prototype.removeListener = function(cb) {
     if (!this.eventOptions.supportsListeners)
-      throw new Error("This event does not support listeners.");
+      throw new $Error.self("This event does not support listeners.");

[not at google - send to devlin, 2014/08/19 16:45:54]

new Error()

 
     var idx = this.findListener_(cb);
     if (idx == -1)
       return;
 
     var removedListener = $Array.splice(this.listeners, idx, 1)[0];
     this.attachmentStrategy.onRemovedListener(removedListener);
 
     if (this.listeners.length == 0) {
       var i = $Array.indexOf(allAttachedEvents, this);
       if (i >= 0)
         delete allAttachedEvents[i];
       if (this.eventName) {
         if (!attachedNamedEvents[this.eventName]) {
-          throw new Error(
+          throw new $Error.self(
               "Event '" + this.eventName + "' is not attached.");
         }
         delete attachedNamedEvents[this.eventName];
       }
     }
   };
 
   // Test if the given callback is registered for this event.
   EventImpl.prototype.hasListener = function(cb) {
     if (!this.eventOptions.supportsListeners)
-      throw new Error("This event does not support listeners.");
+      throw new $Error.self("This event does not support listeners.");

[not at google - send to devlin, 2014/08/19 16:45:55]

new Error()

     return this.findListener_(cb) > -1;
   };
 
   // Test if any callbacks are registered for this event.
   EventImpl.prototype.hasListeners = function() {
     return this.getListenerCount_() > 0;
   };
 
   // Returns the number of listeners on this event.
   EventImpl.prototype.getListenerCount_ = function() {
     if (!this.eventOptions.supportsListeners)
-      throw new Error("This event does not support listeners.");
+      throw new $Error.self("This event does not support listeners.");

[not at google - send to devlin, 2014/08/19 16:45:55]

new Error()

     return this.listeners.length;
   };
 
   // Returns the index of the given callback if registered, or -1 if not
   // found.
   EventImpl.prototype.findListener_ = function(cb) {
     for (var i = 0; i < this.listeners.length; i++) {
       if (this.listeners[i].callback == cb) {
         return i;
       }
     }
 
     return -1;
   };
 
   EventImpl.prototype.dispatch_ = function(args, listenerIDs) {
     if (this.destroyed) {
-      throw new Error(this.eventName + ' was already destroyed at: ' +
+      throw new $Error.self(this.eventName + ' was already destroyed at: ' +
                       this.destroyed);
     }
     if (!this.eventOptions.supportsListeners)
-      throw new Error("This event does not support listeners.");
+      throw new $Error.self("This event does not support listeners.");

[not at google - send to devlin, 2014/08/19 16:45:55]

new Error()

 
     if (this.argSchemas && logging.DCHECK_IS_ON()) {
       try {
         validate(args, this.argSchemas);
       } catch (e) {
         e.message += ' in ' + this.eventName;
         throw e;
       }
     }
 
@@ skipping 36 matching lines @@
   };
 
   EventImpl.prototype.destroy_ = function() {
     this.listeners.length = 0;
     this.detach_();
     this.destroyed = exceptionHandler.getStackTrace();
   };
 
   EventImpl.prototype.addRules = function(rules, opt_cb) {
     if (!this.eventOptions.supportsRules)
-      throw new Error("This event does not support rules.");
+      throw new $Error.self("This event does not support rules.");

[not at google - send to devlin, 2014/08/19 16:45:55]

new Error()

 
     // Takes a list of JSON datatype identifiers and returns a schema fragment
     // that verifies that a JSON object corresponds to an array of only these
     // data types.
     function buildArrayOfChoicesSchema(typesList) {
       return {
         'type': 'array',
         'items': {
           'choices': typesList.map(function(el) {return {'$ref': el};})
         }
@@ skipping 10 matching lines @@
     function validateRules(rules, conditions, actions) {
       var conditionsSchema = buildArrayOfChoicesSchema(conditions);
       var actionsSchema = buildArrayOfChoicesSchema(actions);
       $Array.forEach(rules, function(rule) {
         validate([rule.conditions], [conditionsSchema]);
         validate([rule.actions], [actionsSchema]);
       });
     };
 
     if (!this.eventOptions.conditions || !this.eventOptions.actions) {
-      throw new Error('Event ' + this.eventName + ' misses ' +
+      throw new $Error.self('Event ' + this.eventName + ' misses ' +
                       'conditions or actions in the API specification.');
     }
 
     validateRules(rules,
                   this.eventOptions.conditions,
                   this.eventOptions.actions);
 
     ensureRuleSchemasLoaded();
     // We remove the first parameter from the validation to give the user more
     // meaningful error messages.
     validate([this.webViewInstanceId, rules, opt_cb],
              $Array.splice(
                  $Array.slice(ruleFunctionSchemas.addRules.parameters), 1));
     sendRequest(
       "events.addRules",
       [this.eventName, this.webViewInstanceId, rules,  opt_cb],
       ruleFunctionSchemas.addRules.parameters);
   }
 
   EventImpl.prototype.removeRules = function(ruleIdentifiers, opt_cb) {
     if (!this.eventOptions.supportsRules)
-      throw new Error("This event does not support rules.");
+      throw new $Error.self("This event does not support rules.");

[not at google - send to devlin, 2014/08/19 16:45:55]

new Error()

     ensureRuleSchemasLoaded();
     // We remove the first parameter from the validation to give the user more
     // meaningful error messages.
     validate([this.webViewInstanceId, ruleIdentifiers, opt_cb],
              $Array.splice(
                  $Array.slice(ruleFunctionSchemas.removeRules.parameters), 1));
     sendRequest("events.removeRules",
                 [this.eventName,
                  this.webViewInstanceId,
                  ruleIdentifiers,
                  opt_cb],
                 ruleFunctionSchemas.removeRules.parameters);
   }
 
   EventImpl.prototype.getRules = function(ruleIdentifiers, cb) {
     if (!this.eventOptions.supportsRules)
-      throw new Error("This event does not support rules.");
+      throw new $Error.self("This event does not support rules.");

[not at google - send to devlin, 2014/08/19 16:45:55]

new Error()

     ensureRuleSchemasLoaded();
     // We remove the first parameter from the validation to give the user more
     // meaningful error messages.
     validate([this.webViewInstanceId, ruleIdentifiers, cb],
              $Array.splice(
                  $Array.slice(ruleFunctionSchemas.getRules.parameters), 1));
 
     sendRequest(
       "events.getRules",
       [this.eventName, this.webViewInstanceId, ruleIdentifiers, cb],
@@ skipping 19 matching lines @@
     'removeRules',
     'getRules'
   ] });
 
   // NOTE: Event is (lazily) exposed as chrome.Event from dispatcher.cc.
   exports.Event = Event;
 
   exports.dispatchEvent = dispatchEvent;
   exports.parseEventOptions = parseEventOptions;
   exports.registerArgumentMassager = registerArgumentMassager;