From 0164030e56b1db8117a1a0bdff91987321b2cd1a Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 24 Jan 2018 09:41:32 -0330 Subject: [PATCH 1/3] Handle errors when getting and setting to localStore. --- app/scripts/background.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index 280c28d70..88600bf1e 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -57,7 +57,13 @@ async function loadStateFromPersistence () { // fetch from extension store and merge in data if (localStore.isSupported) { - const localData = await localStore.get() + let localData + try { + localData = await localStore.get() + } catch (err) { + log.error('error fetching state from local store:', err) + } + // TODO: handle possible exceptions (https://developer.chrome.com/apps/runtime#property-lastError) versionedData = Object.keys(localData).length > 0 ? localData : versionedData } @@ -113,7 +119,11 @@ function setupController (initState) { function syncDataWithExtension(state) { if (localStore.isSupported) { - localStore.set(state) // TODO: handle possible exceptions (https://developer.chrome.com/apps/runtime#property-lastError) + try { + localStore.set(state) + } catch (err) { + log.error('error setting state in local store:', err) + } } return state } From b7ae77f57a0e2bc68e9548364baa120805a1420c Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 24 Jan 2018 09:43:20 -0330 Subject: [PATCH 2/3] Check that extension.storage exists before attempting to call methods on it. --- app/scripts/lib/extension-store.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/scripts/lib/extension-store.js b/app/scripts/lib/extension-store.js index 67ee71f16..4a970321c 100644 --- a/app/scripts/lib/extension-store.js +++ b/app/scripts/lib/extension-store.js @@ -15,12 +15,12 @@ const handleDisabledSyncAndResolve = (resolve, toResolve) => { module.exports = class ExtensionStore { constructor() { - this.isSupported = !!(extension.storage.sync) + this.isSupported = !!(extension.storage && extension.storage.sync) this.isEnabled = true // TODO: get value from user settings } async fetch() { return new Promise((resolve) => { - extension.storage.sync.get(KEYS_TO_SYNC, (data) => { + extension.storage && extension.storage.sync.get(KEYS_TO_SYNC, (data) => { handleDisabledSyncAndResolve(resolve, data) }) }) @@ -31,7 +31,7 @@ module.exports = class ExtensionStore { return result }, {}) return new Promise((resolve) => { - extension.storage.sync.set(dataToSync, () => { + extension.storage && extension.storage.sync.set(dataToSync, () => { handleDisabledSyncAndResolve(resolve) }) }) From 598390e83ec68feaef256c0cfa3a239c6595475a Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 24 Jan 2018 09:44:00 -0330 Subject: [PATCH 3/3] Finish tests for extension-store fetch and sync. --- test/unit/extension-store-test.js | 50 +++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/test/unit/extension-store-test.js b/test/unit/extension-store-test.js index e3b5713fb..e32f37d3c 100644 --- a/test/unit/extension-store-test.js +++ b/test/unit/extension-store-test.js @@ -1,5 +1,18 @@ const assert = require('assert') +const sinon = require('sinon') +const KEYS_TO_SYNC = ['KeyringController', 'PreferencesController'] +const mockSyncGetResult = 123 +const syncGetStub = sinon.stub().callsFake((str, cb) => cb(mockSyncGetResult)) +const syncSetStub = sinon.stub().callsFake((str, cb) => cb()) + +window.storage = { + sync: { + get: syncGetStub, + set: syncSetStub, + }, +} +window.runtime = {} const ExtensionStore = require('../../app/scripts/lib/extension-store') describe('Extension Store', function () { @@ -11,19 +24,44 @@ describe('Extension Store', function () { describe('#fetch', function () { it('should return a promise', function () { - + const extensionStoreFetchResult = extensionStore.fetch() + assert.ok(Promise.resolve(extensionStoreFetchResult) === extensionStoreFetchResult) }) - it('after promise resolution, should have loaded the proper data from the extension', function () { - + it('after promise resolution, should have loaded the proper data from the extension', function (done) { + extensionStore.fetch() + .then((result) => { + assert.deepEqual(syncGetStub.getCall(0).args[0], KEYS_TO_SYNC.slice(0)) + assert.equal(result, mockSyncGetResult) + done() + }) }) }) describe('#sync', function () { it('should return a promise', function () { - + const extensionStoreSyncResult = extensionStore.sync() + assert.ok(Promise.resolve(extensionStoreSyncResult) === extensionStoreSyncResult) }) - it('after promise resolution, should have synced the proper data from the extension', function () { - + it('after promise resolution, should have synced the proper data from the extension', function (done) { + const mockState = { + data: { + KeyringController: 5, + PreferencesController: 6, + someOtherData: 7 + }, + someOtherProp: { + evenMoreData: 8, + }, + } + const expectedDataToSync = { + KeyringController: 5, + PreferencesController: 6, + } + extensionStore.sync(mockState) + .then(() => { + assert.deepEqual(syncSetStub.getCall(0).args[0], expectedDataToSync) + done() + }) }) }) })