Merge pull request #533 from pelias/improved_error_handling

refactor elasticsearch error detection, improve test coverage

middleware/sendJSON.js

@@ -1,4 +1,12 @@
-var check = require('check-types');
+var check = require('check-types'),
+    es = require('elasticsearch'),
+    exceptions = require('elasticsearch-exceptions/lib/exceptions/SupportedExceptions');
+
+// create a list of regular expressions to match against.
+// note: list created when the server starts up; for performance reasons.
+var exceptionRegexList = exceptions.map( function( exceptionName ){
+  return new RegExp( '^' + exceptionName );
+});
 
 function sendJSONResponse(req, res, next) {
 
@@ -8,10 +16,11 @@ function sendJSONResponse(req, res, next) {
   }
 
   // default status
-  var statusCode = 200;
+  var statusCode = 200; // 200 OK
 
   // vary status code whenever an error was reported
   var geocoding = res.body.geocoding;
+
   if( check.array( geocoding.errors ) && geocoding.errors.length ){
 
     // default status for errors is 400 Bad Request
@@ -19,18 +28,33 @@ function sendJSONResponse(req, res, next) {
 
     // iterate over all reported errors
     geocoding.errors.forEach( function( err ){
+
       // custom status codes for instances of the Error() object.
       if( err instanceof Error ){
-        // we can extract the error type from the constructor name
-        switch( err.constructor.name ){
-          // elasticsearch errors
-          // see: https://github.com/elastic/elasticsearch-js/blob/master/src/lib/errors.js
-          case 'RequestTimeout': statusCode = 408; break; // 408 Request Timeout
-          case 'NoConnections': statusCode = 502; break; // 502 Bad Gateway
-          case 'ConnectionFault': statusCode = 502; break; // 502 Bad Gateway
-          case 'Serialization': statusCode = 500; break; // 500 Internal Server Error
-          case 'Generic': statusCode = 500; break; // 500 Internal Server Error
-          default: statusCode = 500; // 500 Internal Server Error
+        /*
+          elasticsearch errors
+          see: https://github.com/elastic/elasticsearch-js/blob/master/src/lib/errors.js
+
+          408 Request Timeout
+          500 Internal Server Error
+          502 Bad Gateway
+        */
+        if( err instanceof es.errors.RequestTimeout ){ statusCode = Math.max( statusCode, 408 ); }
+        else if( err instanceof es.errors.NoConnections ){ statusCode = Math.max( statusCode, 502 ); }
+        else if( err instanceof es.errors.ConnectionFault ){ statusCode = Math.max( statusCode, 502 ); }
+        else { statusCode = Math.max( statusCode, 500 ); }
+
+      /*
+        some elasticsearch errors are only returned as strings (not instances of Error).
+        in this case we (unfortunately) need to match the exception at position 0 inside the string.
+      */
+      } else if( check.string( err ) ){
+        for( var i=0; i<exceptionRegexList.length; i++ ){
+          // check error string against a list of known elasticsearch exceptions
+          if( err.match( exceptionRegexList[i] ) ){
+            statusCode = Math.max( statusCode, 500 );
+            break; // break on first match
+          }
         }
       }
     });

package.json

@@ -39,6 +39,7 @@
     "check-types": "^6.0.0",
     "cluster2": "git://github.com/missinglink/cluster2.git#node_zero_twelve",
     "elasticsearch": "^11.0.0",
+    "elasticsearch-exceptions": "0.0.4",
     "express": "^4.8.8",
     "express-http-proxy": "^0.6.0",
     "extend": "3.0.0",

test/ciao_test_data.js

@@ -54,5 +54,5 @@ actions.push( function( done ){
 
 // perform all actions in series
 async.series( actions, function( err, resp ){
-  console.log('test data inported');
+  console.log('test data imported');
 });

test/unit/middleware/sendJSON.js

@@ -0,0 +1,236 @@
+var es = require('elasticsearch'),
+    middleware = require('../../../middleware/sendJSON');
+
+module.exports.tests = {};
+
+module.exports.tests.invalid = function(test, common) {
+  test('invalid $res', function(t) {
+    var res;
+
+    middleware(null, res, function () {
+      t.pass('next() called.');
+      t.end();
+    });
+  });
+
+  test('invalid $res.body', function(t) {
+    var res = { body: 1 };
+
+    middleware(null, res, function () {
+      t.pass('next() called.');
+      t.end();
+    });
+  });
+
+  test('invalid $res.body.geocoding', function(t) {
+    var res = { body: { geocoding: 1 } };
+
+    middleware(null, res, function () {
+      t.pass('next() called.');
+      t.end();
+    });
+  });
+};
+
+module.exports.tests.default_status = function(test, common) {
+  test('no errors', function(t) {
+    var res = { body: { geocoding: {} } };
+
+    res.status = function( code ){
+      return { json: function( body ){
+        t.equal( code, 200, '200 OK' );
+        t.deepEqual( body, res.body, 'body set' );
+        t.end();
+      }};
+    };
+
+    middleware(null, res);
+  });
+
+  test('empty errors array', function(t) {
+    var res = { body: { geocoding: {}, errors: [] } };
+
+    res.status = function( code ){
+      return { json: function( body ){
+        t.equal( code, 200, '200 OK' );
+        t.deepEqual( body, res.body, 'body set' );
+        t.end();
+      }};
+    };
+
+    middleware(null, res);
+  });
+};
+
+module.exports.tests.default_error_status = function(test, common) {
+  test('default error code', function(t) {
+    var res = { body: { geocoding: {
+      errors: [ 'an error' ]
+    }}};
+
+    res.status = function( code ){
+      return { json: function( body ){
+        t.equal( code, 400, '400 Bad Request' );
+        t.deepEqual( body, res.body, 'body set' );
+        t.end();
+      }};
+    };
+
+    middleware(null, res);
+  });
+};
+
+module.exports.tests.generic_server_error = function(test, common) {
+  test('generic server error', function(t) {
+    var res = { body: { geocoding: {
+      errors: [ new Error('an error') ]
+    }}};
+
+    res.status = function( code ){
+      return { json: function( body ){
+        t.equal( code, 500, 'Internal Server Error' );
+        t.deepEqual( body, res.body, 'body set' );
+        t.end();
+      }};
+    };
+
+    middleware(null, res);
+  });
+};
+
+module.exports.tests.generic_elasticsearch_error = function(test, common) {
+  test('generic elasticsearch error', function(t) {
+    var res = { body: { geocoding: {
+      errors: [ new es.errors.Generic('an error') ]
+    }}};
+
+    res.status = function( code ){
+      return { json: function( body ){
+        t.equal( code, 500, 'Internal Server Error' );
+        t.deepEqual( body, res.body, 'body set' );
+        t.end();
+      }};
+    };
+
+    middleware(null, res);
+  });
+};
+
+module.exports.tests.request_timeout = function(test, common) {
+  test('request timeout', function(t) {
+    var res = { body: { geocoding: {
+      errors: [ new es.errors.RequestTimeout('an error') ]
+    }}};
+
+    res.status = function( code ){
+      return { json: function( body ){
+        t.equal( code, 408, 'Request Timeout' );
+        t.deepEqual( body, res.body, 'body set' );
+        t.end();
+      }};
+    };
+
+    middleware(null, res);
+  });
+};
+
+module.exports.tests.no_connections = function(test, common) {
+  test('no connections', function(t) {
+    var res = { body: { geocoding: {
+      errors: [ new es.errors.NoConnections('an error') ]
+    }}};
+
+    res.status = function( code ){
+      return { json: function( body ){
+        t.equal( code, 502, 'Bad Gateway' );
+        t.deepEqual( body, res.body, 'body set' );
+        t.end();
+      }};
+    };
+
+    middleware(null, res);
+  });
+};
+
+module.exports.tests.connection_fault = function(test, common) {
+  test('connection fault', function(t) {
+    var res = { body: { geocoding: {
+      errors: [ new es.errors.ConnectionFault('an error') ]
+    }}};
+
+    res.status = function( code ){
+      return { json: function( body ){
+        t.equal( code, 502, 'Bad Gateway' );
+        t.deepEqual( body, res.body, 'body set' );
+        t.end();
+      }};
+    };
+
+    middleware(null, res);
+  });
+};
+
+module.exports.tests.serialization = function(test, common) {
+  test('serialization', function(t) {
+    var res = { body: { geocoding: {
+      errors: [ new es.errors.Serialization('an error') ]
+    }}};
+
+    res.status = function( code ){
+      return { json: function( body ){
+        t.equal( code, 500, 'Internal Server Error' );
+        t.deepEqual( body, res.body, 'body set' );
+        t.end();
+      }};
+    };
+
+    middleware(null, res);
+  });
+};
+
+module.exports.tests.search_phase_execution_exception = function(test, common) {
+  test('search phase execution exception', function(t) {
+    var res = { body: { geocoding: {
+      errors: [ 'SearchPhaseExecutionException[ foo ]' ]
+    }}};
+
+    res.status = function( code ){
+      return { json: function( body ){
+        t.equal( code, 500, 'Internal Server Error' );
+        t.deepEqual( body, res.body, 'body set' );
+        t.end();
+      }};
+    };
+
+    middleware(null, res);
+  });
+};
+
+module.exports.tests.unknown_exception = function(test, common) {
+  test('unknown exception', function(t) {
+    var res = { body: { geocoding: {
+      errors: [ 'MadeUpExceptionName[ foo ]' ]
+    }}};
+
+    res.status = function( code ){
+      return { json: function( body ){
+        t.equal( code, 400, '400 Bad Request' );
+        t.deepEqual( body, res.body, 'body set' );
+        t.end();
+      }};
+    };
+
+    middleware(null, res);
+  });
+};
+
+module.exports.all = function (tape, common) {
+
+  function test(name, testFunction) {
+    return tape('[middleware] sendJSON: ' + name, testFunction);
+  }
+
+  for( var testCase in module.exports.tests ){
+    module.exports.tests[testCase](test, common);
+  }
+};

test/unit/run.js

@@ -27,6 +27,7 @@ var tests = [
   require('./middleware/localNamingConventions'),
   require('./middleware/dedupe'),
   require('./middleware/parseBBox'),
+  require('./middleware/sendJSON'),
   require('./middleware/normalizeParentIds'),
   require('./query/autocomplete'),
   require('./query/autocomplete_defaults'),