From a2e96987fc4679b7b74a433a212b6086439e62b0 Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Thu, 10 Mar 2016 10:50:29 +0200 Subject: [PATCH 1/7] Remove expensive JSON.stringify from api debug logging calls --- controller/place.js | 4 ++-- controller/search.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/controller/place.js b/controller/place.js index 29acfe8b..a4996a11 100644 --- a/controller/place.js +++ b/controller/place.js @@ -22,7 +22,7 @@ function setup( backend ){ }; }); - logger.debug( '[ES req]', JSON.stringify(query) ); + logger.debug( '[ES req]', query ); service.mget( backend, query, function( err, docs ) { console.log('err:' + err); @@ -35,7 +35,7 @@ function setup( backend ){ else { res.data = docs; } - logger.debug('[ES response]', JSON.stringify(docs)); + logger.debug('[ES response]', docs); next(); }); diff --git a/controller/search.js b/controller/search.js index a564e7b4..b7b226e5 100644 --- a/controller/search.js +++ b/controller/search.js @@ -29,7 +29,7 @@ function setup( backend, query ){ cmd.type = req.clean.layers; } - logger.debug( '[ES req]', JSON.stringify(cmd) ); + logger.debug( '[ES req]', cmd ); // query backend service.search( backend, cmd, function( err, docs, meta ){ @@ -43,7 +43,7 @@ function setup( backend, query ){ res.data = docs; res.meta = meta; } - logger.debug('[ES response]', JSON.stringify(docs)); + logger.debug('[ES response]', docs); next(); }); From ab19a12e1c63ee39ddd2580565f6b18747e4a9cd Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 4 Apr 2016 11:45:53 -0400 Subject: [PATCH 2/7] Use multi_match query for admin fields --- query/search.js | 18 ++++-- test/unit/fixture/search_full_address.js | 65 ++++--------------- test/unit/fixture/search_partial_address.js | 71 +++++---------------- test/unit/fixture/search_regions_address.js | 65 ++++--------------- 4 files changed, 50 insertions(+), 169 deletions(-) diff --git a/query/search.js b/query/search.js index 77fcb3f5..5689d9ec 100644 --- a/query/search.js +++ b/query/search.js @@ -3,6 +3,13 @@ var peliasQuery = require('pelias-query'), textParser = require('./text_parser'), check = require('check-types'), geolib = require('geolib'); +var placeTypes = require('../helper/placeTypes'); + +// region_a is also an admin field. addressit tries to detect +// region_a, in which case we use a match query specifically for it. +// but address it doesn't know about all of them so it helps to search +// against this with the other admin parts as a fallback +var adminFields = placeTypes.concat(['region_a']); //------------------------------ // general-purpose search query @@ -25,15 +32,12 @@ query.score( peliasQuery.view.address('street') ); query.score( peliasQuery.view.address('postcode') ); // admin components -query.score( peliasQuery.view.admin('country') ); +// country_a and region_a are left as matches here because the text-analyzer +// can sometimes detect them, in which case a query more specific than a +// multi_match is appropriate. query.score( peliasQuery.view.admin('country_a') ); -query.score( peliasQuery.view.admin('region') ); query.score( peliasQuery.view.admin('region_a') ); -query.score( peliasQuery.view.admin('county') ); -query.score( peliasQuery.view.admin('borough') ); -query.score( peliasQuery.view.admin('localadmin') ); -query.score( peliasQuery.view.admin('locality') ); -query.score( peliasQuery.view.admin('neighbourhood') ); +query.score( peliasQuery.view.admin_multi_match(adminFields), 'peliasAdmin' ); // non-scoring hard filters query.filter( peliasQuery.view.boundary_circle ); diff --git a/test/unit/fixture/search_full_address.js b/test/unit/fixture/search_full_address.js index dfd64e34..c7ba7abc 100644 --- a/test/unit/fixture/search_full_address.js +++ b/test/unit/fixture/search_full_address.js @@ -99,14 +99,6 @@ module.exports = { 'analyzer': vs['address:postcode:analyzer'] } } - }, { - 'match': { - 'parent.country': { - 'query': 'new york', - 'boost': vs['admin:country:boost'], - 'analyzer': vs['admin:country:analyzer'] - } - } }, { 'match': { 'parent.country_a': { @@ -115,14 +107,6 @@ module.exports = { 'analyzer': vs['admin:country_a:analyzer'] } } - }, { - 'match': { - 'parent.region': { - 'query': 'new york', - 'boost': vs['admin:region:boost'], - 'analyzer': vs['admin:region:analyzer'] - } - } }, { 'match': { 'parent.region_a': { @@ -132,44 +116,19 @@ module.exports = { } } }, { - 'match': { - 'parent.county': { - 'query': 'new york', - 'boost': vs['admin:county:boost'], - 'analyzer': vs['admin:county:analyzer'] - } - } - }, { - 'match': { - 'parent.borough': { - 'query': 'new york', - 'boost': vs['admin:borough:boost'], - 'analyzer': vs['admin:borough:analyzer'] - } - } - }, { - 'match': { - 'parent.localadmin': { - 'query': 'new york', - 'boost': vs['admin:localadmin:boost'], - 'analyzer': vs['admin:localadmin:analyzer'] - } - } - }, { - 'match': { - 'parent.locality': { - 'query': 'new york', - 'boost': vs['admin:locality:boost'], - 'analyzer': vs['admin:locality:analyzer'] - } - } - }, { - 'match': { - 'parent.neighbourhood': { + 'multi_match': { + 'fields': [ + 'parent.country^4', + 'parent.region^3', + 'parent.county^2', + 'parent.localadmin^1', + 'parent.locality^1', + 'parent.borough^1', + 'parent.neighbourhood^1', + 'parent.region_a^3' + ], 'query': 'new york', - 'boost': vs['admin:neighbourhood:boost'], - 'analyzer': vs['admin:neighbourhood:analyzer'] - } + 'analyzer': 'peliasAdmin' } }] } diff --git a/test/unit/fixture/search_partial_address.js b/test/unit/fixture/search_partial_address.js index 746899b7..98fa8167 100644 --- a/test/unit/fixture/search_partial_address.js +++ b/test/unit/fixture/search_partial_address.js @@ -75,69 +75,28 @@ module.exports = { 'weight': 2 }] } - },{ - 'match': { - 'parent.country': { - 'query': 'new york', - 'boost': vs['admin:country:boost'], - 'analyzer': vs['admin:country:analyzer'] - } - } - }, { - 'match': { - 'parent.region': { - 'query': 'new york', - 'boost': vs['admin:region:boost'], - 'analyzer': vs['admin:region:analyzer'] - } - } }, { 'match': { 'parent.region_a': { - 'query': 'new york', - 'boost': vs['admin:region_a:boost'], - 'analyzer': vs['admin:region_a:analyzer'] - } - } - }, { - 'match': { - 'parent.county': { - 'query': 'new york', - 'boost': vs['admin:county:boost'], - 'analyzer': vs['admin:county:analyzer'] + 'analyzer': 'peliasAdmin', + 'boost': 3, + 'query': 'new york' } } }, { - 'match': { - 'parent.borough': { + 'multi_match': { + 'fields': [ + 'parent.country^4', + 'parent.region^3', + 'parent.county^2', + 'parent.localadmin^1', + 'parent.locality^1', + 'parent.borough^1', + 'parent.neighbourhood^1', + 'parent.region_a^3' + ], 'query': 'new york', - 'boost': vs['admin:borough:boost'], - 'analyzer': vs['admin:borough:analyzer'] - } - } - }, { - 'match': { - 'parent.localadmin': { - 'query': 'new york', - 'boost': vs['admin:localadmin:boost'], - 'analyzer': vs['admin:localadmin:analyzer'] - } - } - }, { - 'match': { - 'parent.locality': { - 'query': 'new york', - 'boost': vs['admin:locality:boost'], - 'analyzer': vs['admin:locality:analyzer'] - } - } - }, { - 'match': { - 'parent.neighbourhood': { - 'query': 'new york', - 'boost': vs['admin:neighbourhood:boost'], - 'analyzer': vs['admin:neighbourhood:analyzer'] - } + 'analyzer': 'peliasAdmin' } }] } diff --git a/test/unit/fixture/search_regions_address.js b/test/unit/fixture/search_regions_address.js index 0a8b199d..264d5eea 100644 --- a/test/unit/fixture/search_regions_address.js +++ b/test/unit/fixture/search_regions_address.js @@ -91,22 +91,6 @@ module.exports = { 'analyzer': vs['address:street:analyzer'] } } - }, { - 'match': { - 'parent.country': { - 'query': 'manhattan', - 'boost': vs['admin:country:boost'], - 'analyzer': vs['admin:country:analyzer'] - } - } - }, { - 'match': { - 'parent.region': { - 'query': 'manhattan', - 'boost': vs['admin:region:boost'], - 'analyzer': vs['admin:region:analyzer'] - } - } }, { 'match': { 'parent.region_a': { @@ -116,44 +100,19 @@ module.exports = { } } }, { - 'match': { - 'parent.county': { - 'query': 'manhattan', - 'boost': vs['admin:county:boost'], - 'analyzer': vs['admin:county:analyzer'] - } - } - }, { - 'match': { - 'parent.borough': { - 'query': 'manhattan', - 'boost': vs['admin:borough:boost'], - 'analyzer': vs['admin:borough:analyzer'] - } - } - }, { - 'match': { - 'parent.localadmin': { - 'query': 'manhattan', - 'boost': vs['admin:localadmin:boost'], - 'analyzer': vs['admin:localadmin:analyzer'] - } - } - }, { - 'match': { - 'parent.locality': { - 'query': 'manhattan', - 'boost': vs['admin:locality:boost'], - 'analyzer': vs['admin:locality:analyzer'] - } - } - }, { - 'match': { - 'parent.neighbourhood': { + 'multi_match': { + 'fields': [ + 'parent.country^4', + 'parent.region^3', + 'parent.county^2', + 'parent.localadmin^1', + 'parent.locality^1', + 'parent.borough^1', + 'parent.neighbourhood^1', + 'parent.region_a^3' + ], 'query': 'manhattan', - 'boost': vs['admin:neighbourhood:boost'], - 'analyzer': vs['admin:neighbourhood:analyzer'] - } + 'analyzer': 'peliasAdmin' } }] } From 373a08e79b6a18beef996f5a68e6d5081deb17c6 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 6 Apr 2016 15:44:26 -0400 Subject: [PATCH 3/7] Set all admin boosts to 1 They seem to interfere with the new multi_match query. More tweaking in the future is definitely warranted. --- query/search_defaults.js | 10 +++++----- test/unit/fixture/search_full_address.js | 8 ++++---- test/unit/fixture/search_partial_address.js | 10 +++++----- test/unit/fixture/search_regions_address.js | 8 ++++---- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/query/search_defaults.js b/query/search_defaults.js index 281d25ae..0ffdd79f 100644 --- a/query/search_defaults.js +++ b/query/search_defaults.js @@ -52,23 +52,23 @@ module.exports = _.merge({}, peliasQuery.defaults, { 'admin:country_a:analyzer': 'standard', 'admin:country_a:field': 'parent.country_a', - 'admin:country_a:boost': 5, + 'admin:country_a:boost': 1, 'admin:country:analyzer': 'peliasAdmin', 'admin:country:field': 'parent.country', - 'admin:country:boost': 4, + 'admin:country:boost': 1, 'admin:region:analyzer': 'peliasAdmin', 'admin:region:field': 'parent.region', - 'admin:region:boost': 3, + 'admin:region:boost': 1, 'admin:region_a:analyzer': 'peliasAdmin', 'admin:region_a:field': 'parent.region_a', - 'admin:region_a:boost': 3, + 'admin:region_a:boost': 1, 'admin:county:analyzer': 'peliasAdmin', 'admin:county:field': 'parent.county', - 'admin:county:boost': 2, + 'admin:county:boost': 1, 'admin:localadmin:analyzer': 'peliasAdmin', 'admin:localadmin:field': 'parent.localadmin', diff --git a/test/unit/fixture/search_full_address.js b/test/unit/fixture/search_full_address.js index c7ba7abc..ea85f716 100644 --- a/test/unit/fixture/search_full_address.js +++ b/test/unit/fixture/search_full_address.js @@ -118,14 +118,14 @@ module.exports = { }, { 'multi_match': { 'fields': [ - 'parent.country^4', - 'parent.region^3', - 'parent.county^2', + 'parent.country^1', + 'parent.region^1', + 'parent.county^1', 'parent.localadmin^1', 'parent.locality^1', 'parent.borough^1', 'parent.neighbourhood^1', - 'parent.region_a^3' + 'parent.region_a^1' ], 'query': 'new york', 'analyzer': 'peliasAdmin' diff --git a/test/unit/fixture/search_partial_address.js b/test/unit/fixture/search_partial_address.js index 98fa8167..a6cf47e5 100644 --- a/test/unit/fixture/search_partial_address.js +++ b/test/unit/fixture/search_partial_address.js @@ -79,21 +79,21 @@ module.exports = { 'match': { 'parent.region_a': { 'analyzer': 'peliasAdmin', - 'boost': 3, + 'boost': 1, 'query': 'new york' } } }, { 'multi_match': { 'fields': [ - 'parent.country^4', - 'parent.region^3', - 'parent.county^2', + 'parent.country^1', + 'parent.region^1', + 'parent.county^1', 'parent.localadmin^1', 'parent.locality^1', 'parent.borough^1', 'parent.neighbourhood^1', - 'parent.region_a^3' + 'parent.region_a^1' ], 'query': 'new york', 'analyzer': 'peliasAdmin' diff --git a/test/unit/fixture/search_regions_address.js b/test/unit/fixture/search_regions_address.js index 264d5eea..195358c7 100644 --- a/test/unit/fixture/search_regions_address.js +++ b/test/unit/fixture/search_regions_address.js @@ -102,14 +102,14 @@ module.exports = { }, { 'multi_match': { 'fields': [ - 'parent.country^4', - 'parent.region^3', - 'parent.county^2', + 'parent.country^1', + 'parent.region^1', + 'parent.county^1', 'parent.localadmin^1', 'parent.locality^1', 'parent.borough^1', 'parent.neighbourhood^1', - 'parent.region_a^3' + 'parent.region_a^1' ], 'query': 'manhattan', 'analyzer': 'peliasAdmin' From b15cfb379575deb07b4b6b3ef94f1e430db869c1 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Tue, 3 May 2016 18:35:25 +0200 Subject: [PATCH 4/7] refactor elasticsearch error detection, improve test coverage --- middleware/sendJSON.js | 26 ++-- test/ciao_test_data.js | 2 +- test/unit/middleware/sendJSON.js | 200 +++++++++++++++++++++++++++++++ test/unit/run.js | 1 + 4 files changed, 216 insertions(+), 13 deletions(-) create mode 100644 test/unit/middleware/sendJSON.js diff --git a/middleware/sendJSON.js b/middleware/sendJSON.js index 94353c6c..6a33cfe7 100644 --- a/middleware/sendJSON.js +++ b/middleware/sendJSON.js @@ -1,4 +1,5 @@ -var check = require('check-types'); +var check = require('check-types'), + es = require('elasticsearch'); function sendJSONResponse(req, res, next) { @@ -21,17 +22,18 @@ function sendJSONResponse(req, res, next) { 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 = 408; } + else if( err instanceof es.errors.NoConnections ){ statusCode = 502; } + else if( err instanceof es.errors.ConnectionFault ){ statusCode = 502; } + else { statusCode = 500; } } }); } diff --git a/test/ciao_test_data.js b/test/ciao_test_data.js index 70b329f6..2ccee53c 100644 --- a/test/ciao_test_data.js +++ b/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'); }); diff --git a/test/unit/middleware/sendJSON.js b/test/unit/middleware/sendJSON.js new file mode 100644 index 00000000..d4166bc8 --- /dev/null +++ b/test/unit/middleware/sendJSON.js @@ -0,0 +1,200 @@ +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, 'default status' ); + 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, 'default status' ); + 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, 'default status' ); + 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, 'default status' ); + 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, 'default status' ); + 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, 'default status' ); + 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, 'default status' ); + 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, 'default status' ); + 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, 'default status' ); + 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); + } +}; diff --git a/test/unit/run.js b/test/unit/run.js index 1a6f7a90..828d99b5 100644 --- a/test/unit/run.js +++ b/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'), From 0caca08787170977bb2d3fc16b1a62100fe0150c Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Wed, 4 May 2016 15:58:26 +0200 Subject: [PATCH 5/7] 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 Date: Wed, 4 May 2016 16:04:32 +0200 Subject: [PATCH 6/7] better comment --- middleware/sendJSON.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/middleware/sendJSON.js b/middleware/sendJSON.js index 8f31681f..be2e56b0 100644 --- a/middleware/sendJSON.js +++ b/middleware/sendJSON.js @@ -3,7 +3,7 @@ var check = require('check-types'), exceptions = require('elasticsearch-exceptions/lib/exceptions/SupportedExceptions'); // create a list of regular expressions to match against. -// note: list created at runtime for performance reasons. +// note: list created when the server starts up; for performance reasons. var exceptionRegexList = exceptions.map( function( exceptionName ){ return new RegExp( '^' + exceptionName ); }); From 63d9329afdbd05a985223b2ec93cbcfd104fe40e Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Wed, 4 May 2016 16:06:53 +0200 Subject: [PATCH 7/7] add additional test case --- test/unit/middleware/sendJSON.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/unit/middleware/sendJSON.js b/test/unit/middleware/sendJSON.js index 0bc21911..d02138ff 100644 --- a/test/unit/middleware/sendJSON.js +++ b/test/unit/middleware/sendJSON.js @@ -206,6 +206,24 @@ module.exports.tests.search_phase_execution_exception = function(test, common) { }); }; +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) {