Merge pull request #2122 from plotly/geo-robust

harden locationToFeature against non-string country names

Author: alexcjohnson

src/lib/geo_location_utils.js

@@ -23,6 +23,8 @@ var locationmodeToIdFinder = {
 };
 
 exports.locationToFeature = function(locationmode, location, features) {
+    if(!location || typeof location !== 'string') return false;
+
     var locationId = getLocationId(locationmode, location);
 
     if(locationId) {

test/jasmine/tests/choropleth_test.js

@@ -160,3 +160,29 @@ describe('Test choropleth hover:', function() {
         .then(done);
     });
 });
+
+describe('choropleth bad data', function() {
+    var gd;
+
+    beforeEach(function() {
+        gd = createGraphDiv();
+    });
+
+    afterEach(destroyGraphDiv);
+
+    it('should not throw an error with bad locations', function(done) {
+        spyOn(Lib, 'warn');
+        Plotly.newPlot(gd, [{
+            locations: ['canada', 0, null, '', 'utopia'],
+            z: [1, 2, 3, 4, 5],
+            locationmode: 'country names',
+            type: 'choropleth'
+        }])
+        .then(function() {
+            // only utopia warns - others are silently ignored
+            expect(Lib.warn).toHaveBeenCalledTimes(1);
+        })
+        .catch(fail)
+        .then(done);
+    });
+});

test/jasmine/tests/scattergeo_test.js

@@ -13,6 +13,7 @@ var mouseEvent = require('../assets/mouse_event');
 var customAssertions = require('../assets/custom_assertions');
 var assertHoverLabelStyle = customAssertions.assertHoverLabelStyle;
 var assertHoverLabelContent = customAssertions.assertHoverLabelContent;
+var fail = require('../assets/fail_test');
 
 describe('Test scattergeo defaults', function() {
     var traceIn,
@@ -246,6 +247,7 @@ describe('Test scattergeo hover', function() {
             lat: [10, 20, 30],
             text: ['A', 'B', 'C']
         }])
+        .catch(fail)
         .then(done);
     });
 
@@ -280,27 +282,31 @@ describe('Test scattergeo hover', function() {
         Plotly.restyle(gd, 'hoverinfo', 'lon+lat+text+name').then(function() {
             check([381, 221], ['(10°, 10°)\nA', 'trace 0']);
         })
+        .catch(fail)
         .then(done);
     });
 
     it('should generate hover label info (\'text\' single value case)', function(done) {
         Plotly.restyle(gd, 'text', 'text').then(function() {
             check([381, 221], ['(10°, 10°)\ntext', null]);
         })
+        .catch(fail)
         .then(done);
     });
 
     it('should generate hover label info (\'hovertext\' single value case)', function(done) {
         Plotly.restyle(gd, 'hovertext', 'hovertext').then(function() {
             check([381, 221], ['(10°, 10°)\nhovertext', null]);
         })
+        .catch(fail)
         .then(done);
     });
 
     it('should generate hover label info (\'hovertext\' array case)', function(done) {
         Plotly.restyle(gd, 'hovertext', ['Apple', 'Banana', 'Orange']).then(function() {
             check([381, 221], ['(10°, 10°)\nApple', null]);
         })
+        .catch(fail)
         .then(done);
     });
 
@@ -318,13 +324,40 @@ describe('Test scattergeo hover', function() {
                 fontFamily: 'Arial'
             });
         })
+        .catch(fail)
         .then(done);
     });
 
     it('should generate hover label with arrayOk \'hoverinfo\' settings', function(done) {
         Plotly.restyle(gd, 'hoverinfo', [['lon', null, 'lat+name']]).then(function() {
             check([381, 221], ['lon: 10°', null]);
         })
+        .catch(fail)
+        .then(done);
+    });
+});
+
+describe('scattergeo bad data', function() {
+    var gd;
+
+    beforeEach(function() {
+        gd = createGraphDiv();
+    });
+
+    afterEach(destroyGraphDiv);
+
+    it('should not throw an error with bad locations', function(done) {
+        spyOn(Lib, 'warn');
+        Plotly.newPlot(gd, [{
+            locations: ['canada', 0, null, '', 'utopia'],
+            locationmode: 'country names',
+            type: 'scattergeo'
+        }])
+        .then(function() {
+            // only utopia warns - others are silently ignored
+            expect(Lib.warn).toHaveBeenCalledTimes(1);
+        })
+        .catch(fail)
         .then(done);
     });
 });