From 7a2fde439591bf75268b67cc35c29d59d767c388 Mon Sep 17 00:00:00 2001 From: Mickey Reiss Date: Mon, 11 Dec 2017 15:55:15 -0800 Subject: [PATCH] Add safe wrappers around localStorage --- src/__tests__/LDClient-test.js | 53 +++++++++++++++++++++++++++++++--- src/index.js | 14 +++++---- src/messages.js | 3 ++ src/store.js | 27 +++++++++++++++++ 4 files changed, 87 insertions(+), 10 deletions(-) create mode 100644 src/store.js diff --git a/src/__tests__/LDClient-test.js b/src/__tests__/LDClient-test.js index 047d26c3..257bfdce 100644 --- a/src/__tests__/LDClient-test.js +++ b/src/__tests__/LDClient-test.js @@ -110,6 +110,53 @@ describe('LDClient', function() { }); }); + it('should handle localStorage getItem throwing an exception', function(done) { + sandbox.restore(window.localStorage.__proto__, 'getItem') + sandbox.stub(window.localStorage.__proto__, 'getItem').throws() + + var warnSpy = sandbox.spy(console, 'warn'); + + var user = {key: 'user'}; + var client = LDClient.initialize('UNKNOWN_ENVIRONMENT_ID', user, { + bootstrap: 'localstorage' + }); + + client.on('ready', function() { + expect(warnSpy.calledWith(messages.localStorageUnavailable())).to.be.true; + done(); + }); + + requests[0].respond( + 200, + { 'Content-Type': 'application/json' }, + '[{"key": "known", "kind": "custom"}]' + ); + + }); + + it('should handle localStorage setItem throwing an exception', function(done) { + sandbox.restore(window.localStorage.__proto__, 'setItem') + sandbox.stub(window.localStorage.__proto__, 'setItem').throws() + + var user = {key: 'user'}; + var client = LDClient.initialize('UNKNOWN_ENVIRONMENT_ID', user, { + bootstrap: 'localstorage' + }); + + var warnSpy = sandbox.spy(console, 'warn'); + + requests[0].respond( + 200, + { 'Content-Type': 'application/json' }, + '[{"key": "known", "kind": "custom"}]' + ); + + client.on('ready', function() { + expect(warnSpy.calledWith(messages.localStorageUnavailable())).to.be.true; + done(); + }); + }); + it('should not update cached settings if there was an error fetching flags', function(done) { var user = {key: 'user'}; var json = '{"enable-foo": true}'; @@ -140,7 +187,7 @@ describe('LDClient', function() { bootstrap: {} // so the client doesn't request settings }); - var warnSpy = sinon.spy(console, 'warn'); + var warnSpy = sandbox.spy(console, 'warn'); requests[0].respond( 200, @@ -151,7 +198,6 @@ describe('LDClient', function() { client.on('ready', function() { client.track('known'); expect(warnSpy.calledWith('Custom event key does not exist')).to.be.false; - warnSpy.restore(); done(); }); }); @@ -184,7 +230,7 @@ describe('LDClient', function() { bootstrap: {} // so the client doesn't request settings }); - var warnSpy = sinon.spy(console, 'warn'); + var warnSpy = sandbox.spy(console, 'warn'); requests[0].respond( 200, @@ -195,7 +241,6 @@ describe('LDClient', function() { client.on('ready', function() { client.track('unknown'); expect(warnSpy.calledWith(messages.unknownCustomEventKey('unknown'))).to.be.true; - warnSpy.restore(); done(); }); }); diff --git a/src/index.js b/src/index.js index b684acc3..f0641d3e 100644 --- a/src/index.js +++ b/src/index.js @@ -6,6 +6,7 @@ var Requestor = require('./Requestor'); var Identity = require('./Identity'); var utils = require('./utils'); var messages = require('./messages'); +var store = require('./store'); var flags = {}; var environment; @@ -174,7 +175,7 @@ function updateSettings(settings) { flags = settings; if (useLocalStorage) { - localStorage.setItem(lsKey(environment, ident.getUser()), JSON.stringify(flags)); + store.set(lsKey(environment, ident.getUser()), JSON.stringify(flags)); } if (keys.length > 0) { @@ -258,11 +259,12 @@ function initialize(env, user, options) { } else if (typeof(options.bootstrap) === 'string' && options.bootstrap.toUpperCase() === 'LOCALSTORAGE' && typeof(Storage) !== 'undefined') { useLocalStorage = true; - // check if localstorage data is corrupted, if so clear it + + // check if localStorage data is corrupted, if so clear it try { - flags = JSON.parse(localStorage.getItem(localStorageKey)); + flags = JSON.parse(store.get(localStorageKey)); } catch (error) { - localStorage.setItem(localStorageKey, null); + store.clear(localStorageKey); } if (flags === null) { @@ -272,7 +274,7 @@ function initialize(env, user, options) { emitter.emit(errorEvent) } flags = settings; - settings && localStorage.setItem(localStorageKey, JSON.stringify(flags)); + settings && store.set(localStorageKey, JSON.stringify(flags)); emitter.emit(readyEvent); }); } else { @@ -285,7 +287,7 @@ function initialize(env, user, options) { console.error('Error fetching flag settings: ', err); emitter.emit(errorEvent) } - settings && localStorage.setItem(localStorageKey, JSON.stringify(settings)); + settings && store.set(localStorageKey, JSON.stringify(settings)); }); } } diff --git a/src/messages.js b/src/messages.js index ba82f009..86086f8f 100644 --- a/src/messages.js +++ b/src/messages.js @@ -2,6 +2,9 @@ module.exports ={ invalidKey: function() { return 'Event key must be a string'; }, + localStorageUnavailable: function() { + return 'localStorage is unavailable'; + }, unknownCustomEventKey: function(key) { return 'Custom event "' + key + '" does not exist' } diff --git a/src/store.js b/src/store.js new file mode 100644 index 00000000..fe3262c2 --- /dev/null +++ b/src/store.js @@ -0,0 +1,27 @@ +var messages = require('./messages'); + +function get(key) { + try { + return window.localStorage.getItem(key); + } catch (ex) { + console.warn(messages.localStorageUnavailable()); + } +} + +function set(key, item) { + try { + window.localStorage.setItem(key, item); + } catch (ex) { + console.warn(messages.localStorageUnavailable()); + } +} + +function clear(key) { + set(key, null); +} + +module.exports = { + get: get, + set: set, + clear: clear, +};