Merge pull request #171 from jlipps/master

better cleanup, less crashing

app/controller.js

@@ -138,6 +138,7 @@ exports.createSession = function(req, res) {
   } else {
     req.appium.start(req.body.desiredCapabilities, function(err, instance) {
       if (err) {
+        logger.error("Failed to start an Appium session, err was: " + err);
         respondError(req, res, status.codes.NoSuchDriver);
       } else {
         next(req.appium.sessionId, instance);
@@ -527,3 +528,7 @@ var mobileCmdMap = {
 exports.produceError = function(req, res) {
   req.device.proxy("thisisnotvalidjs", getResponseHandler(req, res));
 };
+
+exports.crash = function() {
+  throw new Error("We just tried to crash Appium!");
+};

app/ios.js

@@ -5,6 +5,7 @@ var path = require('path')
   , _ = require('underscore')
   , logger = require('../logger').get('appium')
   , sock = '/tmp/instruments_sock'
+  , glob = require('glob')
   , instruments = require('../instruments/instruments')
   , uuid = require('uuid-js')
   , timeWarp = require('../warp.js').timeWarp
@@ -57,6 +58,28 @@ var IOS = function(rest, app, udid, verbose, removeTraceDir, warp) {
   };
 };
 
+IOS.prototype.cleanup = function(cb) {
+  logger.info("Cleaning up instruments files");
+  if (this.removeTraceDir) {
+    glob("*.trace", {}, function(err, files) {
+      if (err) {
+        logger.error("Could not glob for tracedirs: " + err.message);
+      } else {
+        _.each(files, function(file) {
+          file = path.resolve(process.cwd(), file);
+          rimraf(file, function() {
+            logger.info("Cleaned up " + file);
+          });
+        });
+      }
+    });
+  }
+  rimraf(sock, function() {
+    logger.info("Cleaned up instruments socket " + sock);
+    cb();
+  });
+};
+
 IOS.prototype.start = function(cb, onDie) {
   var me = this;
   var didLaunch = false;
@@ -92,6 +115,7 @@ IOS.prototype.start = function(cb, onDie) {
     this.instruments = null;
     if (me.removeTraceDir && traceDir) {
       rimraf(traceDir, function() {
+        logger.info("Deleted tracedir we heard about from instruments (" + traceDir + ")");
         me.onStop(code);
       });
     } else {
@@ -100,18 +124,20 @@ IOS.prototype.start = function(cb, onDie) {
   };
 
   if (this.instruments === null) {
-    if (this.warp) {
-      timeWarp(50, 1000);
-    }
-    this.instruments = instruments(
-      this.app
-      , this.udid
-      , path.resolve(__dirname, 'uiauto/bootstrap.js')
-      , path.resolve(__dirname, 'uiauto/Automation.tracetemplate')
-      , sock
-      , onLaunch
-      , onExit
-    );
+    this.cleanup(_.bind(function() {
+      if (this.warp) {
+        timeWarp(50, 1000);
+      }
+      this.instruments = instruments(
+        this.app
+        , this.udid
+        , path.resolve(__dirname, 'uiauto/bootstrap.js')
+        , path.resolve(__dirname, 'uiauto/Automation.tracetemplate')
+        , sock
+        , onLaunch
+        , onExit
+      );
+    }, this));
   }
 
 };

app/routing.js

@@ -57,8 +57,9 @@ module.exports = function(appium) {
   rest.post('/wd/hub/session/:sessionId?/execute', controller.execute);
   rest.get('/wd/hub/session/:sessionId?/title', controller.title);
 
-  // this is for testing purposes only
+  // these are for testing purposes only
   rest.post('/wd/hub/produce_error', controller.produceError);
+  rest.post('/wd/hub/crash', controller.crash);
 
   // appium-specific extensions to JSONWP
   // these aren't part of JSONWP but we want them or something like them to be

package.json

@@ -41,6 +41,7 @@
     "node-bplist-creator": "~0.0.1",
     "node-uuid": "~1.4.0",
     "underscore.string": "~2.3.1",
+    "glob": "~3.1.20",
     "unzip": "~0.1.1"
   },
   "scripts": {

server.js

@@ -6,6 +6,7 @@ var http = require('http')
   , appium = require('./app/appium')
   , bodyParser = require('./middleware').parserWrap
   , checkWarpDrive = require('./warp.js').checkWarpDrive
+  , status = require('./app/uiauto/lib/status')
   , parser = require('./app/parser');
 
 var doWarpCheck = function(wantWarp, cb) {
@@ -51,6 +52,14 @@ var main = function(args, readyCb, doneCb) {
     rest.use(bodyParser);
     rest.use(express.methodOverride());
     rest.use(rest.router);
+    // catch all error handler
+    rest.use(function(e, req, res, next) {
+      res.send(500, {
+        status: status.codes.UnknownError.code
+        , value: "ERROR running Appium command: " + e.message
+      });
+      next(e);
+    });
   });
 
   // Instantiate the appium instance

test/functional/appium/appiumutils.js

@@ -109,8 +109,8 @@ describe("appiumutils", function() {
 
   it('should respond nicely to js errors', function(done) {
     device.proxy("blargimarg", function(err, res) {
-      should.exist(err);
-      should.ok(~res);
+      should.not.exist(err);
+      res.status.should.equal(17);
       done();
     });
   });

test/functional/appium/jsonwp.js

@@ -4,13 +4,17 @@
 var should = require("should")
   , serverUrl = 'http://localhost:4723'
   , serverHub = serverUrl + '/wd/hub/session'
+  , path = require('path')
+  , appPath = "../../../sample-code/apps/TestApp/build/Release-iphonesimulator/TestApp.app"
   , request = require('request');
 
 var describeWithSession = function(desc, tests) {
   describe(desc, function() {
     var sessObj = {sessionId: null};
     beforeEach(function(done) {
-      var caps = {desiredCapabilities: {}};
+      var caps = {desiredCapabilities: {
+        app: path.resolve(__dirname, appPath)
+      }};
       request.post({url: serverHub, json: caps}, function(err, res) {
         should.not.exist(err);
         res.statusCode.should.equal(303);
@@ -56,7 +60,7 @@ describe('JSONWP request', function() {
       request.post(url, function(err, res, body) {
         should.not.exist(err);
         res.statusCode.should.equal(501);
-        body.should.equal("Not Implemented");
+        JSON.parse(body).status.should.equal(13);
         done();
       });
     });
@@ -79,7 +83,20 @@ describe('JSONWP request', function() {
         res.statusCode.should.equal(500);
         should.ok(body);
         body = JSON.parse(body);
-        should.ok(body.value.message);
+        should.ok(body.value);
+        done();
+      });
+    });
+  });
+  describeWithSession('that generates a server crash', function() {
+    it('should respond with a 500', function(done) {
+      var url = serverUrl + '/wd/hub/crash';
+      request.post(url, function(err, res, body) {
+        should.not.exist(err);
+        res.statusCode.should.equal(500);
+        should.ok(body);
+        body = JSON.parse(body);
+        should.ok(body.value);
         done();
       });
     });