Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve state sync error handling #74264

Merged
merged 8 commits into from
Aug 6, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
2 changes: 2 additions & 0 deletions src/plugins/dashboard/public/application/legacy_app.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -65,6 +66,7 @@ export function initDashboardApp(app, deps) {
createKbnUrlStateStorage({
history,
useHash: deps.uiSettings.get('state:storeInSessionStorage'),
...withNotifyOnErrors(deps.core.notifications.toasts),
})
);

Expand Down
1 change: 1 addition & 0 deletions src/plugins/discover/public/application/angular/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 11 additions & 0 deletions src/plugins/discover/public/application/angular/context_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions src/plugins/discover/public/application/angular/discover_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/plugins/kibana_utils/docs/state_sync/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
6 changes: 6 additions & 0 deletions src/plugins/kibana_utils/docs/state_sync/error_handling.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down Expand Up @@ -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),
});
```
1 change: 1 addition & 0 deletions src/plugins/kibana_utils/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export {
getStateFromKbnUrl,
getStatesFromKbnUrl,
setStateToKbnUrl,
withNotifyOnErrors,
} from './state_management/url';
export {
syncState,
Expand Down
62 changes: 62 additions & 0 deletions src/plugins/kibana_utils/public/state_management/url/errors.ts
Original file line number Diff line number Diff line change
@@ -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,
});
},
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
);

Expand All @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion src/plugins/kibana_utils/public/state_sync/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Loading