Add error handling middleware

- Add rest-json-error-handler package for handling Connect errors

Known Issues:
- Unable to propagate an error from within an endpoint (any function
  passed to `JsonRoutes.add()`). Tried adding a middleware handler,
  `JsonRoutes.errorMiddleware()` similar to `JsonRoutes.middleware`, but
  added to the stack after the endpoint, but middleware added this way
  does not seem to be called.
- jshint error needs to be supressed since error handling middleware
  requires we pass a 4th, unused argument (just run tests to see error)

.travis.yml

@@ -10,3 +10,4 @@ env:
   - PACKAGES=./packages/rest-accounts-password PACKAGE_DIRS=./packages/
   - PACKAGES=./packages/rest-bearer-token-parser PACKAGE_DIRS=./packages/
   - PACKAGES=./packages/authenticate-user-by-token PACKAGE_DIRS=./packages/
+  - PACKAGES=./packages/rest-json-error-handler PACKAGE_DIRS=./packages/

packages/json-routes/json-routes.js

@@ -22,11 +22,19 @@ JsonRoutes.routes = [];
 // Save reference to router for later
 var connectRouter;
 
+// Error middleware must be added last, to catch errors from prior middleware
+JsonRoutes.errorMiddleware = connect();
+WebApp.rawConnectHandlers.use(JsonRoutes.errorMiddleware);
+
 // Register as a middleware
 WebApp.rawConnectHandlers.use(connectRoute(function (router) {
   connectRouter = router;
 }));
 
