Bug 675125 - Utils.notify should pass observers the exception as subject. r=rnewman

services/sync/modules/policies.js

@@ -138,7 +138,7 @@ let SyncScheduler = {
         this.scheduleNextSync(sync_interval);
         break;
       case "weave:engine:sync:finish":
-        if (subject == "clients") {
+        if (data == "clients") {
           // Update the client mode because it might change what we sync.
           this.updateClientMode();
         }

services/sync/modules/util.js

@@ -142,31 +142,44 @@ let Utils = {
   },
 
   /**
-   * Wrap functions to notify when it starts and finishes executing or if it got
-   * an error. The message is a combination of a provided prefix and local name
-   * with the current state and the subject is the provided subject.
-   *
-   * @usage function MyObj() { this._notify = Utils.notify("prefix:"); }
-   *        MyObj.foo = function() { this._notify(name, subject, func)(); }
+   * Wrap functions to notify when it starts and finishes executing or if it
+   * threw an error.
+   * 
+   * The message is a combination of a provided prefix, the local name, and
+   * the event. Possible events are: "start", "finish", "error". The subject
+   * is the function's return value on "finish" or the caught exception on
+   * "error". The data argument is the predefined data value.
+   * 
+   * Example:
+   * 
+   * @usage function MyObj(name) {
+   *          this.name = name;
+   *          this._notify = Utils.notify("obj:");
+   *        }
+   *        MyObj.prototype = {
+   *          foo: function() this._notify("func", "data-arg", function () {
+   *            //...
+   *          }(),
+   *        };
    */
   notify: function Utils_notify(prefix) {
-    return function NotifyMaker(name, subject, func) {
+    return function NotifyMaker(name, data, func) {
       let thisArg = this;
-      let notify = function(state) {
+      let notify = function(state, subject) {
         let mesg = prefix + name + ":" + state;
         thisArg._log.trace("Event: " + mesg);
-        Observers.notify(mesg, subject);
+        Observers.notify(mesg, subject, data);
       };
 
       return function WrappedNotify() {
         try {
-          notify("start");
+          notify("start", null);
           let ret = func.call(thisArg);
-          notify("finish");
+          notify("finish", ret);
           return ret;
         }
         catch(ex) {
-          notify("error");
+          notify("error", ex);
           throw ex;
         }
       };

services/sync/tests/unit/test_engine.js

@@ -45,7 +45,7 @@ let engineObserver = {
   topics: [],
 
   observe: function(subject, topic, data) {
-    do_check_eq(subject, "steam");
+    do_check_eq(data, "steam");
     this.topics.push(topic);
   },
 

services/sync/tests/unit/test_utils_notify.js

@@ -46,14 +46,17 @@ function run_test() {
   do_check_eq(ret, 5);
   do_check_true(rightThis);
   do_check_true(didCall);
+
   do_check_eq(fs.state, 1);
-  do_check_eq(fs.subject, "baz");
+  do_check_eq(fs.subject, undefined);
   do_check_eq(fs.topic, "foo:bar:start");
-  do_check_eq(fs.data, undefined);
+  do_check_eq(fs.data, "baz");
+
   do_check_eq(ff.state, 2);
-  do_check_eq(ff.subject, "baz");
+  do_check_eq(ff.subject, 5);
   do_check_eq(ff.topic, "foo:bar:finish");
-  do_check_eq(ff.data, undefined);
+  do_check_eq(ff.data, "baz");
+
   do_check_eq(fe.state, undefined);
   do_check_eq(fe.subject, undefined);
   do_check_eq(fe.topic, undefined);
@@ -75,16 +78,19 @@ function run_test() {
   do_check_eq(ret, null);
   do_check_true(rightThis);
   do_check_true(didCall);
+
   do_check_eq(ts.state, 3);
-  do_check_eq(ts.subject, "one");
+  do_check_eq(ts.subject, undefined);
   do_check_eq(ts.topic, "foo:bad:start");
-  do_check_eq(ts.data, undefined);
+  do_check_eq(ts.data, "one");
+
   do_check_eq(tf.state, undefined);
   do_check_eq(tf.subject, undefined);
   do_check_eq(tf.topic, undefined);
   do_check_eq(tf.data, undefined);
+
   do_check_eq(te.state, 4);
-  do_check_eq(te.subject, "one");
+  do_check_eq(te.subject, 10);
   do_check_eq(te.topic, "foo:bad:error");
-  do_check_eq(te.data, undefined);
+  do_check_eq(te.data, "one");
 }