Skip to content

Commit 3669df6

Browse files
alexcjohnsonalexcjohnson
committed
cleaner error when storage fills up
and a bit of test cleanup
1 parent 690b0a3 commit 3669df6

File tree

2 files changed

+48
-30
lines changed

2 files changed

+48
-30
lines changed

‎dash-renderer/src/persistence.js‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,8 @@ function keyPrefixMatch(prefix, separator) {
100100
}
101101

102102
const UNDEFINED = 'U';
103-
const _parse = val => val === UNDEFINED ? void 0 : JSON.parse(val || null);
104-
const _stringify = val => val === void 0 ? UNDEFINED : JSON.stringify(val);
103+
const _parse = val => (val === UNDEFINED ? void 0 : JSON.parse(val || null));
104+
const _stringify = val => (val === void 0 ? UNDEFINED : JSON.stringify(val));
105105

106106
class WebStore {
107107
constructor(backEnd) {
@@ -119,22 +119,26 @@ class WebStore {
119119
return _parse(this._storage.getItem(storePrefix + key));
120120
}
121121

122+
_setItem(key, value) {
123+
// unprotected version of setItem, for use by tryGetWebStore
124+
this._storage.setItem(storePrefix + key, _stringify(value));
125+
}
122126
/*
123127
* In addition to the regular key->value to set, setItem takes
124128
* dispatch as a parameter, so it can report OOM to devtools
125129
*/
126130
setItem(key, value, dispatch) {
127131
try {
128-
this._storage.setItem(storePrefix + key, _stringify(value));
132+
this._setItem(key, value);
129133
} catch (e) {
130-
if (dispatch) {
131-
dispatch(err(e));
132-
} else {
133-
throw e;
134-
}
135-
// TODO: Should we clear storage here? Or fall back to memory?
136-
// Probably not, unless we want to handle this at a higher level
137-
// so we can keep all 3 items in sync
134+
dispatch(
135+
err(
136+
`${key} failed to save in ${this._name}. Persisted props may be lost.`
137+
)
138+
);
139+
// TODO: at some point we may want to convert this to fall back
140+
// on memory, pulling out all persistence keys and putting them
141+
// in a MemStore that gets used from then onward.
138142
}
139143
}
140144

@@ -226,7 +230,7 @@ function tryGetWebStore(backEnd, dispatch) {
226230
const storeTest = longString();
227231
const testKey = storePrefix + 'x.x';
228232
try {
229-
store.setItem(testKey, storeTest);
233+
store._setItem(testKey, storeTest);
230234
if (store.getItem(testKey) !== storeTest) {
231235
dispatch(
232236
err(`${backEnd} init failed set/get, falling back to memory`)
@@ -242,7 +246,7 @@ function tryGetWebStore(backEnd, dispatch) {
242246
}
243247
try {
244248
store.clear();
245-
store.setItem(testKey, storeTest);
249+
store._setItem(testKey, storeTest);
246250
if (store.getItem(testKey) !== storeTest) {
247251
throw new Error('nope');
248252
}

‎dash-renderer/tests/persistence.test.js‎

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,6 @@
33
/* eslint-disable no-console */
44
import {recordUiEdit, stores, storePrefix} from '../src/persistence';
55

6-
const _dispatch = a => {
7-
return evt => {
8-
a.push(evt.payload.error.message);
9-
}
10-
};
11-
126
const longString = pow => {
137
let s = 's';
148
for (let i = 0; i < pow; i++) {
@@ -74,6 +68,14 @@ describe('storage fallbacks and equivalence', () => {
7468
const propStr = String(propVal);
7569
let originalConsoleErr;
7670
let consoleCalls;
71+
let dispatchCalls;
72+
73+
const _dispatch = evt => {
74+
// verify that dispatch is sending errors to the devtools,
75+
// and record the message sent
76+
expect(evt.type).toEqual('ON_ERROR');
77+
dispatchCalls.push(evt.payload.error.message);
78+
}
7779

7880
beforeEach(() => {
7981
window.my_components = {
@@ -85,8 +87,9 @@ describe('storage fallbacks and equivalence', () => {
8587
}
8688
};
8789

88-
originalConsoleErr = console.error;
90+
dispatchCalls = [];
8991
consoleCalls = [];
92+
originalConsoleErr = console.error;
9093
console.error = msg => {
9194
consoleCalls.push(msg);
9295
};
@@ -107,21 +110,17 @@ describe('storage fallbacks and equivalence', () => {
107110
const layout = layoutA(storeType);
108111

109112
test(`empty ${storeName} works`, () => {
110-
const dispatchCalls = [];
111-
store.clear();
112-
113-
recordUiEdit(layout, {p1: propVal}, _dispatch(dispatchCalls));
113+
recordUiEdit(layout, {p1: propVal}, _dispatch);
114114
expect(dispatchCalls).toEqual([]);
115115
expect(consoleCalls).toEqual([]);
116116
expect(store.getItem(`${storePrefix}a.p1`)).toEqual(propStr);
117117
expect(store.getItem(`${storePrefix}a.p1.orig`)).toEqual('U');
118118
});
119119

120120
test(`${storeName} full from persistence works with warnings`, () => {
121-
const dispatchCalls = [];
122121
fillStorage(store, `${storePrefix}x.x`);
123122

124-
recordUiEdit(layout, {p1: propVal}, _dispatch(dispatchCalls));
123+
recordUiEdit(layout, {p1: propVal}, _dispatch);
125124
expect(dispatchCalls).toEqual([
126125
`${storeName} init first try failed; clearing and retrying`,
127126
`${storeName} init set/get succeeded after clearing!`
@@ -134,10 +133,9 @@ describe('storage fallbacks and equivalence', () => {
134133
});
135134

136135
test(`${storeName} full from other stuff falls back on memory`, () => {
137-
const dispatchCalls = [];
138136
fillStorage(store, 'not_ours');
139137

140-
recordUiEdit(layout, {p1: propVal}, _dispatch(dispatchCalls));
138+
recordUiEdit(layout, {p1: propVal}, _dispatch);
141139
expect(dispatchCalls).toEqual([
142140
`${storeName} init first try failed; clearing and retrying`,
143141
`${storeName} init still failed, falling back to memory`
@@ -147,6 +145,22 @@ describe('storage fallbacks and equivalence', () => {
147145
const x = Boolean(store.getItem('not_ours'));
148146
expect(x).toBe(true);
149147
});
148+
149+
test(`${storeName} that fills up later on just logs an error`, () => {
150+
// Maybe not ideal long-term behavior, but this is what happens
151+
152+
// initialize and ensure the store is happy
153+
recordUiEdit(layout, {p1: propVal}, _dispatch);
154+
expect(dispatchCalls).toEqual([]);
155+
expect(consoleCalls).toEqual([]);
156+
157+
// now flood it.
158+
recordUiEdit(layout, {p1: longString(26)}, _dispatch);
159+
expect(dispatchCalls).toEqual([
160+
`a.p1 failed to save in ${storeName}. Persisted props may be lost.`
161+
]);
162+
expect(consoleCalls).toEqual(dispatchCalls);
163+
});
150164
});
151165

152166
['local', 'session', 'memory'].forEach(storeType => {
@@ -155,7 +169,7 @@ describe('storage fallbacks and equivalence', () => {
155169

156170
test(`${storeType} primitives in/out match`, () => {
157171
// ensure storage is instantiated
158-
recordUiEdit(layout, {p1: propVal}, _dispatch());
172+
recordUiEdit(layout, {p1: propVal}, _dispatch);
159173
const store = stores[storeType];
160174
[
161175
0, 1, 1.1, true, false, null, undefined, '', 'hi', '0', '1'
@@ -166,7 +180,7 @@ describe('storage fallbacks and equivalence', () => {
166180
});
167181

168182
test(`${storeType} arrays and objects in/out are clones`, () => {
169-
recordUiEdit(layout, {p1: propVal}, _dispatch());
183+
recordUiEdit(layout, {p1: propVal}, _dispatch);
170184
const store = stores[storeType];
171185

172186
[[1, 2, 3], {a: 1, b: 2}].forEach(val => {

0 commit comments

Comments
 (0)