From 0caca08787170977bb2d3fc16b1a62100fe0150c Mon Sep 17 00:00:00 2001
From: Peter Johnson <insomnia@rcpt.at>
Date: Wed, 4 May 2016 15:58:26 +0200
Subject: [PATCH] improve error matching by using string prefix matching on
 known elasticsearch exception names

---
 middleware/sendJSON.js           | 34 ++++++++++++++++++++++++------
 package.json                     |  1 +
 test/unit/middleware/sendJSON.js | 36 ++++++++++++++++++++++++--------
 3 files changed, 56 insertions(+), 15 deletions(-)

diff --git a/middleware/sendJSON.js b/middleware/sendJSON.js
index 6a33cfe7..8f31681f 100644
--- a/middleware/sendJSON.js
+++ b/middleware/sendJSON.js
@@ -1,5 +1,12 @@
 var check = require('check-types'),
-    es = require('elasticsearch');
+    es = require('elasticsearch'),
+    exceptions = require('elasticsearch-exceptions/lib/exceptions/SupportedExceptions');
+
+// create a list of regular expressions to match against.
+// note: list created at runtime for performance reasons.
+var exceptionRegexList = exceptions.map( function( exceptionName ){
+  return new RegExp( '^' + exceptionName );
+});
 
 function sendJSONResponse(req, res, next) {
 
@@ -9,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
@@ -20,6 +28,7 @@ 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 ){
         /*
@@ -30,10 +39,23 @@ function sendJSONResponse(req, res, next) {
           500 Internal Server Error
           502 Bad Gateway
         */
-        if( err instanceof es.errors.RequestTimeout ){ statusCode = 408; }
-        else if( err instanceof es.errors.NoConnections ){ statusCode = 502; }
-        else if( err instanceof es.errors.ConnectionFault ){ statusCode = 502; }
-        else { statusCode = 500; }
+        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
+          }
+        }
       }
     });
   }
diff --git a/package.json b/package.json
index f70cb57b..26ba7808 100644
--- a/package.json
+++ b/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",
diff --git a/test/unit/middleware/sendJSON.js b/test/unit/middleware/sendJSON.js
index d4166bc8..0bc21911 100644
--- a/test/unit/middleware/sendJSON.js
+++ b/test/unit/middleware/sendJSON.js
@@ -38,7 +38,7 @@ module.exports.tests.default_status = function(test, common) {
 
     res.status = function( code ){
       return { json: function( body ){
-        t.equal( code, 200, 'default status' );
+        t.equal( code, 200, '200 OK' );
         t.deepEqual( body, res.body, 'body set' );
         t.end();
       }};
@@ -52,7 +52,7 @@ module.exports.tests.default_status = function(test, common) {
 
     res.status = function( code ){
       return { json: function( body ){
-        t.equal( code, 200, 'default status' );
+        t.equal( code, 200, '200 OK' );
         t.deepEqual( body, res.body, 'body set' );
         t.end();
       }};
@@ -70,7 +70,7 @@ module.exports.tests.default_error_status = function(test, common) {
 
     res.status = function( code ){
       return { json: function( body ){
-        t.equal( code, 400, 'default status' );
+        t.equal( code, 400, '400 Bad Request' );
         t.deepEqual( body, res.body, 'body set' );
         t.end();
       }};
@@ -88,7 +88,7 @@ module.exports.tests.generic_server_error = function(test, common) {
 
     res.status = function( code ){
       return { json: function( body ){
-        t.equal( code, 500, 'default status' );
+        t.equal( code, 500, 'Internal Server Error' );
         t.deepEqual( body, res.body, 'body set' );
         t.end();
       }};
@@ -106,7 +106,7 @@ module.exports.tests.generic_elasticsearch_error = function(test, common) {
 
     res.status = function( code ){
       return { json: function( body ){
-        t.equal( code, 500, 'default status' );
+        t.equal( code, 500, 'Internal Server Error' );
         t.deepEqual( body, res.body, 'body set' );
         t.end();
       }};
@@ -124,7 +124,7 @@ module.exports.tests.request_timeout = function(test, common) {
 
     res.status = function( code ){
       return { json: function( body ){
-        t.equal( code, 408, 'default status' );
+        t.equal( code, 408, 'Request Timeout' );
         t.deepEqual( body, res.body, 'body set' );
         t.end();
       }};
@@ -142,7 +142,7 @@ module.exports.tests.no_connections = function(test, common) {
 
     res.status = function( code ){
       return { json: function( body ){
-        t.equal( code, 502, 'default status' );
+        t.equal( code, 502, 'Bad Gateway' );
         t.deepEqual( body, res.body, 'body set' );
         t.end();
       }};
@@ -160,7 +160,7 @@ module.exports.tests.connection_fault = function(test, common) {
 
     res.status = function( code ){
       return { json: function( body ){
-        t.equal( code, 502, 'default status' );
+        t.equal( code, 502, 'Bad Gateway' );
         t.deepEqual( body, res.body, 'body set' );
         t.end();
       }};
@@ -178,7 +178,25 @@ module.exports.tests.serialization = function(test, common) {
 
     res.status = function( code ){
       return { json: function( body ){
-        t.equal( code, 500, 'default status' );
+        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();
       }};