Improve state sync error handling (#74264) (#74524)

Fixes #71461 regression since 7.7

New state syncing utils didn't properly handle errors,
Errors happening during URL parsing or writing wasn't handled, so state syncing could stop or in worth case blow out. (see #71461)

There are not much scenarios where missing proper error handling could really impact users, except the one described in #71461:

Kibana users state:storeInSessionStorage
Users often intuitively share hashed dashboard urls directly
When someone opens those urls - there is a blank screen with warning
In 7.6 - dashboard would still load with default state.
Since 7.7 these still could be achieved by removing query params for URL, but it is not obvious for regular users.

This PR makes sure that behaviour is similar to one we had before 7.7.


Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

docs/development/plugins/kibana_utils/public/state_sync/kibana-plugin-plugins-kibana_utils-public-state_sync.createkbnurlstatestorage.md

@@ -9,8 +9,10 @@ Creates [IKbnUrlStateStorage](./kibana-plugin-plugins-kibana_utils-public-state_
 <b>Signature:</b>
 
 ```typescript
-createKbnUrlStateStorage: ({ useHash, history }?: {
+createKbnUrlStateStorage: ({ useHash, history, onGetError, onSetError, }?: {
     useHash: boolean;
     history?: History<any> | undefined;
+    onGetError?: ((error: Error) => void) | undefined;
+    onSetError?: ((error: Error) => void) | undefined;
 }) => IKbnUrlStateStorage
 ```

src/plugins/dashboard/public/application/legacy_app.js

@@ -30,6 +30,7 @@ import {
   createKbnUrlStateStorage,
   redirectWhenMissing,
   SavedObjectNotFound,
+  withNotifyOnErrors,
 } from '../../../kibana_utils/public';
 import { DashboardListing, EMPTY_FILTER } from './listing/dashboard_listing';
 import { addHelpMenuToAppChrome } from './help_menu/help_menu_util';
@@ -65,6 +66,7 @@ export function initDashboardApp(app, deps) {
     createKbnUrlStateStorage({
       history,
       useHash: deps.uiSettings.get('state:storeInSessionStorage'),
+      ...withNotifyOnErrors(deps.core.notifications.toasts),
     })
   );
 

src/plugins/discover/public/application/angular/context.js

@@ -83,6 +83,7 @@ function ContextAppRouteController($routeParams, $scope, $route) {
     timeFieldName: indexPattern.timeFieldName,
     storeInSessionStorage: getServices().uiSettings.get('state:storeInSessionStorage'),
     history: getServices().history(),
+    toasts: getServices().core.notifications.toasts,
   });
   this.state = { ...appState.getState() };
   this.anchorId = $routeParams.id;

src/plugins/discover/public/application/angular/context_state.ts

@@ -18,11 +18,13 @@
  */
 import _ from 'lodash';
 import { History } from 'history';
+import { NotificationsStart } from 'kibana/public';
 import {
   createStateContainer,
   createKbnUrlStateStorage,
   syncStates,
   BaseStateContainer,
+  withNotifyOnErrors,
 } from '../../../../kibana_utils/public';
 import { esFilters, FilterManager, Filter, Query } from '../../../../data/public';
 
@@ -74,6 +76,13 @@ interface GetStateParams {
    * History instance to use
    */
   history: History;
+
+  /**
+   * Core's notifications.toasts service
+   * In case it is passed in,
+   * kbnUrlStateStorage will use it notifying about inner errors
+   */
+  toasts?: NotificationsStart['toasts'];
 }
 
 interface GetStateReturn {
@@ -123,10 +132,12 @@ export function getState({
   timeFieldName,
   storeInSessionStorage = false,
   history,
+  toasts,
 }: GetStateParams): GetStateReturn {
   const stateStorage = createKbnUrlStateStorage({
     useHash: storeInSessionStorage,
     history,
+    ...(toasts && withNotifyOnErrors(toasts)),
   });
 
   const globalStateInitial = stateStorage.get(GLOBAL_STATE_URL_KEY) as GlobalState;

src/plugins/discover/public/application/angular/discover.js

@@ -220,6 +220,7 @@ function discoverController($element, $route, $scope, $timeout, $window, Promise
     defaultAppState: getStateDefaults(),
     storeInSessionStorage: config.get('state:storeInSessionStorage'),
     history,
+    toasts: core.notifications.toasts,
   });
   if (appStateContainer.getState().index !== $scope.indexPattern.id) {
     //used index pattern is different than the given by url/state which is invalid

src/plugins/discover/public/application/angular/discover_state.ts

@@ -18,12 +18,14 @@
  */
 import { isEqual } from 'lodash';
 import { History } from 'history';
+import { NotificationsStart } from 'kibana/public';
 import {
   createStateContainer,
   createKbnUrlStateStorage,
   syncState,
   ReduxLikeStateContainer,
   IKbnUrlStateStorage,
+  withNotifyOnErrors,
 } from '../../../../kibana_utils/public';
 import { esFilters, Filter, Query } from '../../../../data/public';
 import { migrateLegacyQuery } from '../../../../kibana_legacy/public';
@@ -68,6 +70,13 @@ interface GetStateParams {
    * Browser history
    */
   history: History;
+
+  /**
+   * Core's notifications.toasts service
+   * In case it is passed in,
+   * kbnUrlStateStorage will use it notifying about inner errors
+   */
+  toasts?: NotificationsStart['toasts'];
 }
 
 export interface GetStateReturn {
@@ -122,10 +131,12 @@ export function getState({
   defaultAppState = {},
   storeInSessionStorage = false,
   history,
+  toasts,
 }: GetStateParams): GetStateReturn {
   const stateStorage = createKbnUrlStateStorage({
     useHash: storeInSessionStorage,
     history,
+    ...(toasts && withNotifyOnErrors(toasts)),
   });
 
   const appStateFromUrl = stateStorage.get(APP_STATE_URL_KEY) as AppState;

src/plugins/kibana_utils/docs/state_sync/README.md

@@ -58,3 +58,4 @@ To run them, start kibana with `--run-examples` flag.
 - [On-the-fly state migrations](./on_fly_state_migrations.md).
 - [syncStates helper](./sync_states.md).
 - [Helpers for Data plugin (syncing TimeRange, RefreshInterval and Filters)](./data_plugin_helpers.md).
+- [Error handling](./error_handling.md)

src/plugins/kibana_utils/docs/state_sync/error_handling.md

@@ -0,0 +1,6 @@
+# Error handling
+
+State syncing util doesn't have specific api for handling errors.
+It expects that errors are handled on storage level.
+
+see [KbnUrlStateStorage](./storages/kbn_url_storage.md#) error handling section for details.

src/plugins/kibana_utils/docs/state_sync/storages/kbn_url_storage.md

@@ -65,7 +65,7 @@ To prevent bugs caused by missing history updates, make sure your app uses one i
 For example, if you use `react-router`:
 
 ```tsx
-const App = props => {
+const App = (props) => {
   useEffect(() => {
     const stateStorage = createKbnUrlStateStorage({
       useHash: props.uiSettings.get('state:storeInSessionStorage'),
@@ -160,3 +160,58 @@ const { start, stop } = syncStates([
 
 <Router history={history} />;
 ```
+
+### Error handling
+
+Errors could occur both during `kbnUrlStateStorage.get()` and `kbnUrlStateStorage.set()`
+
+#### Handling kbnUrlStateStorage.get() errors
+
+Possible error scenarios during `kbnUrlStateStorage.get()`:
+
+1. Rison in URL is malformed. Parsing exception.
+2. useHash is enabled and current hash is missing in `sessionStorage`
+
+In all the cases error is handled internally and `kbnUrlStateStorage.get()` returns `null`, just like if there is no state in the URL anymore
+
+You can pass callback to get notified about errors. Use it, for example, for notifying users
+
+```ts
+const kbnUrlStateStorage = createKbnUrlStateStorage({
+  history,
+  onGetError: (error) => {
+    alert(error.message);
+  },
+});
+```
+
+#### Handling kbnUrlStateStorage.set() errors
+
+Possible errors during `kbnUrlStateStorage.set()`:
+
+1. `useHash` is enabled and can't store state in `sessionStorage` (overflow or no access)
+
+In all the cases error is handled internally and URL update is skipped
+
+You can pass callback to get notified about errors. Use it, for example, for notifying users:
+
+```ts
+const kbnUrlStateStorage = createKbnUrlStateStorage({
+  history,
+  onSetError: (error) => {
+    alert(error.message);
+  },
+});
+```
+
+#### Helper to integrate with core.notifications.toasts
+
+The most common scenario is to notify users about issues with state syncing using toast service from core
+There is a convenient helper for this:
+
+```ts
+const kbnUrlStateStorage = createKbnUrlStateStorage({
+  history,
+  ...withNotifyOnErrors(core.notifications.toasts),
+});
+```

src/plugins/kibana_utils/public/index.ts

@@ -57,6 +57,7 @@ export {
   getStateFromKbnUrl,
   getStatesFromKbnUrl,
   setStateToKbnUrl,
+  withNotifyOnErrors,
 } from './state_management/url';
 export {
   syncState,

src/plugins/kibana_utils/public/state_management/url/errors.ts

@@ -0,0 +1,62 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { i18n } from '@kbn/i18n';
+import { NotificationsStart } from 'kibana/public';
+
+export const restoreUrlErrorTitle = i18n.translate(
+  'kibana_utils.stateManagement.url.restoreUrlErrorTitle',
+  {
+    defaultMessage: `Error restoring state from URL`,
+  }
+);
+
+export const saveStateInUrlErrorTitle = i18n.translate(
+  'kibana_utils.stateManagement.url.saveStateInUrlErrorTitle',
+  {
+    defaultMessage: `Error saving state in URL`,
+  }
+);
+
+/**
+ * Helper for configuring {@link IKbnUrlStateStorage} to notify about inner errors
+ *
+ * @example
+ * ```ts
+ * const kbnUrlStateStorage = createKbnUrlStateStorage({
+ *  history,
+ *  ...withNotifyOnErrors(core.notifications.toast))
+ * }
+ * ```
+ * @param toast - toastApi from core.notifications.toasts
+ */
+export const withNotifyOnErrors = (toasts: NotificationsStart['toasts']) => {
+  return {
+    onGetError: (error: Error) => {
+      toasts.addError(error, {
+        title: restoreUrlErrorTitle,
+      });
+    },
+    onSetError: (error: Error) => {
+      toasts.addError(error, {
+        title: saveStateInUrlErrorTitle,
+      });
+    },
+  };
+};

src/plugins/kibana_utils/public/state_management/url/index.ts

@@ -27,3 +27,4 @@ export {
 } from './kbn_url_storage';
 export { createKbnUrlTracker } from './kbn_url_tracker';
 export { createUrlTracker } from './url_tracker';
+export { withNotifyOnErrors, saveStateInUrlErrorTitle, restoreUrlErrorTitle } from './errors';

src/plugins/kibana_utils/public/state_management/url/kbn_url_storage.ts

@@ -103,7 +103,7 @@ export function setStateToKbnUrl<State>(
 export interface IKbnUrlControls {
   /**
    * Listen for url changes
-   * @param cb - get's called when url has been changed
+   * @param cb - called when url has been changed
    */
   listen: (cb: () => void) => () => void;
 
@@ -142,12 +142,12 @@ export interface IKbnUrlControls {
    */
   cancel: () => void;
 }
-export type UrlUpdaterFnType = (currentUrl: string) => string;
+export type UrlUpdaterFnType = (currentUrl: string) => string | undefined;
 
 export const createKbnUrlControls = (
   history: History = createBrowserHistory()
 ): IKbnUrlControls => {
-  const updateQueue: Array<(currentUrl: string) => string> = [];
+  const updateQueue: UrlUpdaterFnType[] = [];
 
   // if we should replace or push with next async update,
   // if any call in a queue asked to push, then we should push
@@ -188,7 +188,7 @@ export const createKbnUrlControls = (
   function getPendingUrl() {
     if (updateQueue.length === 0) return undefined;
     const resultUrl = updateQueue.reduce(
-      (url, nextUpdate) => nextUpdate(url),
+      (url, nextUpdate) => nextUpdate(url) ?? url,
       getCurrentUrl(history)
     );
 
@@ -201,7 +201,7 @@ export const createKbnUrlControls = (
         cb();
       }),
     update: (newUrl: string, replace = false) => updateUrl(newUrl, replace),
-    updateAsync: (updater: (currentUrl: string) => string, replace = false) => {
+    updateAsync: (updater: UrlUpdaterFnType, replace = false) => {
       updateQueue.push(updater);
       if (shouldReplace) {
         shouldReplace = replace;

src/plugins/kibana_utils/public/state_sync/public.api.md

@@ -8,9 +8,11 @@ import { History } from 'history';
 import { Observable } from 'rxjs';
 
 // @public
-export const createKbnUrlStateStorage: ({ useHash, history }?: {
+export const createKbnUrlStateStorage: ({ useHash, history, onGetError, onSetError, }?: {
     useHash: boolean;
     history?: History<any> | undefined;
+    onGetError?: ((error: Error) => void) | undefined;
+    onSetError?: ((error: Error) => void) | undefined;
 }) => IKbnUrlStateStorage;
 
 // Warning: (ae-unresolved-link) The @link reference could not be resolved: The package "kibana" does not have an export "Storage"