Fix and test some runner error handling issues

- Exit with failure if proxy starts to listen. Previously, we got stuck
  inside ProxyRunner.start(), and since we weren't waiting on any IO
  we'd exit 0!  (The existence of a yielded fiber is not sufficient to
  block Node exit.)

- Don't print various bits of startup text if we are stopped midway.

- Stop main Runner immediately if proxy or mongo runners invoke
  onFailure, rather than waiting for wait to be called and return.

- Ensure that a few Futures don't get return called on them multiple
  times.

- Ensure we don't try to call close() on the proxy's TCP server if it
  failed to listen in the first place.

tools/run-all.js

@@ -86,7 +86,7 @@ _.extend(Runner.prototype, {
     self.proxy.start();
 
     // print the banner only once we've successfully bound the port
-    if (! self.quiet) {
+    if (! self.quiet & ! self.stopped) {
       self.runLog.log("[[[[[ " + self.banner + " ]]]]]\n");
       self.runLog.log("=> Started proxy.");
     }
@@ -127,7 +127,8 @@ _.extend(Runner.prototype, {
 
       if (! self.quiet) {
         clearInterval(mongoProgressTimer);
-        self.runLog.log("=> Started MongoDB.");
+        if (! self.stopped)
+          self.runLog.log("=> Started MongoDB.");
       }
     }
 
@@ -151,6 +152,9 @@ _.extend(Runner.prototype, {
   // Idempotent
   stop: function () {
     var self = this;
+    if (self.stopped)
+      return;
+
     self.stopped = true;
     self.proxy.stop();
     self.updater.stop();
@@ -169,7 +173,7 @@ _.extend(Runner.prototype, {
     if (self.specifiedAppPort) {
       self.appPort = self.specifiedAppPort;
     } else {
-      self.appPort = 20000 + Math.floor(Math.random() * 10000);
+      self.appPort = require('./utils.js').randomPort();
     }
     if (self.proxy)
       self.proxy.proxyToPort = self.appPort;
@@ -223,15 +227,21 @@ _.extend(Runner.prototype, {
 //   appDir. Useful when you have autogenerated a test harness app
 //   based on some other app.
 exports.run = function (appDir, options) {
-  var fut = new Future;
-
   var runOptions = _.clone(options);
   var once = runOptions.once;
   delete runOptions.once;
 
+  var fut = new Future;
+
   _.extend(runOptions, {
     onFailure: function () {
-      fut['return']({ outcome: 'failure' });
+      // Ensure that runner stops now. You might think this is unnecessary
+      // because the runner is stopped immediately after `fut.wait()`, but if
+      // the failure happens while runner.start() is still running, we want the
+      // rest of start to stop, and it's not like fut['return'] magically makes
+      // us jump to a fut.wait() that hasn't happened yet!.
+      runner.stop();
+      fut.isResolved() || fut['return']({ outcome: 'failure' });
     },
     onRunEnd: function (result) {
       if (once ||
@@ -239,7 +249,7 @@ exports.run = function (appDir, options) {
           (result.outcome === "terminated" &&
            result.signal === undefined && result.code === undefined)) {
         runner.stop();
-        fut['return'](result);
+        fut.isResolved() || fut['return'](result);
         return false;  // stop restarting
       }
       runner.regenerateAppPort();

tools/run-proxy.js

@@ -29,6 +29,8 @@ _.extend(Proxy.prototype, {
     if (self.server)
       throw new Error("already running?");
 
+    self.started = false;
+
     var http = require('http');
     var net = require('net');
     var httpProxy = require('http-proxy');
@@ -65,6 +67,8 @@ _.extend(Proxy.prototype, {
         self.runLog.log('' + err);
       }
       self.onFailure();
+      // Allow start() to return.
+      fut.isResolved() || fut['return']();
     });
 
     // Don't crash if the app doesn't respond. instead return an error
@@ -88,7 +92,8 @@ _.extend(Proxy.prototype, {
 
     var fut = new Future;
     self.server.listen(self.listenPort, function () {
-      fut['return']();
+      self.started = true;
+      fut.isResolved() || fut['return']();
     });
 
     fut.wait();
@@ -98,7 +103,7 @@ _.extend(Proxy.prototype, {
   stop: function () {
     var self = this;
 
-    if (! self.server)
+    if (! self.server || ! self.started)
       return;
 
     // This stops listening but allows existing connections to

tools/selftest.js

@@ -581,7 +581,7 @@ var Run = function (execPath, options) {
   self.fakeMongoPort = null;
   self.fakeMongoConnection = null;
   if (options.fakeMongo) {
-    self.fakeMongoPort = 20000 + Math.floor(Math.random() * 10000);
+    self.fakeMongoPort = require('./utils.js').randomPort();
     self.env.METEOR_TEST_FAKE_MONGOD_CONTROL_PORT = self.fakeMongoPort;
   }
 };

tools/tests/run.js

@@ -1,6 +1,9 @@
 var selftest = require('../selftest.js');
 var Sandbox = selftest.Sandbox;
 var utils = require('../utils.js');
+var net = require('net');
+var Future = require('fibers/future');
+var _ = require('underscore');
 
 var MONGO_LISTENING =
   { stdout: " [initandlisten] waiting for connections on port" };
@@ -85,6 +88,51 @@ selftest.define("run --once", function () {
   run.expectExit(86);
 });
 
+selftest.define("run errors", function () {
+  var s = new Sandbox;
+  s.createApp("myapp", "empty");
+  s.cd("myapp");
+
+  // Prevent mongod from starting up.  (Note that "127.0.0.1" matches the
+  // interface that mongo uses.)
+  var proxyPort = utils.randomPort();
+  var mongoPort = proxyPort + 1;
+  var f = new Future;
+  var server = net.createServer().listen(mongoPort, "127.0.0.1", f.resolver());
+  f.wait();
+
+  var run = s.run("-p", proxyPort);
+  _.times(3, function () {
+    run.waitSecs(3);
+    run.match("Unexpected mongo exit code 48. Restarting.");
+  });
+  run.waitSecs(3);
+  run.match("Can't start Mongo server");
+  run.match("MongoDB exited because its port was closed");
+  run.match("running in the same project.\n");
+  run.expectEnd();
+  run.forbid("Started MongoDB");
+  run.expectExit(254);
+
+  f = new Future;
+  server.close(f.resolver());
+  f.wait();
+
+  // This time, prevent the proxy from starting. (This time, leaving out the
+  // interface name matches.)
+  f = new Future;
+  server = net.createServer().listen(proxyPort, f.resolver());
+  f.wait();
+
+  run = s.run("-p", proxyPort);
+  run.waitSecs(3);
+  run.match(/Can't listen on port.*another Meteor/);
+  run.expectExit(254);
+
+  f = new Future;
+  server.close(f.resolver());
+  f.wait();
+});
 
 selftest.define("update during run", ["checkout"], function () {
   var s = new Sandbox({

tools/utils.js

@@ -132,6 +132,11 @@ exports.randomToken = function () {
   return (Math.random() * 0x100000000 + 1).toString(36);
 };
 
+// Returns a random non-privileged port number.
+exports.randomPort = function () {
+  return 20000 + Math.floor(Math.random() * 10000);
+};
+
 // True if this looks like a valid email address. We deliberately
 // don't support
 // - quoted usernames (eg, "foo"@bar.com, " "@bar.com, "@"@bar.com)