Merge pull request #427 from hypothesis/dont-throw-on-invalid-json

Don't throw on invalid JSON

src/annotator/config/index.js

@@ -36,14 +36,7 @@ function configFrom(window_) {
     return config;
   }
 
-  // Parse config from `<script class="js-hypothesis-config">` tags
-  try {
-    Object.assign(config, sharedSettings.jsonConfigsFrom(window_.document));
-  } catch (err) {
-    console.warn('Could not parse settings from js-hypothesis-config tags',
-      err);
-  }
-
+  Object.assign(config, sharedSettings.jsonConfigsFrom(window_.document));
   Object.assign(config, settings.configFuncSettingsFrom(window_));
 
   // Convert legacy keys/values in config to corresponding current

src/annotator/config/test/index-test.js

@@ -10,7 +10,6 @@ var configFrom = proxyquire('../index', util.noCallThru({
   './settings': fakeSettings,
   '../../shared/settings': fakeSharedSettings,
 }));
-var sandbox = sinon.sandbox.create();
 
 function fakeWindow() {
   return {
@@ -20,10 +19,6 @@ function fakeWindow() {
 }
 
 describe('annotator.config', function() {
-  beforeEach('stub console.warn()', function() {
-    sandbox.stub(console, 'warn');
-  });
-
   beforeEach('reset fakeSharedSettings', function() {
     fakeSharedSettings.jsonConfigsFrom = sinon.stub().returns({});
   });
@@ -35,10 +30,6 @@ describe('annotator.config', function() {
     fakeSettings.configFuncSettingsFrom = sinon.stub().returns({});
   });
 
-  afterEach('reset the sandbox', function() {
-    sandbox.restore();
-  });
-
   it('gets the config.app setting', function() {
     var window_ = fakeWindow();
 
@@ -89,22 +80,6 @@ describe('annotator.config', function() {
     });
   });
 
-  context('when jsonConfigsFrom() throws an error', function() {
-    beforeEach(function() {
-      fakeSharedSettings.jsonConfigsFrom.throws();
-    });
-
-    it('catches the error', function() {
-      configFrom(fakeWindow());
-    });
-
-    it('logs a warning', function() {
-      configFrom(fakeWindow());
-
-      assert.called(console.warn);
-    });
-  });
-
   it('gets config settings from window.hypothesisConfig()', function() {
     var window_ = fakeWindow();
 

src/shared/settings.js

@@ -28,13 +28,21 @@ function assign(dest, src) {
  * @param {Document|Element} document - The root element to search.
  */
 function jsonConfigsFrom(document) {
+  var config = {};
   var settingsElements =
     document.querySelectorAll('script.js-hypothesis-config');
 
-  var config = {};
   for (var i=0; i < settingsElements.length; i++) {
-    assign(config, JSON.parse(settingsElements[i].textContent));
+    var settings;
+    try {
+      settings = JSON.parse(settingsElements[i].textContent);
+    } catch (err) {
+      console.warn('Could not parse settings from js-hypothesis-config tags', err);
+      settings = {};
+    }
+    assign(config, settings);
   }
+
   return config;
 }
 

src/shared/test/settings-test.js

@@ -2,7 +2,14 @@
 
 var settings = require('../settings');
 
+var sandbox = sinon.sandbox.create();
+
 describe('settings', function () {
+
+  afterEach('reset the sandbox', function() {
+    sandbox.restore();
+  });
+
   describe('#jsonConfigsFrom', function() {
     var jsonConfigsFrom = settings.jsonConfigsFrom;
 
@@ -64,15 +71,28 @@ describe('settings', function () {
     });
 
     context("when there's a JSON script containing invalid JSON", function() {
+      beforeEach('stub console.warn()', function() {
+        sandbox.stub(console, 'warn');
+      });
+
       beforeEach('add a JSON script containing invalid JSON', function() {
         appendJSHypothesisConfig(document, 'this is not valid json');
       });
 
-      it('throws a SyntaxError', function() {
-        assert.throws(
-          function() { jsonConfigsFrom(document); },
-          SyntaxError
-        );
+      it('logs a warning', function() {
+        jsonConfigsFrom(document);
+
+        assert.called(console.warn);
+      });
+
+      it('returns {}', function() {
+        assert.deepEqual(jsonConfigsFrom(document), {});
+      });
+
+      it('still returns settings from other JSON scripts', function() {
+        appendJSHypothesisConfig(document, '{"foo": "FOO", "bar": "BAR"}');
+
+        assert.deepEqual(jsonConfigsFrom(document), {foo: 'FOO', bar: 'BAR'});
       });
     });