Add missing callback, check HTTP response code

lib/index.js

@@ -768,6 +768,11 @@ function client () {
       vehicleLocations(null, function (err, response) {
          var i, item, route, data, stop;
 
+         if (err) {
+            callback(err, null);
+            return;
+         }
+
          // our response will be an object mapping routes to vehicles.  we dont
          // care about the values, but the keys tell us exactly which routes are
          // active.
@@ -981,9 +986,9 @@ function client () {
       request(url, function (err, response, data) {
          if (err) {
             cb(err, null);
-         }
-
-         else {
+         } else if (response.statusCode != 200) {
+            cb(new Error("Bad HTTP response " + response.statusCode), null);
+         } else {
             xmlparse(data, function (err, dom) {
                if (err) {
                   cb(err, null);

package.json

@@ -2,30 +2,28 @@
   "author": "Russ Frank <russ.frank@rutgers.edu>",
   "name": "nextbusjs",
   "description": "nextbus api wrapper",
-  "main"    : "lib/index",
+  "main": "lib/index",
   "scripts": {
-    "test": "vows --spec ./test/*.js"
+    "test": "vows --spec test/*.js"
   },
   "version": "1.0.1",
   "license": "MIT",
   "homepage": "http://github.com/russfrank/nextbusjs",
   "repository": {
     "url": "git://github.com/russfrank/nextbusjs.git"
   },
-  "scripts": {
-    "test": "vows --spec test/*.js"
-  },
   "engines": {
     "node": "0.10.x"
   },
   "devDependencies": {
-     "vows": "*",
-     "JSV": "*",
-     "async": "*",
-     "underscore": "*"
+    "JSV": "*",
+    "async": "*",
+    "nock": "^8.0.0",
+    "underscore": "*",
+    "vows": "*"
   },
   "dependencies": {
-     "request": "2.9.x",
-     "jsdom": "0.7.x"
+    "request": "2.9.x",
+    "jsdom": "0.7.x"
   }
 }

test/mocks.js

@@ -0,0 +1,79 @@
+var vows       = require('vows'),
+    assert     = require('assert'),
+    nextbus    = require('../lib/index').client,
+    jsv        = require('JSV').JSV.createEnvironment(),
+    readfile   = require('fs').readFile,
+    async      = require('async'),
+    nock       = require('nock'),
+    rutgers    = nextbus(),
+    invalidnb  = nextbus(),
+    dtconn     = nextbus();
+
+var baseURL = 'http://webservices.nextbus.com';
+var getPath = '/service/publicXMLFeed';
+
+var suite = vows.describe('mocked server responses');
+
+// Use separate batches to make sure the queries run sequentially
+
+suite.addBatch({
+    'agency cache on bad response': {
+        topic: function() {
+            var mockRutgersAgency = nock(baseURL)
+                .get(getPath)
+                .query({command: 'routeConfig', a: 'rutgers'})
+                .reply(404);
+            rutgers.cacheAgency('rutgers', this.callback);
+        },
+        'throws an error for bad cache attempt': function (err, data) {
+            assert.isNotNull(err);
+        }
+    }
+});
+
+suite.addBatch({
+    'agency cache on good response': {
+        topic: function() {
+            var mockRutgersAgency = nock(baseURL)
+                .get(getPath)
+                .query({command: 'routeConfig', a: 'rutgers'})
+                .replyWithFile(200, __dirname+'/replies/rutgers_routeConfig.xml');
+            rutgers.cacheAgency('rutgers', this.callback);
+        },
+        'cached mock routeConfig for rutgers': function (err, data) {
+            assert.isObject(data);
+        }
+    }
+});
+
+suite.addBatch({
+    'vehicleLocations 503 response handling (after a valid cache)' : {
+        topic: function () {
+            var mock503 = nock(baseURL)
+                .get(getPath)
+                .query(true)
+                .replyWithFile(503, __dirname+'/replies/503.html');
+            rutgers.vehicleLocations(null, this.callback, true);
+        },
+        'throws an error for bad response code' : function (err, data) {
+            assert.isNotNull(err);
+        }
+    }
+});
+
+suite.addBatch({
+    'vehicleLocations 200 empty response handling (after a valid cache)' : {
+        topic: function() {
+            var mockEmpty = nock(baseURL)
+                .get(getPath)
+                .query(true)
+                .reply(200, '');
+            rutgers.vehicleLocations(null, this.callback, true);
+        },
+        'throws an error for empty 200 response' : function (err, data) {
+            assert.isNotNull(err);
+        }
+    }
+});
+
+suite.export(module);

test/replies/503.html

@@ -0,0 +1,20 @@
+<html>
+<!-- This file is displayed when there is an internal server error (error 500)
+     which happens when apache is running but Tomcat is not.  This is important
+     because Tomcat takes several seconds to start being able to handle
+     messages.  The idea is that this file displays something not
+     so scary (instead of Internal Server Error) and then automatically
+     does a refresh in 5 seconds to try to load in the page again.  This
+     way the user doesn't have to do anything.  Don't want to do it too
+	 often because that would prevent the user from typing in another url.
+ -->
+
+<head>
+<title>NextBus - Please wait...</title>
+<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
+<meta http-equiv="refresh" content="5">
+</head>
+<font face="Arial, Helvetica, sans-serif" size="2">Please wait...
+</font>
+</body>
+</html>