Skip to content

Commit

Permalink
Throttle toast storms related to saving/restoring URL state (#154792)
Browse files Browse the repository at this point in the history
## Summary

Resolves #153073.

Throttles toast errors related to restoring/saving URL state.

Before:

<img width="330" alt="image"
src="https://user-images.githubusercontent.com/1178348/231315783-98b966a8-b665-4465-91ae-b88933a56c3b.png">

After:

<img width="333" alt="image"
src="https://user-images.githubusercontent.com/1178348/231315724-7b3109eb-bdc7-405f-865f-36c909b907f4.png">

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
lukasolson and kibanamachine authored Apr 13, 2023
1 parent 61f212f commit dc1baa9
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 12 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { withNotifyOnErrors } from './errors';
import { notificationServiceMock } from '@kbn/core-notifications-browser-mocks';

describe('state management URL errors', () => {
const notifications = notificationServiceMock.createStartContract();

beforeEach(() => {
notifications.toasts.addError.mockClear();
});

test('notifies on restore error only once', () => {
const { onGetError } = withNotifyOnErrors(notifications.toasts);
const error = new Error();
onGetError(error);
onGetError(error);
expect(notifications.toasts.addError).toBeCalledTimes(1);
});

test('notifies on save error only once', () => {
const { onSetError } = withNotifyOnErrors(notifications.toasts);
const error = new Error();
onSetError(error);
onSetError(error);
expect(notifications.toasts.addError).toBeCalledTimes(1);
});
});
31 changes: 21 additions & 10 deletions src/plugins/kibana_utils/public/state_management/url/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Side Public License, v 1.
*/

import { throttle } from 'lodash';
import { i18n } from '@kbn/i18n';
import { NotificationsStart } from '@kbn/core/public';

Expand All @@ -23,6 +24,24 @@ export const saveStateInUrlErrorTitle = i18n.translate(
}
);

// Prevent toast storms by throttling. See https://github.com/elastic/kibana/issues/153073
const throttledOnRestoreError = throttle((toasts: NotificationsStart['toasts'], e: Error) => {
toasts.addError(e, {
title: restoreUrlErrorTitle,
});
}, 10000);
const throttledOnSaveError = throttle((toasts: NotificationsStart['toasts'], e: Error) => {
toasts.addError(e, {
title: saveStateInUrlErrorTitle,
});
}, 10000);

// Helper to bypass throttling if consumers need to handle errors right away
export const flushNotifyOnErrors = () => {
throttledOnRestoreError.flush();
throttledOnSaveError.flush();
};

/**
* Helper for configuring {@link IKbnUrlStateStorage} to notify about inner errors
*
Expand All @@ -37,15 +56,7 @@ export const saveStateInUrlErrorTitle = i18n.translate(
*/
export const withNotifyOnErrors = (toasts: NotificationsStart['toasts']) => {
return {
onGetError: (error: Error) => {
toasts.addError(error, {
title: restoreUrlErrorTitle,
});
},
onSetError: (error: Error) => {
toasts.addError(error, {
title: saveStateInUrlErrorTitle,
});
},
onGetError: (e: Error) => throttledOnRestoreError(toasts, e),
onSetError: (e: Error) => throttledOnSaveError(toasts, e),
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,9 @@ export {
} from './kbn_url_storage';
export { createKbnUrlTracker } from './kbn_url_tracker';
export { createUrlTracker } from './url_tracker';
export { withNotifyOnErrors, saveStateInUrlErrorTitle, restoreUrlErrorTitle } from './errors';
export {
withNotifyOnErrors,
flushNotifyOnErrors,
saveStateInUrlErrorTitle,
restoreUrlErrorTitle,
} from './errors';
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { History, createBrowserHistory } from 'history';
import { takeUntil, toArray } from 'rxjs/operators';
import { Subject } from 'rxjs';
import { CoreScopedHistory } from '@kbn/core/public';
import { withNotifyOnErrors } from '../../state_management/url';
import { withNotifyOnErrors, flushNotifyOnErrors } from '../../state_management/url';
import { coreMock } from '@kbn/core/public/mocks';

describe('KbnUrlStateStorage', () => {
Expand Down Expand Up @@ -123,6 +123,7 @@ describe('KbnUrlStateStorage', () => {
const key = '_s';
history.replace(`/#?${key}=(ok:2,test:`); // malformed rison
expect(() => urlStateStorage.get(key)).not.toThrow();
flushNotifyOnErrors();
expect(toasts.addError).toBeCalled();
});
});
Expand Down Expand Up @@ -304,6 +305,7 @@ describe('KbnUrlStateStorage', () => {
const key = '_s';
history.replace(`/?${key}=(ok:2,test:`); // malformed rison
expect(() => urlStateStorage.get(key)).not.toThrow();
flushNotifyOnErrors();
expect(toasts.addError).toBeCalled();
});
});
Expand Down
1 change: 1 addition & 0 deletions src/plugins/kibana_utils/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"@kbn/test-jest-helpers",
"@kbn/rison",
"@kbn/crypto-browser",
"@kbn/core-notifications-browser-mocks",
],
"exclude": [
"target/**/*",
Expand Down

0 comments on commit dc1baa9

Please sign in to comment.