+
+// TODO Add error handling middleware here, so connect errors can be triggered
+// TODO from endpoints (currently only works if triggered from other middleware)
+
 JsonRoutes.add = function (method, path, handler) {
   // Make sure path starts with a slash
   if (path[0] !== "/") {

packages/json-routes/middleware.js

@@ -0,0 +1,3 @@
+/* global RestMiddleware:true */
+
+RestMiddleware = {};

packages/json-routes/package.js

@@ -22,8 +22,10 @@ Package.onUse(function(api) {
     'underscore'
   ], 'server');
   api.addFiles('json-routes.js', 'server');
+  api.addFiles('middleware.js', "server");
 
   api.export('JsonRoutes', 'server');
+  api.export('RestMiddleware');
 });
 
 Package.onTest(function(api) {

packages/rest-json-error-handler/.jshintrc

@@ -0,0 +1,130 @@
+{
+  // JSHint Meteor Configuration File
+  // Match the Meteor Style Guide
+  //
+  // By @raix with contributions from @aldeed and @awatson1978
+  // Source https://github.com/raix/Meteor-jshintrc
+  //
+  // See http://jshint.com/docs/ for more details
+
+  "maxerr"        : 50,       // {int} Maximum error before stopping
+
+  // Enforcing
+  "bitwise"       : true,     // true: Prohibit bitwise operators (&, |, ^, etc.)
+  "camelcase"     : false,    // false: Identifiers do not need to be in camelCase. We would like to enforce this except for when interacting with our api objects which use snakeCase properties.
+  "curly"         : false,    // false: Do not require {} for every new block or scope
+  "eqeqeq"        : true,     // true: Require triple equals (===) for comparison
+  "forin"         : true,     // true: Require filtering for..in loops with obj.hasOwnProperty()
+  "immed"         : false,    // true: Require immediate invocations to be wrapped in parens e.g. `(function () { } ());`
+  "indent"        : 2,        // {int} Number of spaces to use for indentation
+  "latedef"       : false,    // true: Require variables/functions to be defined before being used
+  "newcap"        : false,    // true: Require capitalization of all constructor functions e.g. `new F()`
+  "noarg"         : true,     // true: Prohibit use of `arguments.caller` and `arguments.callee`
+  "noempty"       : true,     // true: Prohibit use of empty blocks
+  "nonew"         : false,    // true: Prohibit use of constructors for side-effects (without assignment)
+  "plusplus"      : false,    // true: Prohibit use of `++` & `--`
+  "quotmark"      : false,    // Quotation mark consistency:
+  //   false    : do nothing (default)
+  //   true     : ensure whatever is used is consistent
+  //   "single" : require single quotes
+  //   "double" : require double quotes
+  "undef"         : true,     // true: Require all non-global variables to be declared (prevents global leaks)
+  "unused"        : true,     // true: Require all defined variables be used
+  "strict"        : false,    // false: Do not require all functions to be run in ES5 Strict Mode
+  "trailing"      : true,     // true: Prohibit trailing whitespaces
+  "maxparams"     : false,    // {int} Max number of formal params allowed per function
+  "maxdepth"      : false,    // {int} Max depth of nested blocks (within functions)
+  "maxstatements" : false,    // {int} Max number statements per function
+  "maxcomplexity" : false,    // {int} Max cyclomatic complexity per function
+  "maxlen"        : 80,       // {int} Max number of characters per line
+
+  // Relaxing
+  "asi"           : false,     // true: Tolerate Automatic Semicolon Insertion (no semicolons)
+  "boss"          : false,     // true: Tolerate assignments where comparisons would be expected
+  "debug"         : false,     // true: Allow debugger statements e.g. browser breakpoints.
+  "eqnull"        : false,     // true: Tolerate use of `== null`
+  "es5"           : false,     // true: Allow ES5 syntax (ex: getters and setters)
+  "esnext"        : false,     // true: Allow ES.next (ES6) syntax (ex: `const`)
+  "moz"           : false,     // true: Allow Mozilla specific syntax (extends and overrides esnext features)
+  // (ex: `for each`, multiple try/catch, function expression…)
+  "evil"          : false,     // true: Tolerate use of `eval` and `new Function()`
+  "expr"          : true,      // true: Tolerate `ExpressionStatement` as Programs
+  "funcscope"     : false,     // true: Tolerate defining variables inside control statements"
+  "globalstrict"  : true,      // true: Allow global "use strict" (also enables 'strict')
+  "iterator"      : false,     // true: Tolerate using the `__iterator__` property
+  "lastsemic"     : false,     // true: Tolerate omitting a semicolon for the last statement of a 1-line block
+  "laxbreak"      : false,     // true: Tolerate possibly unsafe line breakings
+  "laxcomma"      : false,     // true: Tolerate comma-first style coding
+  "loopfunc"      : false,     // true: Tolerate functions being defined in loops
+  "multistr"      : false,     // true: Tolerate multi-line strings
+  "proto"         : false,     // true: Tolerate using the `__proto__` property
+  "scripturl"     : false,     // true: Tolerate script-targeted URLs
+  "smarttabs"     : false,     // true: Tolerate mixed tabs/spaces when used for alignment
+  "shadow"        : false,     // true: Allows re-define variables later in code e.g. `var x=1; x=2;`
+  "sub"           : false,     // true: Tolerate using `[]` notation when it can still be expressed in dot notation
+  "supernew"      : false,     // true: Tolerate `new function () { ... };` and `new Object;`
+  "validthis"     : false,     // true: Tolerate using this in a non-constructor function
+
+  // Environments
+  "browser"       : true,     // Web Browser (window, document, etc)
+  "couch"         : false,    // CouchDB
+  "devel"         : true,     // Development/debugging (alert, confirm, etc)
+  "dojo"          : false,    // Dojo Toolkit
+  "jquery"        : false,    // jQuery
+  "mootools"      : false,    // MooTools
+  "node"          : false,    // Node.js
+  "nonstandard"   : false,    // Widely adopted globals (escape, unescape, etc)
+  "prototypejs"   : false,    // Prototype and Scriptaculous
+  "rhino"         : false,    // Rhino
+  "worker"        : false,    // Web Workers
+  "wsh"           : false,    // Windows Scripting Host
+  "yui"           : false,    // Yahoo User Interface
+  //"meteor"        : false,    // Meteor.js
+
+  // Legacy
+  "nomen"         : false,    // true: Prohibit dangling `_` in variables
+  "onevar"        : false,    // true: Allow only one `var` statement per function
+  "passfail"      : false,    // true: Stop on first error
+  "white"         : false,    // true: Check against strict whitespace and indentation rules
+
+  // Custom globals, from http://docs.meteor.com, in the order they appear there
+  "globals"       : {
+    "Meteor": false,
+    "DDP": false,
+    "Mongo": false, //Meteor.Collection renamed to Mongo.Collection
+    "Session": false,
+    "Accounts": false,
+    "Template": false,
+    "Blaze": false,  //UI renamed to Blaze
+    "Spacebars": false,
+    "Match": false,
+    "check": false,
+    "Tracker": false, //Deps renamed to Tracker
+    "ReactiveVar": false,
+    "ReactiveDict": false,
+    "EJSON": false,
+    "Assets": false,
+    "Package": false,
+    "App": false, //mobile-config.js
+    "cordova": false,
+    "Cordova": false,
+
+    // Meteor internals
+    "DDPServer": false,
+    "global": false,
+    "Log": false,
+    "MongoInternals": false,
+    "process": false,
+    "Retry": false,
+    "WebApp": false,
+    "WebAppInternals": false,
+
+    // globals useful when creating Meteor packages
+    "Npm": false,
+    "Tinytest": false,
+
+    // common Meteor packages
+    "_": false,               // Underscore.js
+    "$": false               // jQuery
+  }
+}

packages/rest-json-error-handler/json_error_handler.js

@@ -0,0 +1,21 @@
+/* global RestMiddleware:true */
+
+/**
+ * Handle any connect errors with a standard JSON response
+ *
+ * Response looks like:
+ *   {
+ *     error: 'Error type',
+  *    reason: 'Cause of error'
+  *  }
+ *
+ * @middleware
+ */
+RestMiddleware.handleErrorAsJson = function (error, request, response, next) {
+  var body = JSON.stringify({ error: error.error, reason: error.reason });
+
+  response.statusCode = error.statusCode;
+  response.setHeader('Content-Type', 'application/json');
+  response.write(body);
+  response.end();
+};

packages/rest-json-error-handler/json_error_handler_tests.js

@@ -0,0 +1,44 @@
+/* global RestMiddleware:true */
+/* global JsonRoutes:true */
+/* global testAsyncMulti:true */
+/* global HTTP:true */
+
+if (Meteor.isServer) {
+  JsonRoutes.errorMiddleware.use(
+    '/handle-error',
+    RestMiddleware.handleErrorAsJson
+  );
+
+  // For testing purposes, trigger error from middleware above error handler in
+  // stack (until errors can be triggered from endpoints and not just prior
+  // middleware)
+  JsonRoutes.middleWare.use(
+    '/handle-error',
+    function (req, res, next) {
+      next({
+        statusCode: 400,
+        error: 'test-error',
+        reason: 'test reason'
+      });
+    }
+  );
+
+  JsonRoutes.add('get', 'handle-error', function (req, res) {
+    // Currently unable to propagate error from within an endpoint
+    //next({statusCode: 400, error: 'test-error', reason: 'test reason'});
+    JsonRoutes.sendResult(res, 200, true);
+  });
+}
+else { // Meteor.isClient
+  testAsyncMulti('Middleware - JSON Error Handling - ' +
+    'handle standard Connect error with JSON response', [
+    function (test, waitFor) {
+      HTTP.get(Meteor.absoluteUrl('/handle-error'),
+        waitFor(function (err, resp) {
+          test.equal(resp.statusCode, 400);
+          test.equal(resp.data.error, 'test-error');
+          test.equal(resp.data.reason, 'test reason');
+      }));
+    }
+  ]);
+}

packages/rest-json-error-handler/package.js

@@ -0,0 +1,25 @@
+Package.describe({
+  name: 'simple:rest-json-error-handler',
+  version: '0.0.1',
+  // Brief, one-line summary of the package.
+  summary: 'simple:rest middleware for handling standard Connect errors',
+  // URL to the Git repository containing the source code for this package.
+  git: 'https://github.com/stubailo/meteor-rest',
+  // By default, Meteor will default to using README.md for documentation.
+  // To avoid submitting documentation, set this field to null.
+  documentation: 'README.md'
+});
+
+Package.onUse(function(api) {
+  api.versionsFrom('1.0');
+  api.use('simple:json-routes@1.0.3');
+  api.addFiles('json_error_handler.js', 'server');
+});
+
+Package.onTest(function(api) {
+  api.use('tinytest');
+  api.use('test-helpers');
+  api.use('simple:json-routes@1.0.3');
+  api.use('simple:rest-json-error-handler');
+  api.addFiles('json_error_handler_tests.js');
+});

run-tests.sh

@@ -15,4 +15,5 @@ meteor test-packages \
   "$DIR/packages/json-routes" \
   "$DIR/packages/rest-bearer-token-parser" \
   "$DIR/packages/authenticate-user-by-token" \
-  "$DIR/packages/rest-accounts-password"
+  "$DIR/packages/rest-accounts-password" \
+  "$DIR/packages/rest-json-error-handler"