From 174ee951531f22588bb4652f2dee90c52e7d1387 Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Mon, 20 Apr 2020 15:13:02 +0200 Subject: [PATCH] Store: Error handling for setObject (#23650) * Error handling for setObject to store * Update public/app/core/store.ts Co-Authored-By: kay delaney <45561153+kaydelaney@users.noreply.github.com> * Update public/app/features/explore/RichHistory/RichHistory.tsx Co-Authored-By: kay delaney <45561153+kaydelaney@users.noreply.github.com> * Move setState outside of try-catch block Co-authored-by: kay delaney <45561153+kaydelaney@users.noreply.github.com> --- .../LocalStorageValueProvider.tsx | 6 ++- public/app/core/store.ts | 10 ++-- public/app/core/utils/explore.ts | 16 ++++-- public/app/core/utils/richHistory.ts | 51 ++++++++++++------- .../components/PanelEditor/state/actions.ts | 6 ++- .../explore/RichHistory/RichHistory.tsx | 11 +++- 6 files changed, 67 insertions(+), 33 deletions(-) diff --git a/public/app/core/components/LocalStorageValueProvider/LocalStorageValueProvider.tsx b/public/app/core/components/LocalStorageValueProvider/LocalStorageValueProvider.tsx index 14bc7bbef92..317e255dde0 100644 --- a/public/app/core/components/LocalStorageValueProvider/LocalStorageValueProvider.tsx +++ b/public/app/core/components/LocalStorageValueProvider/LocalStorageValueProvider.tsx @@ -24,7 +24,11 @@ export class LocalStorageValueProvider extends PureComponent, State< onSaveToStore = (value: T) => { const { storageKey } = this.props; - store.setObject(storageKey, value); + try { + store.setObject(storageKey, value); + } catch (error) { + console.error(error); + } this.setState({ value }); }; diff --git a/public/app/core/store.ts b/public/app/core/store.ts index e29111358c5..c2e5af1a25f 100644 --- a/public/app/core/store.ts +++ b/public/app/core/store.ts @@ -29,21 +29,19 @@ export class Store { return ret; } - // Returns true when successfully stored - setObject(key: string, value: any): boolean { + /* Returns true when successfully stored, throws error if not successfully stored */ + setObject(key: string, value: any) { let json; try { json = JSON.stringify(value); } catch (error) { - console.error(`Could not stringify object: ${key}. [${error}]`); - return false; + throw new Error(`Could not stringify object: ${key}. [${error}]`); } try { this.set(key, json); } catch (error) { // Likely hitting storage quota - console.error(`Could not save item in localStorage: ${key}. [${error}]`); - return false; + throw new Error(`Could not save item in localStorage: ${key}. [${error}]`); } return true; } diff --git a/public/app/core/utils/explore.ts b/public/app/core/utils/explore.ts index a8c9107c03b..012545d9e2b 100644 --- a/public/app/core/utils/explore.ts +++ b/public/app/core/utils/explore.ts @@ -345,18 +345,24 @@ export function updateHistory( queries: T[] ): Array> { const ts = Date.now(); + let updatedHistory = history; queries.forEach(query => { - history = [{ query, ts }, ...history]; + updatedHistory = [{ query, ts }, ...updatedHistory]; }); - if (history.length > MAX_HISTORY_ITEMS) { - history = history.slice(0, MAX_HISTORY_ITEMS); + if (updatedHistory.length > MAX_HISTORY_ITEMS) { + updatedHistory = updatedHistory.slice(0, MAX_HISTORY_ITEMS); } // Combine all queries of a datasource type into one history const historyKey = `grafana.explore.history.${datasourceId}`; - store.setObject(historyKey, history); - return history; + try { + store.setObject(historyKey, updatedHistory); + return updatedHistory; + } catch (error) { + console.error(error); + return history; + } } export function clearHistory(datasourceId: string) { diff --git a/public/app/core/utils/richHistory.ts b/public/app/core/utils/richHistory.ts index 7c51c9b86e4..6a2748b56ad 100644 --- a/public/app/core/utils/richHistory.ts +++ b/public/app/core/utils/richHistory.ts @@ -2,7 +2,8 @@ import _ from 'lodash'; // Services & Utils -import { DataQuery, ExploreMode, dateTime, urlUtil } from '@grafana/data'; +import { DataQuery, ExploreMode, dateTime, AppEvents, urlUtil } from '@grafana/data'; +import appEvents from 'app/core/app_events'; import store from 'app/core/store'; import { serializeStateToUrlParam, SortOrder } from './explore'; import { getExploreDatasources } from '../../features/explore/state/selectors'; @@ -55,18 +56,17 @@ export function addToRichHistory( return richHistory; } - let newHistory = [ + let updatedHistory = [ { queries: queriesToSave, ts, datasourceId, datasourceName, starred, comment, sessionName }, ...queriesToKeep, ]; - /* Combine all queries of a datasource type into one rich history */ - const isSaved = store.setObject(RICH_HISTORY_KEY, newHistory); - - /* If newHistory is succesfully saved, return it. Otherwise return not updated richHistory. */ - if (isSaved) { - return newHistory; - } else { + /* If updatedHistory is succesfully saved, return it. Otherwise return not updated richHistory. */ + try { + store.setObject(RICH_HISTORY_KEY, updatedHistory); + return updatedHistory; + } catch (error) { + appEvents.emit(AppEvents.alertError, [error]); return richHistory; } } @@ -83,7 +83,7 @@ export function deleteAllFromRichHistory() { } export function updateStarredInRichHistory(richHistory: RichHistoryQuery[], ts: number) { - const updatedQueries = richHistory.map(query => { + const updatedHistory = richHistory.map(query => { /* Timestamps are currently unique - we can use them to identify specific queries */ if (query.ts === ts) { const isStarred = query.starred; @@ -93,8 +93,13 @@ export function updateStarredInRichHistory(richHistory: RichHistoryQuery[], ts: return query; }); - store.setObject(RICH_HISTORY_KEY, updatedQueries); - return updatedQueries; + try { + store.setObject(RICH_HISTORY_KEY, updatedHistory); + return updatedHistory; + } catch (error) { + appEvents.emit(AppEvents.alertError, [error]); + return richHistory; + } } export function updateCommentInRichHistory( @@ -102,7 +107,7 @@ export function updateCommentInRichHistory( ts: number, newComment: string | undefined ) { - const updatedQueries = richHistory.map(query => { + const updatedHistory = richHistory.map(query => { if (query.ts === ts) { const updatedQuery = Object.assign({}, query, { comment: newComment }); return updatedQuery; @@ -110,14 +115,24 @@ export function updateCommentInRichHistory( return query; }); - store.setObject(RICH_HISTORY_KEY, updatedQueries); - return updatedQueries; + try { + store.setObject(RICH_HISTORY_KEY, updatedHistory); + return updatedHistory; + } catch (error) { + appEvents.emit(AppEvents.alertError, [error]); + return richHistory; + } } export function deleteQueryInRichHistory(richHistory: RichHistoryQuery[], ts: number) { - const updatedQueries = richHistory.filter(query => query.ts !== ts); - store.setObject(RICH_HISTORY_KEY, updatedQueries); - return updatedQueries; + const updatedHistory = richHistory.filter(query => query.ts !== ts); + try { + store.setObject(RICH_HISTORY_KEY, updatedHistory); + return updatedHistory; + } catch (error) { + appEvents.emit(AppEvents.alertError, [error]); + return richHistory; + } } export const sortQueries = (array: RichHistoryQuery[], sortOrder: SortOrder) => { diff --git a/public/app/features/dashboard/components/PanelEditor/state/actions.ts b/public/app/features/dashboard/components/PanelEditor/state/actions.ts index 2220830ca55..a4c53c18a09 100644 --- a/public/app/features/dashboard/components/PanelEditor/state/actions.ts +++ b/public/app/features/dashboard/components/PanelEditor/state/actions.ts @@ -78,6 +78,10 @@ export function updatePanelEditorUIState(uiState: Partial): return (dispatch, getStore) => { const nextState = { ...getStore().panelEditor.ui, ...uiState }; dispatch(setPanelEditorUIState(nextState)); - store.setObject(PANEL_EDITOR_UI_STATE_STORAGE_KEY, nextState); + try { + store.setObject(PANEL_EDITOR_UI_STATE_STORAGE_KEY, nextState); + } catch (error) { + console.error(error); + } }; } diff --git a/public/app/features/explore/RichHistory/RichHistory.tsx b/public/app/features/explore/RichHistory/RichHistory.tsx index 5356a1ab012..fa0892c7c9a 100644 --- a/public/app/features/explore/RichHistory/RichHistory.tsx +++ b/public/app/features/explore/RichHistory/RichHistory.tsx @@ -114,7 +114,14 @@ class UnThemedRichHistory extends PureComponent { - store.setObject(RICH_HISTORY_SETTING_KEYS.datasourceFilters, value); + try { + store.setObject(RICH_HISTORY_SETTING_KEYS.datasourceFilters, value); + } catch (error) { + console.error(error); + } + /* Set data source filters to state even though they were not successfully saved in + * localStorage to allow interaction and filtering. + **/ this.setState({ datasourceFilters: value }); }; @@ -125,7 +132,7 @@ class UnThemedRichHistory extends PureComponent this.setState({ sortOrder }); /* If user selects activeDatasourceOnly === true, set datasource filter to currently active datasource. - * Filtering based on datasource won't be available. Otherwise set to null, as filtering will be + * Filtering based on datasource won't be available. Otherwise set to null, as filtering will be * available for user. */ updateFilters() {