cleaner error when storage fills up

and a bit of test cleanup

dash-renderer/src/persistence.js

@@ -100,8 +100,8 @@ function keyPrefixMatch(prefix, separator) {
 }
 
 const UNDEFINED = 'U';
-const _parse = val => val === UNDEFINED ? void 0 : JSON.parse(val || null);
-const _stringify = val => val === void 0 ? UNDEFINED : JSON.stringify(val);
+const _parse = val => (val === UNDEFINED ? void 0 : JSON.parse(val || null));
+const _stringify = val => (val === void 0 ? UNDEFINED : JSON.stringify(val));
 
 class WebStore {
  constructor(backEnd) {
@@ -119,22 +119,26 @@ class WebStore {
  return _parse(this._storage.getItem(storePrefix + key));
  }
 
+ _setItem(key, value) {
+ // unprotected version of setItem, for use by tryGetWebStore
+ this._storage.setItem(storePrefix + key, _stringify(value));
+ }
  /*
  * In addition to the regular key->value to set, setItem takes
  * dispatch as a parameter, so it can report OOM to devtools
  */
  setItem(key, value, dispatch) {
  try {
- this._storage.setItem(storePrefix + key, _stringify(value));
+ this._setItem(key, value);
  } catch (e) {
- if (dispatch) {
- dispatch(err(e));
- } else {
- throw e;
- }
- // TODO: Should we clear storage here? Or fall back to memory?
- // Probably not, unless we want to handle this at a higher level
- // so we can keep all 3 items in sync
+ dispatch(
+ err(
+  `${key} failed to save in ${this._name}. Persisted props may be lost.`
+ )
+ );
+ // TODO: at some point we may want to convert this to fall back
+ // on memory, pulling out all persistence keys and putting them
+ // in a MemStore that gets used from then onward.
  }
  }
 
@@ -226,7 +230,7 @@ function tryGetWebStore(backEnd, dispatch) {
  const storeTest = longString();
  const testKey = storePrefix + 'x.x';
  try {
- store.setItem(testKey, storeTest);
+ store._setItem(testKey, storeTest);
  if (store.getItem(testKey) !== storeTest) {
  dispatch(
  err(`${backEnd} init failed set/get, falling back to memory`)
@@ -242,7 +246,7 @@ function tryGetWebStore(backEnd, dispatch) {
  }
  try {
  store.clear();
- store.setItem(testKey, storeTest);
+ store._setItem(testKey, storeTest);
  if (store.getItem(testKey) !== storeTest) {
  throw new Error('nope');
  }

dash-renderer/tests/persistence.test.js

@@ -3,12 +3,6 @@
 /* eslint-disable no-console */
 import {recordUiEdit, stores, storePrefix} from '../src/persistence';
 
-const _dispatch = a => {
- return evt => {
- a.push(evt.payload.error.message);
- }
-};
-
 const longString = pow => {
  let s = 's';
  for (let i = 0; i < pow; i++) {
@@ -74,6 +68,14 @@ describe('storage fallbacks and equivalence', () => {
  const propStr = String(propVal);
  let originalConsoleErr;
  let consoleCalls;
+ let dispatchCalls;
+
+ const _dispatch = evt => {
+ // verify that dispatch is sending errors to the devtools,
+ // and record the message sent
+ expect(evt.type).toEqual('ON_ERROR');
+ dispatchCalls.push(evt.payload.error.message);
+ }
 
  beforeEach(() => {
  window.my_components = {
@@ -85,8 +87,9 @@ describe('storage fallbacks and equivalence', () => {
  }
  };
 
- originalConsoleErr = console.error;
+ dispatchCalls = [];
  consoleCalls = [];
+ originalConsoleErr = console.error;
  console.error = msg => {
  consoleCalls.push(msg);
  };
@@ -107,21 +110,17 @@ describe('storage fallbacks and equivalence', () => {
  const layout = layoutA(storeType);
 
  test(`empty ${storeName} works`, () => {
- const dispatchCalls = [];
- store.clear();
-
- recordUiEdit(layout, {p1: propVal}, _dispatch(dispatchCalls));
+ recordUiEdit(layout, {p1: propVal}, _dispatch);
  expect(dispatchCalls).toEqual([]);
  expect(consoleCalls).toEqual([]);
  expect(store.getItem(`${storePrefix}a.p1`)).toEqual(propStr);
  expect(store.getItem(`${storePrefix}a.p1.orig`)).toEqual('U');
  });
 
  test(`${storeName} full from persistence works with warnings`, () => {
- const dispatchCalls = [];
  fillStorage(store, `${storePrefix}x.x`);
 
- recordUiEdit(layout, {p1: propVal}, _dispatch(dispatchCalls));
+ recordUiEdit(layout, {p1: propVal}, _dispatch);
  expect(dispatchCalls).toEqual([
  `${storeName} init first try failed; clearing and retrying`,
  `${storeName} init set/get succeeded after clearing!`
@@ -134,10 +133,9 @@ describe('storage fallbacks and equivalence', () => {
  });
 
  test(`${storeName} full from other stuff falls back on memory`, () => {
- const dispatchCalls = [];
  fillStorage(store, 'not_ours');
 
- recordUiEdit(layout, {p1: propVal}, _dispatch(dispatchCalls));
+ recordUiEdit(layout, {p1: propVal}, _dispatch);
  expect(dispatchCalls).toEqual([
  `${storeName} init first try failed; clearing and retrying`,
  `${storeName} init still failed, falling back to memory`
@@ -147,6 +145,22 @@ describe('storage fallbacks and equivalence', () => {
  const x = Boolean(store.getItem('not_ours'));
  expect(x).toBe(true);
  });
+
+ test(`${storeName} that fills up later on just logs an error`, () => {
+ // Maybe not ideal long-term behavior, but this is what happens
+
+ // initialize and ensure the store is happy
+ recordUiEdit(layout, {p1: propVal}, _dispatch);
+ expect(dispatchCalls).toEqual([]);
+ expect(consoleCalls).toEqual([]);
+
+ // now flood it.
+ recordUiEdit(layout, {p1: longString(26)}, _dispatch);
+ expect(dispatchCalls).toEqual([
+ `a.p1 failed to save in ${storeName}. Persisted props may be lost.`
+ ]);
+ expect(consoleCalls).toEqual(dispatchCalls);
+ });
  });
 
  ['local', 'session', 'memory'].forEach(storeType => {
@@ -155,7 +169,7 @@ describe('storage fallbacks and equivalence', () => {
 
  test(`${storeType} primitives in/out match`, () => {
  // ensure storage is instantiated
- recordUiEdit(layout, {p1: propVal}, _dispatch());
+ recordUiEdit(layout, {p1: propVal}, _dispatch);
  const store = stores[storeType];
  [
  0, 1, 1.1, true, false, null, undefined, '', 'hi', '0', '1'
@@ -166,7 +180,7 @@ describe('storage fallbacks and equivalence', () => {
  });
 
  test(`${storeType} arrays and objects in/out are clones`, () => {
- recordUiEdit(layout, {p1: propVal}, _dispatch());
+ recordUiEdit(layout, {p1: propVal}, _dispatch);
  const store = stores[storeType];
 
  [[1, 2, 3], {a: 1, b: 2}].forEach(val => {