From c72822b8055725bb81dfc975731c33a822d3c9d9 Mon Sep 17 00:00:00 2001
From: missinglink <insomnia@rcpt.at>
Date: Mon, 25 Sep 2017 17:40:47 +0200
Subject: [PATCH] feat(coarse_reverse): add try/catch block around
 synthesizeDoc code

---
 controller/coarse_reverse.js           | 71 +++++++++++++++-----------
 test/unit/controller/coarse_reverse.js | 54 ++++++++++++++++++++
 2 files changed, 96 insertions(+), 29 deletions(-)

diff --git a/controller/coarse_reverse.js b/controller/coarse_reverse.js
index e49dc8c9..1ed47c58 100644
--- a/controller/coarse_reverse.js
+++ b/controller/coarse_reverse.js
@@ -62,40 +62,50 @@ function synthesizeDoc(results) {
   const most_granular_layer = getMostGranularLayerOfResult(_.keys(results));
   const id = results[most_granular_layer][0].id;
 
-  const doc = new Document('whosonfirst', most_granular_layer, id.toString());
-  doc.setName('default', results[most_granular_layer][0].name);
+  try {
+    const doc = new Document('whosonfirst', most_granular_layer, id.toString());
+    doc.setName('default', results[most_granular_layer][0].name);
 
-  // assign the administrative hierarchy
-  _.keys(results).forEach((layer) => {
-    doc.addParent(layer, results[layer][0].name, results[layer][0].id.toString(), results[layer][0].abbr);
-  });
+    // assign the administrative hierarchy
+    _.keys(results).forEach((layer) => {
+      doc.addParent(layer, results[layer][0].name, results[layer][0].id.toString(), results[layer][0].abbr);
+    });
 
-  // set centroid if available
-  if (_.has(results[most_granular_layer][0], 'centroid')) {
-    doc.setCentroid( results[most_granular_layer][0].centroid );
-  }
+    // set centroid if available
+    if (_.has(results[most_granular_layer][0], 'centroid')) {
+      doc.setCentroid( results[most_granular_layer][0].centroid );
+    }
 
-  // set bounding box if available
-  if (_.has(results[most_granular_layer][0], 'bounding_box')) {
-    const parsed_bounding_box = results[most_granular_layer][0].bounding_box.split(',').map(parseFloat);
-    doc.setBoundingBox({
-      upperLeft: {
-        lat: parsed_bounding_box[3],
-        lon: parsed_bounding_box[0]
-      },
-      lowerRight: {
-        lat: parsed_bounding_box[1],
-        lon: parsed_bounding_box[2]
-      }
-    });
+    // set bounding box if available
+    if (_.has(results[most_granular_layer][0], 'bounding_box')) {
+      const parsed_bounding_box = results[most_granular_layer][0].bounding_box.split(',').map(parseFloat);
+      doc.setBoundingBox({
+        upperLeft: {
+          lat: parsed_bounding_box[3],
+          lon: parsed_bounding_box[0]
+        },
+        lowerRight: {
+          lat: parsed_bounding_box[1],
+          lon: parsed_bounding_box[2]
+        }
+      });
 
-  }
+    }
+
+    const esDoc = doc.toESDocument();
+    esDoc.data._id = esDoc._id;
+    esDoc.data._type = esDoc._type;
+    return esDoc.data;
 
-  const esDoc = doc.toESDocument();
-  esDoc.data._id = esDoc._id;
-  esDoc.data._type = esDoc._type;
-  return esDoc.data;
+  } catch( e ) {
 
+    // an error occurred when generating a new Document
+    logger.info(`[controller:coarse_reverse][error]`);
+    logger.error(e);
+    logger.error(results);
+
+    return null;
+  }
 }
 
 function setup(service, should_execute) {
@@ -141,7 +151,10 @@ function setup(service, should_execute) {
 
       // if there's a result at the requested layer(s), synthesize a doc from results
       if (hasResultsAtRequestedLayers(applicable_results, effective_layers)) {
-        res.data.push(synthesizeDoc(applicable_results));
+        const doc = synthesizeDoc(applicable_results);
+        if (doc){
+          res.data.push(doc);
+        }
       }
       debugLog.stopTimer(req, initialTime);
       return next();
diff --git a/test/unit/controller/coarse_reverse.js b/test/unit/controller/coarse_reverse.js
index fa42ef0a..cc7c5443 100644
--- a/test/unit/controller/coarse_reverse.js
+++ b/test/unit/controller/coarse_reverse.js
@@ -860,6 +860,60 @@ module.exports.tests.failure_conditions = (test, common) => {
 
   });
 
+  test('service returns 0 length name', (t) => {
+    t.plan(5);
+
+    const service = (req, callback) => {
+      t.deepEquals(req, { clean: { layers: ['neighbourhood'] } } );
+
+      const results = {
+        neighbourhood: [
+          { id: 20, name: '' }
+        ]
+      };
+
+      callback(undefined, results);
+    };
+
+    const logger = require('pelias-mock-logger')();
+
+    const controller = proxyquire('../../../controller/coarse_reverse', {
+      'pelias-logger': logger
+    })(service, _.constant(true));
+
+    const req = {
+      clean: {
+        layers: ['neighbourhood']
+      }
+    };
+
+    const res = { };
+
+    // verify that next was called
+    const next = () => {
+      t.pass('next() was called');
+    };
+
+    controller(req, res, next);
+
+    const expected = {
+      meta: {},
+      data: []
+    };
+
+    t.deepEquals(res, expected);
+    t.deepEquals(logger.getMessages('info'), [
+      '[controller:coarse_reverse][queryType:pip][result_count:1]',
+      '[controller:coarse_reverse][error]'
+    ]);
+    t.deepEquals(logger.getMessages('error'), [
+      '{ [PeliasModelError: invalid document type, expecting: truthy, ' +
+      'got: ]\n  name: \'PeliasModelError\',\n  message: \'invalid document type, expecting: truthy, got: \' }',
+      '{ neighbourhood: [ { id: 20, name: \'\' } ] }'
+    ]);
+    t.end();
+
+  });
 };
 
 module.exports.all = (tape, common) => {