From ef46d1ea5804a550ff5f715a15a36243440feab4 Mon Sep 17 00:00:00 2001
From: Geoff Schmidt <geoff@geoffschmidt.com>
Date: Wed, 14 Mar 2012 20:20:19 -0700
Subject: [PATCH 01/10] eliminate App, proxy methods from Meteor

---
 packages/autopublish/autopublish.js        |  2 +-
 packages/livedata/client_convenience.js    | 33 ++++++++--------
 packages/livedata/livedata_connection.js   | 10 ++---
 packages/livedata/livedata_server.js       |  7 ----
 packages/livedata/livedata_test_service.js |  4 +-
 packages/livedata/livedata_tests.js        | 45 +++++++++++++---------
 packages/livedata/server_convenience.js    | 12 ++++--
 packages/mongo-livedata/collection.js      |  7 +++-
 packages/tinytest/tinytest_client.js       |  6 +--
 packages/tinytest/tinytest_server.js       |  4 +-
 10 files changed, 68 insertions(+), 62 deletions(-)

diff --git a/packages/autopublish/autopublish.js b/packages/autopublish/autopublish.js
index 4f5c1342c0..bedee57223 100644
--- a/packages/autopublish/autopublish.js
+++ b/packages/autopublish/autopublish.js
@@ -1 +1 @@
-App.autopublish();
\ No newline at end of file
+Meteor.default_server.autopublish();
diff --git a/packages/livedata/client_convenience.js b/packages/livedata/client_convenience.js
index e41cf43eda..207401c4d2 100644
--- a/packages/livedata/client_convenience.js
+++ b/packages/livedata/client_convenience.js
@@ -1,24 +1,21 @@
-// XXX this isn't going to work -- when connecting to a remote server,
-// the user isn't going to know to include /sockjs. need to add it in
-// stream_client..
-
-// Path matches sockjs 'prefix' in stream_server. We should revisit this
-// once we specify the 'on the wire' aspects of livedata more clearly.
-App = Meteor.connect('/sockjs', true /* restart_on_update */);
-
 _.extend(Meteor, {
-  status: function () {
-    return App.status();
-  },
+  // XXX this isn't going to work -- when connecting to a remote
+  // server, the user isn't going to know to include /sockjs. need to
+  // add it in stream_client..
 
-  reconnect: function () {
-    return App.reconnect();
-  },
-
-  subscribe: function (/* arguments */) {
-    return App.subscribe.apply(App, _.toArray(arguments));
-  },
+  // Path matches sockjs 'prefix' in stream_server. We should revisit
+  // this once we specify the 'on the wire' aspects of livedata more
+  // clearly.
+  default_connection: Meteor.connect('/sockjs', true /* restart_on_update */),
 
   refresh: function (notification) {
   }
 });
+
+// Proxy the public methods of Meteor.default_connection so they can
+// be called directly on Meteor.
+_.each(['subscribe', 'methods', 'call', 'apply', 'status', 'reconnect'],
+       function (name) {
+         Meteor[name] = _.bind(Meteor.default_connection[name],
+                               Meteor.default_connection);
+       });
diff --git a/packages/livedata/livedata_connection.js b/packages/livedata/livedata_connection.js
index 5f56c03d0a..7b797bad47 100644
--- a/packages/livedata/livedata_connection.js
+++ b/packages/livedata/livedata_connection.js
@@ -330,11 +330,11 @@ _.extend(Meteor._LivedataConnection.prototype, {
     return self.stream.reconnect();
   },
 
-  // called when we are up-to-date with the server. intended for use
-  // only in tests. currently, you are very limited in what you may do
-  // inside your callback -- in particular, don't do anything that
-  // could result in another call to onQuiesce, or results are
-  // undefined.
+  // PRIVATE: called when we are up-to-date with the server. intended
+  // for use only in tests. currently, you are very limited in what
+  // you may do inside your callback -- in particular, don't do
+  // anything that could result in another call to onQuiesce, or
+  // results are undefined.
   onQuiesce: function (f) {
     var self = this;
 
diff --git a/packages/livedata/livedata_server.js b/packages/livedata/livedata_server.js
index 799ce6bd06..b471edbd3c 100644
--- a/packages/livedata/livedata_server.js
+++ b/packages/livedata/livedata_server.js
@@ -910,12 +910,5 @@ _.extend(Meteor._LivedataServer.prototype, {
       self.on_autopublish.push(f);
     else
       f();
-  },
-
-  // called when we are up-to-date. intended for use only in tests.
-  onQuiesce: function (f) {
-    var self = this;
-    // the server is always up-to-date
-    f();
   }
 });
diff --git a/packages/livedata/livedata_test_service.js b/packages/livedata/livedata_test_service.js
index 01409b6d44..efcabba787 100644
--- a/packages/livedata/livedata_test_service.js
+++ b/packages/livedata/livedata_test_service.js
@@ -1,4 +1,4 @@
-App.methods({
+Meteor.methods({
   echo: function (/* arguments */) {
     return _.toArray(arguments);
   },
@@ -31,7 +31,7 @@ if (Meteor.is_server)
                                               world: world}});
   });
 
