Repository: parse-community/Parse-SDK-JS
Pull request #803 (2019-04-24 15:57:42)

LDS: Improve Error Handling
Fixes: https://github.com/parse-community/Parse-SDK-JS/issues/773

Should we log key, values?

Console.log vs warn vs error?

acinader: test failing
codecov[bot]: # [Codecov](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/803?src=pr&el=h1) Report
> Merging [#803](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/803?src=pr&el=desc) into [master](https://codecov.io/gh/parse-community/Parse-SDK-JS/commit/1fe021be9a8c2838bfb1c8dbbb937e355cd58a41?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `66.66%`.

[![Impacted file tree graph](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/803/graphs/tree.svg?width=650&token=8nCENM0QHj&height=150&src=pr)](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/803?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #803      +/-   ##
==========================================
- Coverage   92.03%   92.01%   -0.02%     
==========================================
  Files          54       54              
  Lines        4920     4922       +2     
  Branches     1092     1092              
==========================================
+ Hits         4528     4529       +1     
- Misses        392      393       +1
```


| [Impacted Files](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/803?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/LocalDatastoreController.browser.js](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/803/diff?src=pr&el=tree#diff-c3JjL0xvY2FsRGF0YXN0b3JlQ29udHJvbGxlci5icm93c2VyLmpz) | `95% <66.66%> (-2.37%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/803?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/803?src=pr&el=footer). Last update [1fe021b...1057671](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/803?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

dplewis: @acinader After much trial and error, I discovered with the latest jest / jsdom you can't spy after mocking localStorage. It is already set in window.localStorage by jest. No need to mock it

Patch:
diff --git a/src/LocalDatastoreController.browser.js b/src/LocalDatastoreController.browser.js
index 667fc41..dacb77a 100644
--- a/src/LocalDatastoreController.browser.js
+++ b/src/LocalDatastoreController.browser.js
@@ -42,7 +42,11 @@ const LocalDatastoreController = {
       const key = localStorage.key(i);
       if (isLocalDatastoreKey(key)) {
         const value = localStorage.getItem(key);
-        LDS[key] = JSON.parse(value);
+        try {
+          LDS[key] = JSON.parse(value);
+        } catch (error) {
+          console.error('Error getAllContents: ', error);
+        }
       }
     }
     return LDS;


Test patch:
diff --git a/src/__tests__/LocalDatastore-test.js b/src/__tests__/LocalDatastore-test.js
index 3b13f6c..dc6bbb1 100644
--- a/src/__tests__/LocalDatastore-test.js
+++ b/src/__tests__/LocalDatastore-test.js
@@ -6,6 +6,7 @@
  * LICENSE file in the root directory of this source tree. An additional grant
  * of patent rights can be found in the PATENTS file in the same directory.
  */
+/* global window */
 
 jest.autoMockOff();
 jest.unmock('../LocalDatastoreUtils');
@@ -103,10 +104,6 @@ const RNDatastoreController = require('../LocalDatastoreController.react-native'
 const BrowserDatastoreController = require('../LocalDatastoreController.browser');
 const DefaultDatastoreController = require('../LocalDatastoreController.default');
 
-const mockLocalStorage = require('./test_helpers/mockLocalStorage');
-
-global.localStorage = mockLocalStorage;
-
 const item1 = new ParseObject('Item');
 const item2 = new ParseObject('Item');
 const item3 = new ParseObject('Item');
@@ -657,7 +654,6 @@ describe('LocalDatastore', () => {
     user._localId = null;
 
     const USER_KEY = LocalDatastore.getKeyForObject(user);
-    console.log(USER_KEY);
     const LDS = {
       [USER_KEY]: [user._toFullJSON()],
       [`${PIN_PREFIX}_testPinName`]: [USER_KEY],
@@ -872,20 +868,30 @@ describe('LocalDatastore (BrowserDatastoreController)', () => {
     expect(await LocalDatastore._getRawStorage()).toEqual({});
   });
 
+  it('can handle getAllContent error', async () => {
+    await LocalDatastore.pinWithName('_default', [{ value: 'WILL_BE_MOCKED' }]);
+    const windowSpy = jest.spyOn(Object.getPrototypeOf(window.localStorage), 'getItem')
+      .mockImplementationOnce(() => {
+        return '[1, ]';
+      });
+    const spy = jest.spyOn(console, 'error').mockImplementationOnce(() => {});
+    const LDS = await LocalDatastore._getAllContents();
+    expect(LDS).toEqual({});
+    expect(spy).toHaveBeenCalled();
+    spy.mockRestore();
+    windowSpy.mockRestore();
+  });
+
   it('can handle store error', async () => {
-    const mockStorageError = {
-      setItem() {
+    const windowSpy = jest.spyOn(Object.getPrototypeOf(window.localStorage), 'setItem')
+      .mockImplementationOnce(() => {
         throw new Error('error thrown');
-      },
-    };
-    Object.defineProperty(window, 'localStorage', { // eslint-disable-line
-      value: mockStorageError
-    });
-    try {
-      await LocalDatastore.pinWithName('myKey', [{ name: 'test' }]);
-    } catch (e) {
-      expect(e.message).toBe('error thrown');
-    }
+      });
+    const consoleSpy = jest.spyOn(console, 'log').mockImplementationOnce(() => {});
+    await LocalDatastore.pinWithName('myKey', [{ name: 'test' }]);
+    expect(consoleSpy).toHaveBeenCalled();
+    consoleSpy.mockRestore();
+    windowSpy.mockRestore();
   });
 });
 
diff --git a/src/__tests__/test_helpers/mockLocalStorage.js b/src/__tests__/test_helpers/mockLocalStorage.js
deleted file mode 100644
index eadaf97..0000000
--- a/src/__tests__/test_helpers/mockLocalStorage.js
+++ /dev/null
@@ -1,39 +0,0 @@
-/**
- * Copyright (c) 2015-present, Parse, LLC.
- * All rights reserved.
- *
- * This source code is licensed under the BSD-style license found in the
- * LICENSE file in the root directory of this source tree. An additional grant
- * of patent rights can be found in the PATENTS file in the same directory.
- */
-
-let mockStorage = {};
-const mockLocalStorage = {
-
-  getItem(path) {
-    return mockStorage[path] || null;
-  },
-
-  setItem(path, value) {
-    mockStorage[path] = value;
-  },
-
-  removeItem(path) {
-    delete mockStorage[path];
-  },
-
-  get length() {
-    return Object.keys(mockStorage).length;
-  },
-
-  key: (i) => {
-    const keys = Object.keys(mockStorage);
-    return keys[i] || null;
-  },
-
-  clear() {
-    mockStorage = {};
-  },
-};
-
-module.exports = mockLocalStorage;