-App.methods({
+Meteor.methods({
   'ledger/transfer': function (world, from_name, to_name, amount, cheat) {
     var from = Ledger.findOne({name: from_name, world: world});
     var to = Ledger.findOne({name: to_name, world: world});
diff --git a/packages/livedata/livedata_tests.js b/packages/livedata/livedata_tests.js
index cdee6556a6..c53465ff87 100644
--- a/packages/livedata/livedata_tests.js
+++ b/packages/livedata/livedata_tests.js
@@ -115,18 +115,18 @@ Tinytest.add("livedata - methods with colliding names", function (test) {
   var x = LocalCollection.uuid();
   var m = {};
   m[x] = function () {};
-  App.methods(m);
+  Meteor.methods(m);
 
   test.throws(function () {
-    App.methods(m);
+    Meteor.methods(m);
   });
 });
 
 testAsyncMulti("livedata - basic method invocation", [
   function (test, expect) {
     try {
-      var ret = App.call("unknown method",
-                         expect(failure(test, 404, "Method not found")));
+      var ret = Meteor.call("unknown method",
+                            expect(failure(test, 404, "Method not found")));
     } catch (e) {
       // throws immediately on server, but still calls callback
       test.isTrue(Meteor.is_server);
@@ -139,31 +139,31 @@ testAsyncMulti("livedata - basic method invocation", [
   },
 
   function (test, expect) {
-    var ret = App.call("echo", expect(undefined, []));
+    var ret = Meteor.call("echo", expect(undefined, []));
     test.equal(ret, []);
   },
 
   function (test, expect) {
-    var ret = App.call("echo", 12, expect(undefined, [12]));
+    var ret = Meteor.call("echo", 12, expect(undefined, [12]));
     test.equal(ret, [12]);
   },
 
   function (test, expect) {
-    var ret = App.call("echo", 12, {x: 13}, expect(undefined, [12, {x: 13}]));
+    var ret = Meteor.call("echo", 12, {x: 13}, expect(undefined, [12, {x: 13}]));
     test.equal(ret, [12, {x: 13}]);
   },
 
   function (test, expect) {
     test.throws(function () {
-      var ret = App.call("exception", "both",
-                         expect(failure(test, 500, "Internal server error")));
+      var ret = Meteor.call("exception", "both",
+                            expect(failure(test, 500, "Internal server error")));
     });
   },
 
   function (test, expect) {
     try {
-      var ret = App.call("exception", "server",
-                         expect(failure(test, 500, "Internal server error")));
+      var ret = Meteor.call("exception", "server",
+                            expect(failure(test, 500, "Internal server error")));
     } catch (e) {
       test.isTrue(Meteor.is_server);
       return;
@@ -176,10 +176,10 @@ testAsyncMulti("livedata - basic method invocation", [
   function (test, expect) {
     if (Meteor.is_client) {
       test.throws(function () {
-        var ret = App.call("exception", "client", expect(undefined, undefined));
+        var ret = Meteor.call("exception", "client", expect(undefined, undefined));
       });
     } else {
-      var ret = App.call("exception", "client", expect(undefined, undefined));
+      var ret = Meteor.call("exception", "client", expect(undefined, undefined));
       test.equal(ret, undefined);
     }
   }
@@ -194,6 +194,13 @@ var checkBalances = function (test, a, b) {
   test.equal(bob.balance, b);
 }
 
+var onQuiesce = function (f) {
+  if (Meteor.is_server)
+    f();
+  else
+    Meteor.default_connection.onQuiesce(f);
+};
+
 // would be nice to have a database-aware test harness of some kind --
 // this is a big hack (and XXX pollutes the global test namespace)
 testAsyncMulti("livedata - compound methods", [
@@ -204,20 +211,20 @@ testAsyncMulti("livedata - compound methods", [
     Ledger.insert({name: "bob", balance: 50, world: test.runId()});
   },
   function (test, expect) {
-    App.call('ledger/transfer', test.runId(), "alice", "bob", 10,
-             expect(undefined, undefined));
+    Meteor.call('ledger/transfer', test.runId(), "alice", "bob", 10,
+                expect(undefined, undefined));
 
     checkBalances(test, 90, 60);
 
     var release = expect();
-    App.onQuiesce(function () {
+    onQuiesce(function () {
       checkBalances(test, 90, 60);
       Meteor.defer(release);
     });
   },
   function (test, expect) {
-    App.call('ledger/transfer', test.runId(), "alice", "bob", 100, true,
-             expect(failure(test, 409)));
+    Meteor.call('ledger/transfer', test.runId(), "alice", "bob", 100, true,
+                expect(failure(test, 409)));
 
     if (Meteor.is_client)
       // client can fool itself by cheating, but only until the sync
@@ -227,7 +234,7 @@ testAsyncMulti("livedata - compound methods", [
       checkBalances(test, 90, 60);
 
     var release = expect();
-    App.onQuiesce(function () {
+    onQuiesce(function () {
       checkBalances(test, 90, 60);
       Meteor.defer(release);
     });
diff --git a/packages/livedata/server_convenience.js b/packages/livedata/server_convenience.js
index c41746f316..a3470b39dc 100644
--- a/packages/livedata/server_convenience.js
+++ b/packages/livedata/server_convenience.js
@@ -1,7 +1,5 @@
-App = new Meteor._LivedataServer;
-
 _.extend(Meteor, {
-  publish: _.bind(App.publish, App),
+  default_server: new Meteor._LivedataServer,
 
   refresh: function (notification) {
     var fence = Meteor._CurrentWriteFence.get();
@@ -16,3 +14,11 @@ _.extend(Meteor, {
     });
   }
 });
+
+// Proxy the public methods of Meteor.default_server so they can
+// be called directly on Meteor.
+_.each(['publish', 'methods', 'call', 'apply'],
+       function (name) {
+         Meteor[name] = _.bind(Meteor.default_server[name],
+                               Meteor.default_server);
+       });
diff --git a/packages/mongo-livedata/collection.js b/packages/mongo-livedata/collection.js
index fbffd8b94a..f4e5605dcc 100644
--- a/packages/mongo-livedata/collection.js
+++ b/packages/mongo-livedata/collection.js
@@ -10,10 +10,13 @@ Meteor.Collection = function (name, manager, driver) {
   }
 
   // note: nameless collections never have a manager
-  manager = name && (manager || App);
+  manager = name && (manager ||
+                     (Meteor.is_client ?
+                      Meteor.default_connection : Meteor.default_server));
 
   if (!driver) {
-    if (name && manager === App && Meteor._RemoteCollectionDriver)
+    if (name && manager === Meteor.default_server &&
+        Meteor._RemoteCollectionDriver)
       driver = Meteor._RemoteCollectionDriver;
     else
       driver = Meteor._LocalCollectionDriver;
diff --git a/packages/tinytest/tinytest_client.js b/packages/tinytest/tinytest_client.js
index b8308859d3..d349c7e627 100644
--- a/packages/tinytest/tinytest_client.js
+++ b/packages/tinytest/tinytest_client.js
@@ -19,13 +19,13 @@ Meteor._runTestsEverywhere = function (onReport, onComplete) {
     maybeDone();
   });
 
-  App.call('tinytest/run', run_id, function (error, result) {
+  Meteor.call('tinytest/run', run_id, function (error, result) {
     if (error)
       // XXX better report error
       throw new Error("Test server returned an error");
   });
 
-  App.onQuiesce(function () {
+  Meteor.default_connection.onQuiesce(function () {
     // XXX use _.defer to avoid calling into minimongo
     // reentrantly. we need to handle this better..
     // (XXX code got refactored -- still necessary?)
@@ -46,7 +46,7 @@ Meteor._runTestsEverywhere = function (onReport, onComplete) {
     });
   });
 
-  var sub_handle = App.subscribe('tinytest/results', run_id);
+  var sub_handle = Meteor.subscribe('tinytest/results', run_id);
   var query_handle = Meteor._ServerTestResults.find().observe({
     added: function (doc) {
       _.each(doc.report.events || [], function (event) {
diff --git a/packages/tinytest/tinytest_server.js b/packages/tinytest/tinytest_server.js
index 331f5f0d5e..d91a7cac23 100644
--- a/packages/tinytest/tinytest_server.js
+++ b/packages/tinytest/tinytest_server.js
@@ -2,13 +2,13 @@ Meteor.startup(function () {
   Meteor._ServerTestResults.remove();
 });
 
-App.publish('tinytest/results', function (run_id) {
+Meteor.publish('tinytest/results', function (run_id) {
   return Meteor._ServerTestResults.find({run_id: run_id},
                                         {key: {collection: 'tinytest_results',
                                                run_id: run_id}});
 });
 
-App.methods({
+Meteor.methods({
   'tinytest/run': function (run_id) {
     var request = this;
     request.beginAsync();

From 784529a9af86b32045d4d17c575ac073323a064f Mon Sep 17 00:00:00 2001
From: Geoff Schmidt <geoff@geoffschmidt.com>
Date: Thu, 15 Mar 2012 02:34:21 -0700
Subject: [PATCH 02/10] tighten up exception reporting in async tests

---
 packages/livedata/livedata_tests.js |  2 +-
 packages/tinytest/tinytest.js       | 35 +++++++++++++++++++++++------
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/packages/livedata/livedata_tests.js b/packages/livedata/livedata_tests.js
index c53465ff87..cc68e0cd55 100644
--- a/packages/livedata/livedata_tests.js
+++ b/packages/livedata/livedata_tests.js
@@ -84,7 +84,7 @@ var testAsyncMulti = function (name, funcs) {
           em.cancel();
           test.exception(exception);
           Meteor.clearTimeout(timer);
-          onComplete();
+          // Because we called test.exception, we're not to call onComplete.
           return;
         }
         em.done();
diff --git a/packages/tinytest/tinytest.js b/packages/tinytest/tinytest.js
index 606e2f1822..fdaf0d130e 100644
--- a/packages/tinytest/tinytest.js
+++ b/packages/tinytest/tinytest.js
@@ -182,18 +182,39 @@ _.extend(TestCase.prototype, {
   // test raised (or voluntarily reported) an exception.
   run: function (onEvent, onComplete, onException, stop_at_offset) {
     var self = this;
-    var results = new TestCaseResults(self, onEvent, onException,
-                                      stop_at_offset);
+
+    var completed = false;
+    var markComplete = function () {
+      if (completed) {
+        Meteor._debug("*** Test error -- test '" + self.name +
+                      "' returned multiple times.");
+        return false;
+      }
+      completed = true;
+      return true;
+    }
+
+    var results = new TestCaseResults(self, onEvent,
+                                      function (e) {
+                                        if (markComplete())
+                                          onException(e);
+                                      }, stop_at_offset);
+
     Meteor.defer(function () {
       try {
-        if (self.async)
-          self.func(results, onComplete);
-        else {
+        if (self.async) {
+          self.func(results, function () {
+            if (markComplete())
+              onComplete()
+          });
+        } else {
           self.func(results);
-          onComplete();
+          if (markComplete())
+            onComplete();
         }
       } catch (e) {
-        onException(e);
+        if (markComplete())
+          onException(e);
       }
     });
   }

From c9b14233bbab7694897afd43e53654e9983724aa Mon Sep 17 00:00:00 2001
From: Geoff Schmidt <geoff@geoffschmidt.com>
Date: Thu, 15 Mar 2012 02:49:47 -0700
Subject: [PATCH 03/10] support error() on sync methods on server (temporary)

---
 packages/livedata/livedata_server.js | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/packages/livedata/livedata_server.js b/packages/livedata/livedata_server.js
index b471edbd3c..bcd6f9e7d4 100644
--- a/packages/livedata/livedata_server.js
+++ b/packages/livedata/livedata_server.js
@@ -11,6 +11,7 @@ Meteor._ServerMethodInvocation = function (name, handler) {
   self._handler = handler;
   self._async = false;
   self._responded = false;
+  self._response_was_error = null;
   self._autoresponded = false;
   self._threw = false;
   self._callback = null;
@@ -72,6 +73,10 @@ _.extend(Meteor._ServerMethodInvocation.prototype, {
       // code is responding more than once.
       if (from === "throw")
         return;
+      if (from === "sync" && self._response_was_error)
+        // it has to be OK to call this.error from a sync method,
+        // because how else would you signal an error?
+        return;
       if (self._autoresponded)
         throw new Error(
           "The method has already returned, so it is too late to call " +
@@ -83,6 +88,7 @@ _.extend(Meteor._ServerMethodInvocation.prototype, {
           "respond() or error() may only be called once per request");
     }
     self._responded = true;
+    self._response_was_error = !!error;
     self._autoresponded = (from === "sync");
 
     // if we haven't yet yielded to the next method in the queue, do

From 7be206b11a7694d9770dd58ba0198831af90552e Mon Sep 17 00:00:00 2001
From: Geoff Schmidt <geoff@geoffschmidt.com>
Date: Thu, 15 Mar 2012 00:54:43 -0700
Subject: [PATCH 04/10] database writes can report errors (first pass)

---
 packages/mongo-livedata/collection.js   | 121 ++++++++++-----
 packages/mongo-livedata/mongo_driver.js | 191 +++++++++++++++++-------
 2 files changed, 216 insertions(+), 96 deletions(-)

diff --git a/packages/mongo-livedata/collection.js b/packages/mongo-livedata/collection.js
index f4e5605dcc..583accff89 100644
--- a/packages/mongo-livedata/collection.js
+++ b/packages/mongo-livedata/collection.js
@@ -88,17 +88,29 @@ Meteor.Collection = function (name, manager, driver) {
     self._prefix = '/' + name + '/';
     m[self._prefix + 'insert'] = function (/* selector, options */) {
       self._maybe_snapshot();
-      return self._collection.insert.apply(self._collection, _.toArray(arguments));
+      try {
+        self._collection.insert.apply(self._collection, _.toArray(arguments));
+      } catch (e) {
+        this.error({error: 500, reason: "Database write failed"});
+      }
     };
 
     m[self._prefix + 'update'] = function (/* selector, mutator, options */) {
       self._maybe_snapshot();
-      return self._collection.update.apply(self._collection, _.toArray(arguments));
+      try {
+        self._collection.update.apply(self._collection, _.toArray(arguments));
+      } catch (e) {
+        this.error({error: 500, reason: "Database write failed"});
+      }
     };
 
     m[self._prefix + 'remove'] = function (/* selector */) {
       self._maybe_snapshot();
-      return self._collection.remove.apply(self._collection, _.toArray(arguments));
+      try {
+        self._collection.remove.apply(self._collection, _.toArray(arguments));
+      } catch (e) {
+        this.error({error: 500, reason: "Database write failed"});
+      }
     };
 
     manager.methods(m);
@@ -132,43 +144,70 @@ _.extend(Meteor.Collection.prototype, {
       self._collection.snapshot();
       self._was_snapshot = true;
     }
-  },
-
-  // XXX provide a way for the caller to find out about errors from
-  // the server? probably the answer is: detect a function at the end
-  // of the arguments, use as a callback ... same semantics as methods
-  // usually have?
-
-  insert: function (doc) {
-    var self = this;
-
-    // shallow-copy the document and generate an ID
-    doc = _.extend({}, doc);
-    doc._id = Meteor.uuid();
-
-    if (self._manager)
-      self._manager.call(self._prefix + 'insert', doc);
-    else
-      self._collection.insert(doc);
-
-    return doc;
-  },
-
-  update: function (/* arguments */) {
-    var self = this;
-
-    if (self._manager)
-      self._manager.apply(self._prefix + 'update', _.toArray(arguments));
-    else
-      self._collection.update.apply(self._collection, _.toArray(arguments));
-  },
-
-  remove: function (/* arguments */) {
-    var self = this;
-
-    if (self._manager)
-      self._manager.apply(self._prefix + 'remove', _.toArray(arguments));
-    else
-      self._collection.remove.apply(self._collection, _.toArray(arguments));
   }
 });
+
+// 'insert' returns a copy of the inserted document with the _id
+// added. the others return nothing. all of them may throw exceptions
+// (see below.)
+//
+// all of them can take a function as the last argument. if provided,
+// it will be called when the actual result of the operation is known
+// from the database. it will be called with no arguments if the
+// operation succeeded, or with one argument, the DDP error, if it
+// failed. if no callback is provided, then if an error happens, it
+// will be logged with Meteor._debug.
+//
+// if the operation fails before returning (eg, if on the server the
+// database returns failure), then an exception is raised AND the
+// callback (if provided) is called.
+//
+// database drivers SHOULD default to doing the operations
+// synchronously, so that they don't return until the database has
+// received them, but in the future we MAY provide a flag to turn this
+// off.
+_.each(["insert", "update", "remove"], function (name) {
+  Meteor.Collection.prototype[name] = function (/* arguments */) {
+    var self = this;
+    var args = _.toArray(arguments);
+
+    if (args.length && args[args.length - 1] instanceof Function)
+      var callback = args.pop();
+
+    if (name === "insert") {
+      if (!args.length)
+        throw new Error("insert requires an argument");
+      // shallow-copy the document and generate an ID
+      args[0] = _.extend({}, args[0]);
+      if ('_id' in args[0])
+        throw new Error("Do not pass an _id to insert. Meteor will generate the _id for you.");
+      args[0]._id = Meteor.uuid();
+      var ret = args[0];
+    }
+
+    if (self._manager) {
+      // NB: on failure, allow exception to propagate
+      self._manager.apply(self._prefix + name, args, function (err) {
+        if (err) {
+          if (callback)
+            callback(err);
+          else
+            Meteor._debug(name + " failed: " + err.error + " -- " + err.reason);
+        }
+      });
+    }
+    else {
+      try {
+        self._collection[name].apply(self._collection, args);
+      } catch (e) {
+        if (callback)
+          callback({error: 500, reason: "Local database threw exception",
+                    detail: e.stack});
+        throw e;
+      }
+      callback && callback();
+    }
+
+    return ret;
+  };
+});
diff --git a/packages/mongo-livedata/mongo_driver.js b/packages/mongo-livedata/mongo_driver.js
index 59bbe0b71d..f38f2fc73e 100644
--- a/packages/mongo-livedata/mongo_driver.js
+++ b/packages/mongo-livedata/mongo_driver.js
@@ -74,6 +74,13 @@ _Mongo.prototype._maybeBeginWrite = function () {
 
 //////////// Public API //////////
 
+// The write methods block until the database has confirmed the write
+// (it may not be replicated or stable on disk, but one server has
+// confirmed it.) (In the future we might have an option to turn this
+// off, ie, to enqueue the request on the wire and return
+// immediately.)  They return nothing on success, and raise an
+// exception on failure.
+//
 // After making a write (with insert, update, remove), observers are
 // notified asynchronously. If you want to receive a callback once all
 // of the observer notifications have landed for your write, do the
@@ -88,72 +95,113 @@ _Mongo.prototype.insert = function (collection_name, document) {
   var self = this;
   var write = self._maybeBeginWrite();
 
-  var future = new Future;
-  self._withCollection(collection_name, function(err, collection) {
-    // XXX err handling
-    collection.insert(document, {/* safe: true */}, function(err) {
-      // XXX err handling
-    });
-
+  var finish = Meteor.bindEnvironment(function () {
     Meteor.refresh({collection: collection_name});
     write.committed();
-
-    future.ret();
+  }, function (e) {
+    Meteor._debug("Exception while completing insert: " + e.stack);
   });
 
-  return future.wait();
+  var future = new Future;
+  self._withCollection(collection_name, function (err, collection) {
+    if (err) {
+      future.ret(err);
+      return;
+    }
+
+    collection.insert(document, {safe: true}, function (err) {
+      if (err) {
+        future.ret(err);
+        return;
+      }
+
+      finish();
+      future.ret();
+    });
+  });
+
+  var err = future.wait();
+  if (err)
+    throw err;
 };
 
 _Mongo.prototype.remove = function (collection_name, selector) {
   var self = this;
   var write = self._maybeBeginWrite();
 
+  var finish = Meteor.bindEnvironment(function () {
+    Meteor.refresh({collection: collection_name});
+    write.committed();
+  }, function (e) {
+    Meteor._debug("Exception while completing remove: " + e.stack);
+  });
+
   // XXX does not allow options. matches the client.
   selector = _Mongo._rewriteSelector(selector);
 
   var future = new Future;
-  self._withCollection(collection_name, function(err, collection) {
-    // XXX err handling
-    collection.remove(selector, {/* safe: true */}, function(err) {
-      // XXX err handling
+  self._withCollection(collection_name, function (err, collection) {
+    if (err) {
+      future.ret(err);
+      return;
+    }
+
+    collection.remove(selector, {/* XXXsafe: true*/}, function (err) {
+      if (err) {
+        future.ret(err);
+        return;
+      }
+
+      finish();
+      future.ret();
     });
-
-    Meteor.refresh({collection: collection_name});
-    write.committed();
-
-    future.ret();
   });
 
-  return future.wait();
+  var err = future.wait();
+  if (err)
+    throw err;
 };
 
 _Mongo.prototype.update = function (collection_name, selector, mod, options) {
   var self = this;
   var write = self._maybeBeginWrite();
 
+  var finish = Meteor.bindEnvironment(function () {
+    Meteor.refresh({collection: collection_name});
+    write.committed();
+  }, function (e) {
+    Meteor._debug("Exception while completing update: " + e.stack);
+  });
+
   selector = _Mongo._rewriteSelector(selector);
   if (!options) options = {};
 
   var future = new Future;
-  self._withCollection(collection_name, function(err, collection) {
-    // XXX err handling
+  self._withCollection(collection_name, function (err, collection) {
+    if (err) {
+      future.ret(err);
+      return;
+    }
 
-    var opts = {/* safe: true */};
+    var opts = {safe: true};
     // explictly enumerate options that minimongo supports
     if (options.upsert) opts.upsert = true;
     if (options.multi) opts.multi = true;
 
-    collection.update(selector, mod, opts, function(err) {
-      // XXX err handling
+    collection.update(selector, mod, opts, function (err) {
+      if (err) {
+        future.ret(err);
+        return;
+      }
+
+      finish();
+      future.ret();
     });
-
-    Meteor.refresh({collection: collection_name});
-    write.committed();
-
-    future.ret();
   });
 
-  return future.wait();
+  var err = future.wait();
+  if (err)
+    throw err;
 };
 
 _Mongo.prototype.find = function (collection_name, selector, options) {
@@ -162,7 +210,7 @@ _Mongo.prototype.find = function (collection_name, selector, options) {
   if (arguments.length === 1)
     selector = {};
 
-  return new _Mongo.Cursor(self, collection_name, selector, options);
+  return _Mongo._makeCursor(self, collection_name, selector, options);
 };
 
 _Mongo.prototype.findOne = function (collection_name, selector, options) {
@@ -171,28 +219,52 @@ _Mongo.prototype.findOne = function (collection_name, selector, options) {
   if (arguments.length === 1)
     selector = {};
 
-  return this.find(collection_name, selector, options).fetch()[0];
+  return self.find(collection_name, selector, options).fetch()[0];
 };
 
 // Cursors
 
-_Mongo.Cursor = function (mongo, collection_name, selector, options) {
-  var self = this;
-
-  self.mongo = mongo;
-  self.collection_name = collection_name;
-  self.selector = _Mongo._rewriteSelector(selector);
-  self.options = options || {};
-
+// Returns a _Mongo.Cursor, or throws an exception on
+// failure. Creating a cursor involves a database query, and we block
+// until it returns.
+_Mongo._makeCursor = function (mongo, collection_name, selector, options) {
   var future = new Future;
 
-  self.mongo._withCollection(collection_name, function(err, collection) {
-    // XXX err handling
-    var cursor = collection.find(self.selector, self.options.fields, self.options.skip, self.options.limit, self.options.sort);
-    future.ret(cursor);
+  options = options || {};
+  selector = _Mongo._rewriteSelector(selector);
+
+  mongo._withCollection(collection_name, function (err, collection) {
+    if (err) {
+      future.ret([false, err]);
+      return
+    }
+
+    var cursor = collection.find(selector, options.fields,
+                                 options.skip, options.limit, options.sort);
+    future.ret([true, cursor]);
   });
 
-  this.cursor = future.wait();
+  var result = future.wait();
+  if (!(result[0]))
+    throw result[1];
+
+  return new _Mongo.Cursor(mongo, collection_name, selector, options,
+                           result[1]);
+};
+
+// Do not call directly. Use _Mongo._makeCursor instead.
+_Mongo.Cursor = function (mongo, collection_name, selector, options, cursor) {
+  var self = this;
+
+  if (!cursor)
+    throw new Error("Cursor required");
+
+  // NB: 'options' and 'selector' have already been preprocessed by _makeCursor
+  self.mongo = mongo;
+  self.collection_name = collection_name;
+  self.selector = selector;
+  self.options = options;
+  self.cursor = cursor;
 };
 
 _Mongo.Cursor.prototype.forEach = function (callback) {
@@ -205,7 +277,10 @@ _Mongo.Cursor.prototype.forEach = function (callback) {
     else
       callback(doc);
   });
-  return future.wait();
+
+  var err = future.wait();
+  if (err)
+    throw err;
 };
 
 _Mongo.Cursor.prototype.map = function (callback) {
@@ -229,10 +304,13 @@ _Mongo.Cursor.prototype.fetch = function () {
   var future = new Future;
 
   self.cursor.toArray(function (err, res) {
-    future.ret(err || res);
+    future.ret([err, res]);
   });
 
-  return future.wait();
+  var result = future.wait();
+  if (result[0])
+    throw result[0];
+  return result[1];
 };
 
 _Mongo.Cursor.prototype.count = function () {
@@ -240,10 +318,13 @@ _Mongo.Cursor.prototype.count = function () {
   var future = new Future;
 
   self.cursor.count(function (err, res) {
-    future.ret(err || res);
+    future.ret([err, res]);
   });
 
-  return future.wait();
+  var err = future.wait();
+  if (result[0])
+    throw result[0];
+  return result[1];
 };
 
 // options to contain:
@@ -265,10 +346,10 @@ _Mongo.LiveResultsSet = function (cursor, options) {
 
   // copy my cursor, so that the observe can run independently from
   // some other use of the cursor.
-  self.cursor = new _Mongo.Cursor(cursor.mongo,
-                                  cursor.collection_name,
-                                  cursor.selector,
-                                  cursor.options);
+  self.cursor = _Mongo._makeCursor(cursor.mongo,
+                                   cursor.collection_name,
+                                   cursor.selector,
+                                   cursor.options);
 
   // expose collection name
   self.collection_name = cursor.collection_name;

From 6de5ab62ce82dd0fb967d8d4dc9d1a44ceefd3ec Mon Sep 17 00:00:00 2001
From: Geoff Schmidt <geoff@geoffschmidt.com>
Date: Thu, 15 Mar 2012 20:28:32 -0700
Subject: [PATCH 05/10] Simplify method API

---
 packages/livedata/livedata_common.js       |  39 +++
 packages/livedata/livedata_connection.js   | 176 +++++++-----
 packages/livedata/livedata_server.js       | 305 +++++++--------------
 packages/livedata/livedata_test_service.js |  29 +-
 packages/livedata/livedata_tests.js        | 210 +++++++++-----
 packages/mongo-livedata/collection.js      |  89 +++---
 packages/tinytest/tinytest_server.js       |  11 +-
 7 files changed, 451 insertions(+), 408 deletions(-)

diff --git a/packages/livedata/livedata_common.js b/packages/livedata/livedata_common.js
index d774f8f5d7..c53487aaae 100644
--- a/packages/livedata/livedata_common.js
+++ b/packages/livedata/livedata_common.js
@@ -1,2 +1,41 @@
 // XXX namespacing
+
+Meteor._MethodInvocation = function (is_simulation, unblock) {
+  var self = this;
+
+  // true if we're running not the actual method, but a stub (that is,
+  // if we're on the client and presently running a simulation of a
+  // server-side method for latency compensation purposes). never true
+  // except in a client such as a browser, since there's no point in
+  // running stubs unless you have a zero-latency connection to the
+  // user.
+  this.is_simulation = is_simulation;
+
+  // call this function to allow other method invocations (from the
+  // same client) to continue running without waiting for this one to
+  // complete.
+  this.unblock = unblock || function () {};
+};
+
 Meteor._CurrentInvocation = new Meteor.EnvironmentVariable;
+
+Meteor.Error = function (error, reason, details) {
+  var self = this;
+
+  // Currently, a numeric code, likely similar to a HTTP code (eg,
+  // 404, 500). That is likely to change though.
+  self.error = error;
+
+  // Optional: A short human-readable summary of the error. Not
+  // intended to be shown to end users, just developers. ("Not Found",
+  // "Internal Server Error")
+  self.reason = reason;
+
+  // Optional: Additional information about the error, say for
+  // debugging. It might be a (textual) stack trace if the server is
+  // willing to provide one. The corresponding thing in HTTP would be
+  // the body of a 404 or 500 response. (The difference is that we
+  // never expect this to be shown to end users, only developers, so
+  // it doesn't need to be pretty.)
+  self.details = details;
+};
diff --git a/packages/livedata/livedata_connection.js b/packages/livedata/livedata_connection.js
index 7b797bad47..e45850d1eb 100644
--- a/packages/livedata/livedata_connection.js
+++ b/packages/livedata/livedata_connection.js
@@ -1,66 +1,7 @@
-Meteor._ClientMethodInvocation = function (name, handler) {
-  var self = this;
-
-  // XXX need: user, setRestartHook, setUser (simulated??)
-
-  self._enclosing = null;
-  self.isSimulation = null;
-  self._name = name;
-  self._handler = handler;
-  self._callback = null;
-  self._id = null;
-};
-
-_.extend(Meteor._ClientMethodInvocation.prototype, {
-  beginAsync: function () {
-    // XXX need a much better error message!
-    // duplicated in livedata_server
-    throw new Error("Simulated methods may not be asynchronous");
-  },
-
-  _run: function (args, callback, enqueue) {
-    var self = this;
-    self._enclosing = Meteor._CurrentInvocation.get();
-    self._callback = callback;
-    self.isSimulation = true; // NB: refers to child invocations, not to us
-
-    // run locally (if we have a stub for it)
-    if (self._handler) {
-      try {
-        var ret = Meteor._CurrentInvocation.withValue(self, function () {
-          return self._handler.apply(self, args);
-        });
-      } catch (e) {
-        var stub_exception = e;
-      }
-    }
-
-    if (self._enclosing && self._enclosing.isSimulation) {
-      // In simulation mode, never do an RPC. Use the result of
-      // running the stub instead.
-      if (self._callback) {
-        if (stub_exception)
-          self._callback({error: 500, reason: "Stub threw exception"});
-        else
-          self._callback(ret);
-      }
-    } else {
-      // This invocation is real, not a simulation. Do the RPC.
-
-      // Note that it is important that the function totally complete,
-      // locally, before the message is sent to the server. (Or at
-      // least, we need to guarantee that the snapshot is not restored
-      // until the local copy of the function has stopped doing writes.)
-
-      enqueue({msg: 'method', method: self._name, params: args},
-              self._callback);
-    }
-
-    if (stub_exception)
-      throw stub_exception;
-    return ret;
-  }
-});
+if (Meteor.is_server) {
+  // XXX namespacing
+  var Future = __meteor_bootstrap__.require('fibers/future');
+}
 
 // list of subscription tokens outstanding during a
 // captureDependencies run. only set when we're doing a run. The fact
@@ -294,7 +235,6 @@ _.extend(Meteor._LivedataConnection.prototype, {
   apply: function (name, args, callback) {
     var self = this;
     var enclosing = Meteor._CurrentInvocation.get();
-    var handler = self.method_handlers[name];
 
     if (callback)
       // XXX would it be better form to do the binding in stream.on,
@@ -304,20 +244,101 @@ _.extend(Meteor._LivedataConnection.prototype, {
         Meteor._debug("Exception while delivering result of invoking '" +
                       name + "'", e.stack);
       });
-    else
-      callback = function () {};
 
-    var enqueue = function (msg, callback) {
-      msg.id = '' + (self.next_method_id++);
-      self.outstanding_methods.push({
-        msg: msg, callback: callback});
-      self.unsatisfied_methods[msg.id] = true;
-      self.stream.send(JSON.stringify(msg));
+    var is_simulation = enclosing && enclosing.is_simulation;
+    if (Meteor.is_client) {
+      // If on a client, run the stub, if we have one. The stub is
+      // supposed to make some temporary writes to the database to
+      // give the user a smooth experience until the actual result of
+      // executing the method comes back from the server (whereupon
+      // the temporary writes to the database will be reversed during
+      // the beginUpdate/endUpdate process.)
+      //
+      // Normally, we ignore the return value of the stub (even if it
+      // is an exception), in favor of the real return value from the
+      // server. The exception is if the *caller* is a stub. In that
+      // case, we're not going to do a RPC, so we use the return value
+      // of the stub as our return value.
+      var stub = self.method_handlers[name];
+      if (stub) {
+        var invocation = new Meteor._MethodInvocation(true /* is_simulation */);
+        try {
+          var ret = Meteor._CurrentInvocation.withValue(invocation,function () {
+            return stub.apply(self, args);
+          });
+        }
+        catch (e) {
+          var exception = e;
+        }
+      }
+
+      // If we're in a simulation, stop and return the result we have,
+      // rather than going on to do an RPC. This can only happen on
+      // the client (since we only bother with stubs and simulations
+      // on the client.) If there was not stub, we'll end up returning
+      // undefined.
+      if (is_simulation) {
+        if (callback) {
+          callback(exception, ret);
+          return;
+        }
+        if (exception)
+          throw exception;
+        return ret;
+      }
+
+      // If an exception occurred in a stub, and we're ignoring it
+      // because we're doing an RPC and want to use what the server
+      // returns instead, log it so the developer knows.
+      //
+      // Tests can set the 'expected' flag on an exception so it won't
+      // go to log.
+      if (exception && !exception.expected)
+        Meteor._debug("Exception while simulating the effect of invoking '" +
+                      name + "'", exception.stack);
+    }
+
+    // At this point we're definitely doing an RPC, and we're going to
+    // return the value of the RPC to the caller.
+
+    // If the caller didn't give a callback, decide what to do.
+    if (!callback) {
+      if (Meteor.is_client)
+        // On the client, we don't have fibers, so we can't block. The
+        // only thing we can do is to return undefined and discard the
+        // result of the RPC.
+        callback = function () {};
+      else {
+        // On the server, make the function synchronous.
+        var future = new Future;
+        callback = function (err, result) {
+          future['return']([err, result]);
+        };
+      }
+    }
+
+    // Send the RPC. Note that on the client, it is important that the
+    // stub have finished before we send the RPC (or at least we need
+    // to guaranteed that the snapshot is not restored until the stub
+    // has stopped doing writes.)
+    var msg = {
+      msg: 'method',
+      method: name,
+      params: args,
+      id: '' + (self.next_method_id++)
     };
+    self.outstanding_methods.push({msg: msg, callback: callback});
+    self.unsatisfied_methods[msg.id] = true;
+    self.stream.send(JSON.stringify(msg));
 
-    var invocation = new Meteor._ClientMethodInvocation(name, handler, self);
-    // if _run throws an exception, allow it to propagate
-    return invocation._run(args, callback, enqueue);
+    // If we're using the default callback on the server,
+    // synchronously return the result from the remote host.
+    if (future) {
+      var outcome = future.wait();
+      if (outcome[0])
+        throw outcome[0];
+      return outcome[1];
+    }
   },
 
   status: function () {
@@ -465,8 +486,11 @@ _.extend(Meteor._LivedataConnection.prototype, {
       // callback will have already been bindEnvironment'd by apply(),
       // so no need to catch exceptions
       if ('error' in msg)
-        m.callback(msg.error);
+        m.callback(new Meteor.Error(msg.error.error, msg.error.reason,
+                                    msg.error.details));
       else
+        // msg.result may be undefined if the method didn't return a
+        // value
         m.callback(undefined, msg.result);
     }
 
diff --git a/packages/livedata/livedata_server.js b/packages/livedata/livedata_server.js
index bcd6f9e7d4..d908c8e84c 100644
--- a/packages/livedata/livedata_server.js
+++ b/packages/livedata/livedata_server.js
@@ -1,145 +1,3 @@
-/******************************************************************************/
-/* ServerMethodInvocation                                                     */
-/******************************************************************************/
-
-Meteor._ServerMethodInvocation = function (name, handler) {
-  var self = this;
-
-  self._enclosing = null;
-  self.isSimulation = null;
-  self._name = name;
-  self._handler = handler;
-  self._async = false;
-  self._responded = false;
-  self._response_was_error = null;
-  self._autoresponded = false;
-  self._threw = false;
-  self._callback = null;
-  self._next = null;
-  self._calledNext = false;
-};
-
-_.extend(Meteor._ServerMethodInvocation.prototype, {
-  beginAsync: function (okToContinue) {
-    var self = this;
-
-    if (okToContinue === undefined)
-      okToContinue = true;
-
-    if (self.isSimulation)
-      // XXX need a much better error message!
-      // duplicated in livedata_connection
-      throw new Error("Simulated methods may not be asynchronous");
-    else if (self._responded)
-      throw new Error("The method has already returned, so it is too late " +
-                      "to mark it as asynchronous");
-    else {
-      self._async = true;
-      if (okToContinue && !self._calledNext) {
-        self._calledNext = true;
-        self._next && self._next();
-      }
-    }
-  },
-
-  respond: function (ret) {
-    this._sendResponse(undefined, ret, "async");
-  },
-
-  error: function (code, message) {
-    this._sendResponse({error: code, reason: message}, undefined, "async");
-  },
-
-  // from: "async", "sync", or "throw".
-  // self._threw is set by _run, and indicates that we're about to
-  // report an exception and not to allow any other fibers kicked off by
-  // the method to emit data.
-
-  _sendResponse: function (error, ret, from) {
-    var self = this;
-
-    if (from === "throw")
-      self._threw = true;
-
-    if (self._threw && from !== "throw")
-      // this is a different fiber.  don't emit data, don't print an error.
-      return;
-
-    if (self._responded) {
-      // another fiber already reported a result to the client.  if
-      // this fiber is throwing an exception, there's nothing left to do,
-      // since methods can only return a single result.  the exception
-      // has already been logged.  otherwise, throw an error: the user's
-      // code is responding more than once.
-      if (from === "throw")
-        return;
-      if (from === "sync" && self._response_was_error)
-        // it has to be OK to call this.error from a sync method,
-        // because how else would you signal an error?
-        return;
-      if (self._autoresponded)
-        throw new Error(
-          "The method has already returned, so it is too late to call " +
-            "respond() or error(). If you want to respond asynchronously, " +
-            "first use beginAsync() to prevent a response from being " +
-            "automatically sent.");
-      else
-        throw new Error(
-          "respond() or error() may only be called once per request");
-    }
-    self._responded = true;
-    self._response_was_error = !!error;
-    self._autoresponded = (from === "sync");
-
-    // if we haven't yet yielded to the next method in the queue, do
-    // that now, just before sending the response.
-    if (!self._calledNext) {
-      self._calledNext = true;
-      self._next && self._next();
-    }
-
-    // call the callback.  should happen exactly once per method.
-    self._callback && self._callback(error, ret);
-  },
-
-  // entry point
-  // - returns the immediate value (or throws an exception)
-  // - in any case, calls callback (if truthy) with eventual result
-  // - caller should call bindEnvironment on callback, or otherwise handle
-  //   any exceptions it throws
-  // - 'name' is for exception reporting
-  // - 'next' will be called when it's OK to start the next method from
-  //   this client
-  _run: function (args, callback, next) {
-    var self = this;
-    self._callback = callback;
-    self._next = next;
-    self._enclosing = Meteor._CurrentInvocation.get();
-    self.isSimulation = !!(self._enclosing && self._enclosing.isSimulation);
-
-    try {
-      var ret = Meteor._CurrentInvocation.withValue(self, function () {
-        return self._handler.apply(self, args);
-      });
-      if (!self._responded && !self._async)
-        self._sendResponse(undefined, ret, "sync");
-      return ret;
-    } catch (e) {
-      // send response in "throw" mode, which will lock out any other
-      // fibers kicked off by this method from emitting a response.
-      self._sendResponse({error: 500, reason: "Internal server error"},
-                         undefined, "throw");
-      // XXX improve error message (and how we report it)
-      if (!e.expected)
-        // tests can set the 'expected' flag on an exception so it
-        // won't go to the server log
-        Meteor._debug("Exception while invoking method '" +
-                      self._name + "'", e.stack);
-      throw e;
-    }
-  }
-});
-
 /******************************************************************************/
 /* LivedataSession                                                            */
 /******************************************************************************/
@@ -272,48 +130,57 @@ _.extend(Meteor._LivedataSession.prototype, {
     self.send(msg);
   },
 
-  // Throw 'msg' into the queue to be processed as an incoming
-  // message, but ignore it if 'socket' is not the currently connected
-  // socket.
-  processMessage: function (msg, socket) {
-    var self = this;
-    if (socket === self.socket) {
-      self.in_queue.push(msg);
-      self._tryProcessNext();
-    }
-  },
-
+  // Process 'msg' as an incoming message. (But as a guard against
+  // race conditions during reconnection, ignore the message if
+  // 'socket' is not the currently connected socket.)
+  //
   // We run the messages from the client one at a time, in the order
-  // given by the client. A message can set self.blocked to pause
-  // processing (eg, for an async method to complete.) To resume
-  // processing, clear self.blocked and call _tryProcessNext().
-
+  // given by the client. The message handler is passed an idempotent
+  // function 'unblock' which it may call to allow other messages to
+  // begin running in parallel in another fiber (for example, a method
+  // that wants to yield.) Otherwise, it is automatically unblocked
+  // when it returns.
+  //
   // Actually, we don't have to 'totally order' the messages in this
   // way, but it's the easiest thing that's correct. (unsub needs to
   // be ordered against sub, methods need to be ordered against each
   // other.)
-  _tryProcessNext: function () {
+  processMessage: function (msg_in, socket) {
     var self = this;
-
-    if (self.blocked || !self.in_queue.length || self.worker_running)
+    if (socket !== self.socket)
       return;
 
+    self.in_queue.push(msg_in);
+    if (self.worker_running)
+      return;
     self.worker_running = true;
-    Fiber(function () {
-      while (true) {
-        var more = !self.blocked && self.in_queue.length;
-        if (!more) {
-          self.worker_running = false;
-          return;
-        }
 
-        var msg = self.in_queue.shift();
+    var processNext = function () {
+      var msg = self.in_queue.shift();
+      if (!msg) {
+        self.worker_running = false;
+        return;
+      }
+
+      Fiber(function () {
+        var blocked = true;
+
+        var unblock = function () {
+          if (!blocked)
+            return; // idempotent
+          blocked = false;
+          processNext();
+        };
+
         if (msg.msg in self.protocol_handlers)
-          self.protocol_handlers[msg.msg].call(self, msg);
+          self.protocol_handlers[msg.msg].call(self, msg, unblock);
         else
           self.sendError('Bad request', msg);
-      }
-    }).run();
+        unblock(); // in case the handler didn't already do it
+      }).run();
+    };
+
+    processNext();
   },
 
   protocol_handlers: {
@@ -353,7 +220,7 @@ _.extend(Meteor._LivedataSession.prototype, {
       self.send({msg: 'nosub', id: msg.id});
     },
 
-    method: function (msg) {
+    method: function (msg, unblock) {
       var self = this;
 
       // reject malformed messages
@@ -396,42 +263,39 @@ _.extend(Meteor._LivedataSession.prototype, {
         return;
       }
 
-      // Invocations sent by a given client block later invocations
-      // sent by the same client until they return, unless the method
-      // explicitly allows later methods to run by calling
-      // beginAsync(true).
-
-      // XXX for completeness, should probably have a way of sending a
-      // response *without* unblocking processing (and doing that
-      // later..)
-
-      self.blocked = true;
-
-      var callback = function (error, result) {
-        var payload = error ? {error: error} : {result: result};
-
-        self.result_cache[msg.id] =
-          _.extend({when: +(new Date)}, payload);
-
-        self.send(
-          _.extend({msg: 'result', id: msg.id}, payload));
-        fence.arm();
-      };
-
-      var next = function (error, ret) {
-        self.blocked = false;
-        self._tryProcessNext()
-      };
-
-      var invocation = new Meteor._ServerMethodInvocation(msg.method, handler);
+      var invocation = new Meteor._MethodInvocation(false /* is_simulation */,
+                                                   unblock);
       try {
-        Meteor._CurrentWriteFence.withValue(fence, function () {
-          invocation._run(msg.params || [], callback, next);
-        });
+        var ret =
+          Meteor._CurrentWriteFence.withValue(fence, function () {
+            return Meteor._CurrentInvocation.withValue(invocation, function () {
+              return handler.apply(invocation, msg.params || []);
+            });
+          });
       } catch (e) {
-        // _run will have already logged the exception (and told the
-        // client, if appropriate)
+        var exception = e;
       }
+
+      fence.arm(); // we're done adding writes to the fence
+      unblock(); // unblock, if the method hasn't done it already
+
+      // "blind" exceptions other than those that were deliberately
+      // thrown to signal errors to the client
+      if (exception && !(exception instanceof Meteor.Error)) {
+        // tests can set the 'expected' flag on an exception so it
+        // won't go to the server log
+        if (!exception.expected)
+          Meteor._debug("Exception while invoking method '" +
+                        msg.method + "'", exception.stack);
+        exception = new Meteor.Error(500, "Internal server error");
+      }
+
+      // send response and add to cache
+      var payload =
+        exception ? {error: exception} : (ret !== undefined ?
+                                          {result: ret} : {});
+      self.result_cache[msg.id] = _.extend({when: +(new Date)}, payload);
+      self.send(_.extend({msg: 'result', id: msg.id}, payload));
     }
   },
 
@@ -880,23 +744,38 @@ _.extend(Meteor._LivedataServer.prototype, {
     var self = this;
 
     if (callback)
+      // It's not really necessary to do this, since we immediately
+      // run the callback in this fiber before returning, but we do it
+      // anyway for regularity.
       callback = Meteor.bindEnvironment(callback, function (e) {
         // XXX improve error message (and how we report it)
         Meteor._debug("Exception while delivering result of invoking '" +
                       name + "'", e.stack);
       });
-    else
-      callback = function () {};
 
+    // Run the handler
     var handler = self.method_handlers[name];
-    if (!handler) {
-      if (callback)
-        callback({error: 404, reason: "Method not found"});
-      throw new Error("No such method '" + name + "'");
+    if (!handler)
+      var exception = new Meteor.Error(404, "Method not found");
+    else {
+      var invocation = new Meteor._MethodInvocation(false /* is_simulation */);
+      try {
+        var ret = Meteor._CurrentInvocation.withValue(invocation, function () {
+          return handler.apply(invocation, args)
+        });
+      } catch (e) {
+        var exception = e;
+      }
     }
 
-    var invocation = new Meteor._ServerMethodInvocation(name, handler);
-    return invocation._run(args, callback);
+    // Return the result in whichever way the caller asked for it
+    if (callback) {
+      callback(exception, ret);
+      return;
+    }
+    if (exception)
+      throw exception;
+    return ret;
   },
 
   // A much more elegant way to do this would be: let any autopublish
diff --git a/packages/livedata/livedata_test_service.js b/packages/livedata/livedata_test_service.js
index efcabba787..c87e66ac26 100644
--- a/packages/livedata/livedata_test_service.js
+++ b/packages/livedata/livedata_test_service.js
@@ -1,15 +1,20 @@
 Meteor.methods({
+  nothing: function () {
+  },
   echo: function (/* arguments */) {
     return _.toArray(arguments);
   },
-  exception: function (where) {
+  exception: function (where, intended) {
     var shouldThrow =
       (Meteor.is_server && where === "server") ||
       (Meteor.is_client && where === "client") ||
       where === "both";
 
     if (shouldThrow) {
-      e = new Error("Test method throwing an exception");
+      if (intended)
+        e = new Meteor.Error(999, "Client-visible test exception");
+      else
+        e = new Error("Test method throwing an exception");
       e.expected = true;
       throw e;
     }
@@ -39,20 +44,16 @@ Meteor.methods({
     if (Meteor.is_server)
       cheat = false;
 
-    if (!from) {
-      this.error(404, "No such account " + from_name + " in " + world);
-      return;
-    }
+    if (!from)
+      throw new Meteor.Error(404,
+                             "No such account " + from_name + " in " + world);
 
-    if (!to) {
-      this.error(404, "No such account " + to_name + " in " + world);
-      return;
-    }
+    if (!to)
+      throw new Meteor.Error(404,
+                             "No such account " + to_name + " in " + world);
 
-    if (from.balance < amount && !cheat) {
-      this.error(409, "Insufficient funds");
-      return;
-    }
+    if (from.balance < amount && !cheat)
+      throw new Meteor.Error(409, "Insufficient funds");
 
     Ledger.update({_id: from._id}, {$inc: {balance: -amount}});
     Ledger.update({_id: to._id}, {$inc: {balance: amount}});
diff --git a/packages/livedata/livedata_tests.js b/packages/livedata/livedata_tests.js
index cc68e0cd55..eea08dcd44 100644
--- a/packages/livedata/livedata_tests.js
+++ b/packages/livedata/livedata_tests.js
@@ -103,10 +103,17 @@ var failure = function (test, code, reason) {
     test.equal(result, undefined);
     test.isTrue(error && typeof error === "object");
     if (error && typeof error === "object") {
-      code && test.equal(error.error, code);
-      reason && test.equal(error.reason, reason);
-      // XXX should check that other keys aren't present.. should
-      // probably use something like the Matcher we used to have
+      if (typeof code === "number") {
+        test.instanceOf(error, Meteor.Error);
+        code && test.equal(error.error, code);
+        reason && test.equal(error.reason, reason);
+        // XXX should check that other keys aren't present.. should
+        // probably use something like the Matcher we used to have
+      } else {
+        // for normal Javascript errors
+        test.instanceOf(error, Error);
+        test.equal(error.message, code);
+      }
     }
   };
 }
@@ -123,67 +130,148 @@ Tinytest.add("livedata - methods with colliding names", function (test) {
 });
 
 testAsyncMulti("livedata - basic method invocation", [
+  // Unknown methods
   function (test, expect) {
-    try {
-      var ret = Meteor.call("unknown method",
-                            expect(failure(test, 404, "Method not found")));
-    } catch (e) {
-      // throws immediately on server, but still calls callback
-      test.isTrue(Meteor.is_server);
-      return;
-    }
-
-    // returns undefined on client, then calls callback
-    test.isTrue(Meteor.is_client);
-    test.equal(ret, undefined);
-  },
-
-  function (test, expect) {
-    var ret = Meteor.call("echo", expect(undefined, []));
-    test.equal(ret, []);
-  },
-
-  function (test, expect) {
-    var ret = Meteor.call("echo", 12, expect(undefined, [12]));
-    test.equal(ret, [12]);
-  },
-
-  function (test, expect) {
-    var ret = Meteor.call("echo", 12, {x: 13}, expect(undefined, [12, {x: 13}]));
-    test.equal(ret, [12, {x: 13}]);
-  },
-
-  function (test, expect) {
-    test.throws(function () {
-      var ret = Meteor.call("exception", "both",
-                            expect(failure(test, 500, "Internal server error")));
-    });
-  },
-
-  function (test, expect) {
-    try {
-      var ret = Meteor.call("exception", "server",
-                            expect(failure(test, 500, "Internal server error")));
-    } catch (e) {
-      test.isTrue(Meteor.is_server);
-      return;
-    }
-
-    test.isTrue(Meteor.is_client);
-    test.equal(ret, undefined);
-  },
-
-  function (test, expect) {
-    if (Meteor.is_client) {
-      test.throws(function () {
-        var ret = Meteor.call("exception", "client", expect(undefined, undefined));
-      });
-    } else {
-      var ret = Meteor.call("exception", "client", expect(undefined, undefined));
+    if (Meteor.is_server) {
+      // On server, with no callback, throws exception
+      try {
+        var ret = Meteor.call("unknown method");
+      } catch (e) {
+        test.equal(e.error, 404);
+        var threw = true;
+      }
+      test.isTrue(threw);
       test.equal(ret, undefined);
     }
-  }
 
+    if (Meteor.is_client) {
+      // On client, with no callback, just returns undefined
+      var ret = Meteor.call("unknown method");
+      test.equal(ret, undefined);
+    }
+
+    // On either, with a callback, calls the callback and does not throw
+    var ret = Meteor.call("unknown method",
+                          expect(failure(test, 404, "Method not found")));
+    test.equal(ret, undefined);
+  },
+
+  function (test, expect) {
+    // make sure 'undefined' is preserved as such, instead of turning
+    // into null (JSON does not have 'undefined' so there is special
+    // code for this)
+    if (Meteor.is_server)
+      test.equal(Meteor.call("nothing"), undefined);
+    if (Meteor.is_client)
+      test.equal(Meteor.call("nothing"), undefined);
+
+    test.equal(Meteor.call("nothing", expect(undefined, undefined)), undefined);
+  },
+
+  function (test, expect) {
+    if (Meteor.is_server)
+      test.equal(Meteor.call("echo"), []);
+    if (Meteor.is_client)
+      test.equal(Meteor.call("echo"), undefined);
+
+    test.equal(Meteor.call("echo", expect(undefined, [])), undefined);
+  },
+
+  function (test, expect) {
+    if (Meteor.is_server)
+      test.equal(Meteor.call("echo", 12), [12]);
+    if (Meteor.is_client)
+      test.equal(Meteor.call("echo", 12), undefined);
+
+    test.equal(Meteor.call("echo", 12, expect(undefined, [12])), undefined);
+  },
+
+  function (test, expect) {
+    if (Meteor.is_server)
+      test.equal(Meteor.call("echo", 12, {x: 13}), [12, {x: 13}]);
+    if (Meteor.is_client)
+      test.equal(Meteor.call("echo", 12, {x: 13}), undefined);
+
+    test.equal(Meteor.call("echo", 12, {x: 13},
+                           expect(undefined, [12, {x: 13}])), undefined);
+  },
+
+  function (test, expect) {
+    // No callback
+
+    if (Meteor.is_server) {
+      test.throws(function () {
+        Meteor.call("exception", "both");
+      });
+      test.throws(function () {
+        Meteor.call("exception", "server");
+      });
+      // No exception, because no code will run on the client
+      test.equal(Meteor.call("exception", "client"), undefined);
+    }
+
+    if (Meteor.is_client) {
+      // The client exception is thrown away because it's in the
+      // stub. The server exception is throw away because we didn't
+      // give a callback.
+      test.equal(Meteor.call("exception", "both"), undefined);
+      test.equal(Meteor.call("exception", "server"), undefined);
+      test.equal(Meteor.call("exception", "client"), undefined);
+    }
+
+    // With callback
+
+    if (Meteor.is_client) {
+      test.equal(
+        Meteor.call("exception", "both",
+                    expect(failure(test, 500, "Internal server error"))),
+        undefined);
+      test.equal(
+        Meteor.call("exception", "server",
+                    expect(failure(test, 500, "Internal server error"))),
+        undefined);
+      test.equal(Meteor.call("exception", "client"), undefined);
+    }
+
+    if (Meteor.is_server) {
+      test.equal(
+        Meteor.call("exception", "both",
+                    expect(failure(test, "Test method throwing an exception"))),
+        undefined);
+      test.equal(
+        Meteor.call("exception", "server",
+                    expect(failure(test, "Test method throwing an exception"))),
+        undefined);
+      test.equal(Meteor.call("exception", "client"), undefined);
+    }
+  },
+
+  function (test, expect) {
+    if (Meteor.is_server) {
+      var threw = false;
+      try {
+        Meteor.call("exception", "both", true);
+      } catch (e) {
+        threw = true;
+        test.equal(e.error, 999);
+        test.equal(e.reason, "Client-visible test exception");
+      }
+      test.isTrue(threw);
+    }
+
+    if (Meteor.is_client) {
+      test.equal(
+        Meteor.call("exception", "both", true,
+                    expect(failure(test, 999,
+                                   "Client-visible test exception"))),
+        undefined);
+      test.equal(
+        Meteor.call("exception", "server", true,
+                    expect(failure(test, 999,
+                                   "Client-visible test exception"))),
+        undefined);
+    }
+  }
 ]);
 
 
diff --git a/packages/mongo-livedata/collection.js b/packages/mongo-livedata/collection.js
index 583accff89..be58303c3d 100644
--- a/packages/mongo-livedata/collection.js
+++ b/packages/mongo-livedata/collection.js
@@ -88,29 +88,20 @@ Meteor.Collection = function (name, manager, driver) {
     self._prefix = '/' + name + '/';
     m[self._prefix + 'insert'] = function (/* selector, options */) {
       self._maybe_snapshot();
-      try {
-        self._collection.insert.apply(self._collection, _.toArray(arguments));
-      } catch (e) {
-        this.error({error: 500, reason: "Database write failed"});
-      }
+      // Allow exceptions to propagate
+      self._collection.insert.apply(self._collection, _.toArray(arguments));
     };
 
     m[self._prefix + 'update'] = function (/* selector, mutator, options */) {
       self._maybe_snapshot();
-      try {
-        self._collection.update.apply(self._collection, _.toArray(arguments));
-      } catch (e) {
-        this.error({error: 500, reason: "Database write failed"});
-      }
+      // Allow exceptions to propagate
+      self._collection.update.apply(self._collection, _.toArray(arguments));
     };
 
     m[self._prefix + 'remove'] = function (/* selector */) {
       self._maybe_snapshot();
-      try {
-        self._collection.remove.apply(self._collection, _.toArray(arguments));
-      } catch (e) {
-        this.error({error: 500, reason: "Database write failed"});
-      }
+      // Allow exceptions to propagate
+      self._collection.remove.apply(self._collection, _.toArray(arguments));
     };
 
     manager.methods(m);
@@ -147,24 +138,27 @@ _.extend(Meteor.Collection.prototype, {
   }
 });
 
-// 'insert' returns a copy of the inserted document with the _id
-// added. the others return nothing. all of them may throw exceptions
-// (see below.)
+// 'insert' immediately returns a copy of the inserted document with
+// the _id added. The others return nothing.
 //
-// all of them can take a function as the last argument. if provided,
-// it will be called when the actual result of the operation is known
-// from the database. it will be called with no arguments if the
-// operation succeeded, or with one argument, the DDP error, if it
-// failed. if no callback is provided, then if an error happens, it
-// will be logged with Meteor._debug.
+// Otherwise, the semantics are exactly like other methods: they take
+// a callback as an optional last argument; if no callback is
+// provided, they block until the operation is complete, and throw an
+// exception if it fails; if a callback is provided, then they don't
+// necessarily block, and they call the callback when they finish with
+// zero arguments on success, or one argument, an exception, on
+// failure; on the client, blocking is impossible, so if a callback
+// isn't provided, they just return immediately and any error
+// information is lost.
 //
-// if the operation fails before returning (eg, if on the server the
-// database returns failure), then an exception is raised AND the
-// callback (if provided) is called.
+// There's one more tweak. On the client, if you don't provide a
+// callback, then if there is an error, a message will be logged with
+// Meteor._debug.
 //
-// database drivers SHOULD default to doing the operations
-// synchronously, so that they don't return until the database has
-// received them, but in the future we MAY provide a flag to turn this
+// The intent (though this is actually determined by the underlying
+// drivers) is that the operations should be done synchronously, not
+// generating their result until the database has acknowledged
+// them. In the future maybe we should provide a flag to turn this
 // off.
 _.each(["insert", "update", "remove"], function (name) {
   Meteor.Collection.prototype[name] = function (/* arguments */) {
@@ -174,6 +168,18 @@ _.each(["insert", "update", "remove"], function (name) {
     if (args.length && args[args.length - 1] instanceof Function)
       var callback = args.pop();
 
+    if (Meteor.is_client && !callback)
+      // Client can't block, so it can't report errors by exception,
+      // only by callback. If they forget the callback, give them a
+      // default one that logs the error, so they aren't totally
+      // baffled if their writes don't work because their database is
+      // down.
+      callback = function (err) {
+        if (err) {
+          Meteor._debug(name + " failed: " + err.error + " -- " + err.reason);
+        }
+      };
+
     if (name === "insert") {
       if (!args.length)
         throw new Error("insert requires an argument");
@@ -187,24 +193,25 @@ _.each(["insert", "update", "remove"], function (name) {
 
     if (self._manager) {
       // NB: on failure, allow exception to propagate
-      self._manager.apply(self._prefix + name, args, function (err) {
-        if (err) {
-          if (callback)
-            callback(err);
-          else
-            Meteor._debug(name + " failed: " + err.error + " -- " + err.reason);
-        }
-      });
+      self._manager.apply(self._prefix + name, args, callback);
     }
     else {
       try {
         self._collection[name].apply(self._collection, args);
       } catch (e) {
-        if (callback)
-          callback({error: 500, reason: "Local database threw exception",
-                    detail: e.stack});
+        if (callback) {
+          callback(e);
+          return;
+        }
+
+        // Note that on the client, this will never happen, because
+        // we will have been provided with a default callback. (This
+        // is nice because it matches the behavior of named
+        // collections, which on the client never throw exceptions
+        // directly.)
         throw e;
       }
+
       callback && callback();
     }
 
diff --git a/packages/tinytest/tinytest_server.js b/packages/tinytest/tinytest_server.js
index d91a7cac23..eec2b23ceb 100644
--- a/packages/tinytest/tinytest_server.js
+++ b/packages/tinytest/tinytest_server.js
@@ -10,8 +10,11 @@ Meteor.publish('tinytest/results', function (run_id) {
 
 Meteor.methods({
   'tinytest/run': function (run_id) {
-    var request = this;
-    request.beginAsync();
+    this.unblock();
+
+    // XXX using private API === lame
+    var Future = __meteor_bootstrap__.require('fibers/future');
+    var future = new Future;
 
     Meteor._runTests(function (report) {
       /* onReport */
@@ -19,7 +22,9 @@ Meteor.methods({
       Meteor.refresh({collection: 'tinytest_results', run_id: run_id});
     }, function () {
       /* onComplete */
-      request.respond();
+      future['return']();
     });
+
+    future.wait();
   }
 });

From 413c0778b52fce4f87e6ad3b3935e1ca40f8d4f0 Mon Sep 17 00:00:00 2001
From: Geoff Schmidt <geoff@geoffschmidt.com>
Date: Fri, 16 Mar 2012 02:22:47 -0700
Subject: [PATCH 06/10] move testAsyncMulti into test-helpers

---
 packages/livedata/livedata_tests.js  | 99 ----------------------------
 packages/livedata/package.js         |  1 +
 packages/test-helpers/async_multi.js | 97 +++++++++++++++++++++++++++
 packages/test-helpers/package.js     |  1 +
 4 files changed, 99 insertions(+), 99 deletions(-)
 create mode 100644 packages/test-helpers/async_multi.js

diff --git a/packages/livedata/livedata_tests.js b/packages/livedata/livedata_tests.js
index eea08dcd44..f9e0ca30d7 100644
--- a/packages/livedata/livedata_tests.js
+++ b/packages/livedata/livedata_tests.js
@@ -1,102 +1,3 @@
-// XXX should probably move this into a testing helpers package so it
-// can be used by other tests
-
-var ExpectationManager = function (test, onComplete) {
-  var self = this;
-
-  self.test = test;
-  self.onComplete = onComplete;
-  self.closed = false;
-  self.dead = false;
-  self.outstanding = 0;
-};
-
-_.extend(ExpectationManager.prototype, {
-  expect: function (/* arguments */) {
-    var self = this;
-
-    if (typeof arguments[0] === "function")
-      var expected = arguments[0];
-    else
-      var expected = _.toArray(arguments);
-
-    if (self.closed)
-      throw new Error("Too late to add more expectations to the test");
-    self.outstanding++;
-
-    return function (/* arguments */) {
-      if (typeof expected === "function")
-        expected.apply({}, arguments);
-      else
-        self.test.equal(_.toArray(arguments), expected);
-
-      self.outstanding--;
-      self._check_complete();
-    };
-  },
-
-  done: function () {
-    var self = this;
-    self.closed = true;
-    self._check_complete();
-  },
-
-  cancel: function () {
-    var self = this;
-    self.dead = true;
-  },
-
-  _check_complete: function () {
-    var self = this;
-    if (!self.outstanding && self.closed && !self.dead) {
-      self.dead = true;
-      self.onComplete();
-    }
-  }
-});
-
-var testAsyncMulti = function (name, funcs) {
-  var timeout = 5000;
-
-  Tinytest.addAsync(name, function (test, onComplete) {
-    var remaining = _.clone(funcs);
-
-    var runNext = function () {
-      var func = remaining.shift();
-      if (!func)
-        onComplete();
-      else {
-        var em = new ExpectationManager(test, function () {
-          Meteor.clearTimeout(timer);
-          runNext();
-        });
-
-        var timer = Meteor.setTimeout(function () {
-          em.cancel();
-          test.fail({type: "timeout", message: "Async batch timed out"});
-          onComplete();
-          return;
-        }, timeout);
-
-        try {
-          func(test, _.bind(em.expect, em));
-        } catch (exception) {
-          em.cancel();
-          test.exception(exception);
-          Meteor.clearTimeout(timer);
-          // Because we called test.exception, we're not to call onComplete.
-          return;
-        }
-        em.done();
-      }
-    };
-
-    runNext();
-  });
-};
-
-/******************************************************************************/
-
 // XXX should check error codes
 var failure = function (test, code, reason) {
   return function (error, result) {
diff --git a/packages/livedata/package.js b/packages/livedata/package.js
index dd5936dbe9..cba9c74bb4 100644
--- a/packages/livedata/package.js
+++ b/packages/livedata/package.js
@@ -28,6 +28,7 @@ Package.on_use(function (api) {
 Package.on_test(function (api) {
   api.use('livedata', ['client', 'server']);
   api.use('mongo-livedata', ['client', 'server']);
+  api.use('test-helpers', ['client', 'server']);
   api.use('tinytest');
   api.add_files('livedata_tests.js', ['client', 'server']);
   api.add_files('livedata_test_service.js', ['client', 'server']);
diff --git a/packages/test-helpers/async_multi.js b/packages/test-helpers/async_multi.js
new file mode 100644
index 0000000000..9a7fe7ffa9
--- /dev/null
+++ b/packages/test-helpers/async_multi.js
@@ -0,0 +1,97 @@
+// This depends on tinytest, so it's a little weird to put it in
+// test-helpers, but it'll do for now.
+
+var ExpectationManager = function (test, onComplete) {
+  var self = this;
+
+  self.test = test;
+  self.onComplete = onComplete;
+  self.closed = false;
+  self.dead = false;
+  self.outstanding = 0;
+};
+
+_.extend(ExpectationManager.prototype, {
+  expect: function (/* arguments */) {
+    var self = this;
+
+    if (typeof arguments[0] === "function")
+      var expected = arguments[0];
+    else
+      var expected = _.toArray(arguments);
+
+    if (self.closed)
+      throw new Error("Too late to add more expectations to the test");
+    self.outstanding++;
+
+    return function (/* arguments */) {
+      if (typeof expected === "function")
+        expected.apply({}, arguments);
+      else
+        self.test.equal(_.toArray(arguments), expected);
+
+      self.outstanding--;
+      self._check_complete();
+    };
+  },
+
+  done: function () {
+    var self = this;
+    self.closed = true;
+    self._check_complete();
+  },
+
+  cancel: function () {
+    var self = this;
+    self.dead = true;
+  },
+
+  _check_complete: function () {
+    var self = this;
+    if (!self.outstanding && self.closed && !self.dead) {
+      self.dead = true;
+      self.onComplete();
+    }
+  }
+});
+
+var testAsyncMulti = function (name, funcs) {
+  var timeout = 5000;
+
+  Tinytest.addAsync(name, function (test, onComplete) {
+    var remaining = _.clone(funcs);
+
+    var runNext = function () {
+      var func = remaining.shift();
+      if (!func)
+        onComplete();
+      else {
+        var em = new ExpectationManager(test, function () {
+          Meteor.clearTimeout(timer);
+          runNext();
+        });
+
+        var timer = Meteor.setTimeout(function () {
+          em.cancel();
+          test.fail({type: "timeout", message: "Async batch timed out"});
+          onComplete();
+          return;
+        }, timeout);
+
+        try {
+          func(test, _.bind(em.expect, em));
+        } catch (exception) {
+          em.cancel();
+          test.exception(exception);
+          Meteor.clearTimeout(timer);
+          // Because we called test.exception, we're not to call onComplete.
+          return;
+        }
+        em.done();
+      }
+    };
+
+    runNext();
+  });
+};
+
diff --git a/packages/test-helpers/package.js b/packages/test-helpers/package.js
index ad0838dba4..24ba01cfd3 100644
--- a/packages/test-helpers/package.js
+++ b/packages/test-helpers/package.js
@@ -6,6 +6,7 @@ Package.on_use(function (api, where) {
   where = where || ["client", "server"];
 
   api.add_files('try_all_permutations.js', where);
+  api.add_files('async_multi.js', where);
 });
 
 Package.on_test(function (api) {

From 176dd68199d2b00e205885357c11d90e7240e8a6 Mon Sep 17 00:00:00 2001
From: Geoff Schmidt <geoff@geoffschmidt.com>
Date: Fri, 16 Mar 2012 02:40:44 -0700
Subject: [PATCH 07/10] Suppress console log messages during tests

---
 packages/logging/logging.js      | 12 ++++++++++++
 packages/logging/logging_test.js |  1 +
 2 files changed, 13 insertions(+)

diff --git a/packages/logging/logging.js b/packages/logging/logging.js
index a534158dd8..653ff1151f 100644
--- a/packages/logging/logging.js
+++ b/packages/logging/logging.js
@@ -1,4 +1,6 @@
 (function() {
+  var suppress = 0;
+
   // replacement for console.log. This is a temporary API. We should
   // provide a real logging API soon (possibly just a polyfill for
   // console?)
@@ -10,6 +12,10 @@
   // be very visible. if you change _debug to go someplace else, etc,
   // please fix the autopublish code to do something reasonable.
   Meteor._debug = function (/* arguments */) {
+    if (suppress) {
+      suppress--;
+      return;
+    }
     if (typeof console !== 'undefined' &&
         typeof console.log !== 'undefined') {
       if (arguments.length == 0) { // IE Companion breaks otherwise
@@ -20,4 +26,10 @@
       }
     }
   };
+
+  // Suppress the next 'count' Meteor._debug messsages. Use this to
+  // stop tests from spamming the console.
+  Meteor._suppress_log = function (count) {
+    suppress += count;
+  }
 })();
diff --git a/packages/logging/logging_test.js b/packages/logging/logging_test.js
index 037f993c78..5f121ef402 100644
--- a/packages/logging/logging_test.js
+++ b/packages/logging/logging_test.js
@@ -1,6 +1,7 @@
 Tinytest.add("logging", function (test) {
 
   // Just run a log statement and make sure it doesn't explode.
+  Meteor._suppress_log(3);
   Meteor._debug();
   Meteor._debug("test one arg");
   Meteor._debug("this", "is", "a", "test");

From 6ed6fa75b5731fc418f3ba6a25a33f23e47e9fd4 Mon Sep 17 00:00:00 2001
From: Geoff Schmidt <geoff@geoffschmidt.com>
Date: Fri, 16 Mar 2012 02:42:54 -0700
Subject: [PATCH 08/10] test that Meteor.Errors are Errors

---
 packages/livedata/livedata_common.js | 2 ++
 packages/livedata/livedata_tests.js  | 8 ++++++++
 2 files changed, 10 insertions(+)

diff --git a/packages/livedata/livedata_common.js b/packages/livedata/livedata_common.js
index c53487aaae..0cc79d32f7 100644
--- a/packages/livedata/livedata_common.js
+++ b/packages/livedata/livedata_common.js
@@ -39,3 +39,5 @@ Meteor.Error = function (error, reason, details) {
   // it doesn't need to be pretty.)
   self.details = details;
 };
+
+Meteor.Error.prototype = new Error;
\ No newline at end of file
diff --git a/packages/livedata/livedata_tests.js b/packages/livedata/livedata_tests.js
index f9e0ca30d7..b81b486ad8 100644
--- a/packages/livedata/livedata_tests.js
+++ b/packages/livedata/livedata_tests.js
@@ -19,6 +19,14 @@ var failure = function (test, code, reason) {
   };
 }
 
+Tinytest.add("livedata - Meteor.Error", function (test) {
+  var error = new Meteor.Error(123, "kittens", "puppies");
+  test.instanceOf(error, Error);
+  test.equal(error.error, 123);
+  test.equal(error.reason, "kittens");
+  test.equal(error.details, "puppies");
+});
+
 Tinytest.add("livedata - methods with colliding names", function (test) {
   var x = LocalCollection.uuid();
   var m = {};

From c6cedac7eb7c3bdb73e67a6cebb8a4f68f6a4c46 Mon Sep 17 00:00:00 2001
From: Geoff Schmidt <geoff@geoffschmidt.com>
Date: Fri, 16 Mar 2012 02:43:20 -0700
Subject: [PATCH 09/10] Test database failure reporting

---
 packages/mongo-livedata/collection.js         |  3 +-
 packages/mongo-livedata/mongo_driver.js       | 24 +++++++++++++
 .../mongo-livedata/mongo_livedata_tests.js    | 34 +++++++++++++++++++
 packages/mongo-livedata/package.js            |  6 ++++
 4 files changed, 65 insertions(+), 2 deletions(-)
 create mode 100644 packages/mongo-livedata/mongo_livedata_tests.js

diff --git a/packages/mongo-livedata/collection.js b/packages/mongo-livedata/collection.js
index be58303c3d..242be24a0e 100644
--- a/packages/mongo-livedata/collection.js
+++ b/packages/mongo-livedata/collection.js
@@ -175,9 +175,8 @@ _.each(["insert", "update", "remove"], function (name) {
       // baffled if their writes don't work because their database is
       // down.
       callback = function (err) {
-        if (err) {
+        if (err)
           Meteor._debug(name + " failed: " + err.error + " -- " + err.reason);
-        }
       };
 
     if (name === "insert") {
diff --git a/packages/mongo-livedata/mongo_driver.js b/packages/mongo-livedata/mongo_driver.js
index f38f2fc73e..bceb13178e 100644
--- a/packages/mongo-livedata/mongo_driver.js
+++ b/packages/mongo-livedata/mongo_driver.js
@@ -93,6 +93,14 @@ _Mongo.prototype._maybeBeginWrite = function () {
 
 _Mongo.prototype.insert = function (collection_name, document) {
   var self = this;
+
+  if (collection_name === "___meteor_failure_test_collection" &&
+      document.fail) {
+    var e = new Error("Failure test");
+    e.expected = true;
+    throw e;
+  }
+
   var write = self._maybeBeginWrite();
 
   var finish = Meteor.bindEnvironment(function () {
@@ -127,6 +135,14 @@ _Mongo.prototype.insert = function (collection_name, document) {
 
 _Mongo.prototype.remove = function (collection_name, selector) {
   var self = this;
+
+  if (collection_name === "___meteor_failure_test_collection" &&
+      selector.fail) {
+    var e = new Error("Failure test");
+    e.expected = true;
+    throw e;
+  }
+
   var write = self._maybeBeginWrite();
 
   var finish = Meteor.bindEnvironment(function () {
@@ -164,6 +180,14 @@ _Mongo.prototype.remove = function (collection_name, selector) {
 
 _Mongo.prototype.update = function (collection_name, selector, mod, options) {
   var self = this;
+
+  if (collection_name === "___meteor_failure_test_collection" &&
+      selector.fail) {
+    var e = new Error("Failure test");
+    e.expected = true;
+    throw e;
+  }
+
   var write = self._maybeBeginWrite();
 
   var finish = Meteor.bindEnvironment(function () {
diff --git a/packages/mongo-livedata/mongo_livedata_tests.js b/packages/mongo-livedata/mongo_livedata_tests.js
new file mode 100644
index 0000000000..86aa53dd54
--- /dev/null
+++ b/packages/mongo-livedata/mongo_livedata_tests.js
@@ -0,0 +1,34 @@
+// This is a magic collection that fails its writes on the server when
+// the selector (or inserted document) contains fail: true.
+
+// XXX namespacing
+Meteor._FailureTestCollection =
+  new Meteor.Collection("___meteor_failure_test_collection");
+
+testAsyncMulti("mongo-livedata - database failure reporting", [
+  function (test, expect) {
+    var ftc = Meteor._FailureTestCollection;
+
+    var exception = function (err) {
+      test.instanceOf(err, Error);
+    };
+
+    _.each(["insert", "remove", "update"], function (op) {
+      if (Meteor.is_server) {
+        test.throws(function () {
+          ftc[op]({fail: true});
+        });
+
+        ftc[op]({fail: true}, expect(exception));
+      }
+
+      if (Meteor.is_client) {
+        ftc[op]({fail: true}, expect(exception));
+
+        // This would log to console in normal operation.
+        Meteor._suppress_log(1);
+        ftc[op]({fail: true});
+      }
+    });
+  }
+]);
diff --git a/packages/mongo-livedata/package.js b/packages/mongo-livedata/package.js
index 4aee3d30f6..e0a1d5c75d 100644
--- a/packages/mongo-livedata/package.js
+++ b/packages/mongo-livedata/package.js
@@ -21,3 +21,9 @@ Package.on_use(function (api) {
   api.add_files('remote_collection_driver.js', 'server');
   api.add_files('collection.js', ['client', 'server']);
 });
+
+Package.on_test(function (api) {
+  api.use('mongo-livedata');
+  api.use('tinytest');
+  api.add_files('mongo_livedata_tests.js', ['client', 'server']);
+});
\ No newline at end of file

From 8cdadb376216a4fe142fe68a6a79ad4441a718f4 Mon Sep 17 00:00:00 2001
From: Geoff Schmidt <geoff@geoffschmidt.com>
Date: Fri, 16 Mar 2012 02:50:18 -0700
Subject: [PATCH 10/10] make insert return the id of the new object (not a copy
 of the new object) avoid updating docs to prevent merge conflict (will handle
 out of band)

---
 examples/todos/client/todos.js              | 4 ++--
 examples/todos/server/bootstrap.js          | 2 +-
 examples/unfinished/azrael/client/azrael.js | 8 ++++----
 packages/mongo-livedata/collection.js       | 3 +--
 4 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/examples/todos/client/todos.js b/examples/todos/client/todos.js
index 336284d8c2..220fcd1d92 100644
--- a/examples/todos/client/todos.js
+++ b/examples/todos/client/todos.js
@@ -94,8 +94,8 @@ Template.create_list.events = {
     var target = $(evt.target);
     var text = target.val();
     if (evt.keyCode === 13 && text) {
-      var list = Lists.insert({name: text});
-      Router.setList(list._id);
+      var id = Lists.insert({name: text});
+      Router.setList(id);
       target.val('');
     }
   }
diff --git a/examples/todos/server/bootstrap.js b/examples/todos/server/bootstrap.js
index 1078d2a11d..ab89f0a8dd 100644
--- a/examples/todos/server/bootstrap.js
+++ b/examples/todos/server/bootstrap.js
@@ -40,7 +40,7 @@ Meteor.startup(function () {
 
     var timestamp = (new Date()).getTime();
     for (var i = 0; i < data.length; i++) {
-      var list_id = Lists.insert({name: data[i].name})._id;
+      var list_id = Lists.insert({name: data[i].name});
       for (var j = 0; j < data[i].contents.length; j++) {
         var info = data[i].contents[j];
         Todos.insert({list_id: list_id,
diff --git a/examples/unfinished/azrael/client/azrael.js b/examples/unfinished/azrael/client/azrael.js
index 282b9242e6..cc10742ad2 100644
--- a/examples/unfinished/azrael/client/azrael.js
+++ b/examples/unfinished/azrael/client/azrael.js
@@ -30,10 +30,10 @@ Template.add_room.events = {
   'click': function () {
     // XXX should put up dialog to get name
     // XXX should support automatically set created/updated timestamps
-    var obj = Rooms.insert({name: "New room",
-                            // XXX horrid syntax
-                            created: (new Date()).getTime()});
-    selectRoom(obj._id);
+    var room_id = Rooms.insert({name: "New room",
+                                // XXX horrid syntax
+                                created: (new Date()).getTime()});
+    selectRoom(room_id);
     // XXX XXX XXX this fails to work -- it leaves edit mode after
     // 1RTT. what happens is, the server echos the insert back to us,
     // and that is currently wired up to trigger a changed event on
diff --git a/packages/mongo-livedata/collection.js b/packages/mongo-livedata/collection.js
index 242be24a0e..46319ee89a 100644
--- a/packages/mongo-livedata/collection.js
+++ b/packages/mongo-livedata/collection.js
@@ -186,8 +186,7 @@ _.each(["insert", "update", "remove"], function (name) {
       args[0] = _.extend({}, args[0]);
       if ('_id' in args[0])
         throw new Error("Do not pass an _id to insert. Meteor will generate the _id for you.");
-      args[0]._id = Meteor.uuid();
-      var ret = args[0];
+      var ret = args[0]._id = Meteor.uuid();
     }
 
     if (self._manager) {