[8.7] [Discover] [Embeddable] Fix aborted request handling in saved search embeddable (#153822) (#154052)

# Backport

This will backport the following commits from `main` to `8.7`:
- [[Discover] [Embeddable] Fix aborted request handling in saved search
embeddable (#153822)](https://github.com/elastic/kibana/pull/153822)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Davis
McPhee","email":"davis.mcphee@elastic.co"},"sourceCommit":{"committedDate":"2023-03-30T13:41:04Z","message":"[Discover]
[Embeddable] Fix aborted request handling in saved search embeddable
(#153822)\n\n## Summary\r\n\r\nThis PR fixes how aborted requests are
handled in saved search\r\nembeddables. Previously all failed requests
after the first request were\r\nconsidered aborted by the embeddable
which resulted in at least two\r\nincorrect behaviours:\r\n- Any failed
requests after the first request we're being swallowed by\r\nthe
embeddable since it considered them aborted, meaning no error
state\r\nwas ever shown. Now the embeddable correctly changes to an
error state\r\non failed (non-aborted) requests.\r\n- Replacing a
Dashboard with a saved search panel previously resulted in\r\nan aborted
error state. This no longer happens and the saved search\r\npanel
successfully loads.\r\n\r\nThe issue was originally introduced in
#137690 with work aimed at\r\neliminating aborted error states in the
saved search embeddable.\r\n\r\nFixes #151067.\r\nFixes
#153797.\r\n\r\n### Checklist\r\n\r\n- [ ] ~Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~\r\n-
[
]\r\n~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials~\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] ~Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard\r\naccessibility](https://webaim.org/techniques/keyboard/))~\r\n-
[ ] ~Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~\r\n-
[ ] ~If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~\r\n-
[ ] ~This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~\r\n-
[ ] ~This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)~\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"1126da53d50e943590fe9813b99f0750d058777e","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Embedding","Feature:Discover","release_note:fix","Team:DataDiscovery","backport:prev-minor","v8.8.0"],"number":153822,"url":"https://github.com/elastic/kibana/pull/153822","mergeCommit":{"message":"[Discover]
[Embeddable] Fix aborted request handling in saved search embeddable
(#153822)\n\n## Summary\r\n\r\nThis PR fixes how aborted requests are
handled in saved search\r\nembeddables. Previously all failed requests
after the first request were\r\nconsidered aborted by the embeddable
which resulted in at least two\r\nincorrect behaviours:\r\n- Any failed
requests after the first request we're being swallowed by\r\nthe
embeddable since it considered them aborted, meaning no error
state\r\nwas ever shown. Now the embeddable correctly changes to an
error state\r\non failed (non-aborted) requests.\r\n- Replacing a
Dashboard with a saved search panel previously resulted in\r\nan aborted
error state. This no longer happens and the saved search\r\npanel
successfully loads.\r\n\r\nThe issue was originally introduced in
#137690 with work aimed at\r\neliminating aborted error states in the
saved search embeddable.\r\n\r\nFixes #151067.\r\nFixes
#153797.\r\n\r\n### Checklist\r\n\r\n- [ ] ~Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~\r\n-
[
]\r\n~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials~\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] ~Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard\r\naccessibility](https://webaim.org/techniques/keyboard/))~\r\n-
[ ] ~Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~\r\n-
[ ] ~If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~\r\n-
[ ] ~This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~\r\n-
[ ] ~This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)~\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"1126da53d50e943590fe9813b99f0750d058777e"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/153822","number":153822,"mergeCommit":{"message":"[Discover]
[Embeddable] Fix aborted request handling in saved search embeddable
(#153822)\n\n## Summary\r\n\r\nThis PR fixes how aborted requests are
handled in saved search\r\nembeddables. Previously all failed requests
after the first request were\r\nconsidered aborted by the embeddable
which resulted in at least two\r\nincorrect behaviours:\r\n- Any failed
requests after the first request we're being swallowed by\r\nthe
embeddable since it considered them aborted, meaning no error
state\r\nwas ever shown. Now the embeddable correctly changes to an
error state\r\non failed (non-aborted) requests.\r\n- Replacing a
Dashboard with a saved search panel previously resulted in\r\nan aborted
error state. This no longer happens and the saved search\r\npanel
successfully loads.\r\n\r\nThe issue was originally introduced in
#137690 with work aimed at\r\neliminating aborted error states in the
saved search embeddable.\r\n\r\nFixes #151067.\r\nFixes
#153797.\r\n\r\n### Checklist\r\n\r\n- [ ] ~Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~\r\n-
[
]\r\n~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials~\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] ~Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard\r\naccessibility](https://webaim.org/techniques/keyboard/))~\r\n-
[ ] ~Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~\r\n-
[ ] ~If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~\r\n-
[ ] ~This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~\r\n-
[ ] ~This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)~\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"1126da53d50e943590fe9813b99f0750d058777e"}}]}]
BACKPORT-->

---------

Co-authored-by: Davis McPhee <davis.mcphee@elastic.co>
This commit is contained in:
Kibana Machine 2023-03-30 13:52:22 -04:00 committed by GitHub
parent 9cc3f37862
commit c8754b87d5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 77 additions and 11 deletions

View file

@ -16,7 +16,7 @@ import { discoverServiceMock } from '../__mocks__/services';
import { SavedSearchEmbeddable, SearchEmbeddableConfig } from './saved_search_embeddable';
import { render } from 'react-dom';
import { createSearchSourceMock } from '@kbn/data-plugin/public/mocks';
import { of, throwError } from 'rxjs';
import { Observable, of, throwError } from 'rxjs';
import { ReactWrapper } from 'enzyme';
import { SHOW_FIELD_STATISTICS } from '../../common';
import { IUiSettingsClient } from '@kbn/core-ui-settings-browser';
@ -57,11 +57,12 @@ describe('saved search embeddable', () => {
let viewModeMockValue: VIEW_MODE = VIEW_MODE.DOCUMENT_LEVEL;
const createEmbeddable = (searchMock?: jest.Mock, customTitle?: string) => {
const searchSource = createSearchSourceMock({ index: dataViewMock }, undefined, searchMock);
const savedSearchMock = {
id: 'mock-id',
title: 'saved search',
sort: [['message', 'asc']] as Array<[string, string]>,
searchSource: createSearchSourceMock({ index: dataViewMock }, undefined, searchMock),
searchSource,
viewMode: viewModeMockValue,
};
@ -102,7 +103,7 @@ describe('saved search embeddable', () => {
(input) => (input.lastReloadRequestTime = Date.now())
);
return { embeddable, searchInput };
return { embeddable, searchInput, searchSource };
};
beforeEach(() => {
@ -304,6 +305,7 @@ describe('saved search embeddable', () => {
expect(embeddable.reload).toHaveBeenCalledTimes(1);
expect(search).toHaveBeenCalledTimes(1);
});
it('should not reload and fetch when a input title matches the saved search title', async () => {
const search = jest.fn().mockReturnValue(getSearchResponse(1));
const { embeddable } = createEmbeddable(search);
@ -316,4 +318,30 @@ describe('saved search embeddable', () => {
expect(embeddable.reload).toHaveBeenCalledTimes(0);
expect(search).toHaveBeenCalledTimes(1);
});
it('should correctly handle aborted requests', async () => {
const { embeddable, searchSource } = createEmbeddable();
await waitOneTick();
const updateOutput = jest.spyOn(embeddable, 'updateOutput');
const abortSignals: AbortSignal[] = [];
jest.spyOn(searchSource, 'fetch$').mockImplementation(
(options) =>
new Observable(() => {
if (options?.abortSignal) {
abortSignals.push(options.abortSignal);
}
throw new Error('Search failed');
})
);
embeddable.reload();
embeddable.reload();
await waitOneTick();
expect(updateOutput).toHaveBeenCalledTimes(3);
expect(abortSignals[0].aborted).toBe(true);
expect(abortSignals[1].aborted).toBe(false);
embeddable.reload();
await waitOneTick();
expect(updateOutput).toHaveBeenCalledTimes(5);
expect(abortSignals[2].aborted).toBe(false);
});
});

View file

@ -176,10 +176,11 @@ export class SavedSearchEmbeddable
const { searchSource } = this.savedSearch;
const prevAbortController = this.abortController;
// Abort any in-progress requests
if (this.abortController) this.abortController.abort();
this.abortController = new AbortController();
const currentAbortController = new AbortController();
this.abortController = currentAbortController;
updateSearchSource(
searchSource,
@ -253,7 +254,7 @@ export class SavedSearchEmbeddable
// Request document data
const { rawResponse: resp } = await lastValueFrom(
searchSource.fetch$({
abortSignal: this.abortController.signal,
abortSignal: currentAbortController.signal,
sessionId: searchSessionId,
inspector: {
adapter: this.inspectorAdapters.requests,
@ -280,7 +281,7 @@ export class SavedSearchEmbeddable
this.searchProps!.totalHitCount = resp.hits.total as number;
this.searchProps!.isLoading = false;
} catch (error) {
const cancelled = !!prevAbortController?.signal.aborted;
const cancelled = !!currentAbortController?.signal.aborted;
if (!this.destroyed && !cancelled) {
this.updateOutput({
...this.getOutput(),

View file

@ -12,9 +12,13 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const browser = getService('browser');
const dataGrid = getService('dataGrid');
const dashboardAddPanel = getService('dashboardAddPanel');
const dashboardPanelActions = getService('dashboardPanelActions');
const dashboardReplacePanel = getService('dashboardReplacePanel');
const filterBar = getService('filterBar');
const queryBar = getService('queryBar');
const esArchiver = getService('esArchiver');
const kibanaServer = getService('kibanaServer');
const testSubjects = getService('testSubjects');
const PageObjects = getPageObjects(['common', 'dashboard', 'header', 'timePicker', 'discover']);
describe('discover saved search embeddable', () => {
@ -28,6 +32,13 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await kibanaServer.uiSettings.replace({
defaultIndex: '0bf35f60-3dc9-11e8-8660-4d65aa086b3c',
});
});
after(async () => {
await kibanaServer.savedObjects.cleanStandardList();
});
beforeEach(async () => {
await PageObjects.common.navigateToApp('dashboard');
await filterBar.ensureFieldEditorModalIsClosed();
await PageObjects.dashboard.gotoDashboardLandingPage();
@ -38,10 +49,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
);
});
after(async () => {
await kibanaServer.savedObjects.cleanStandardList();
});
const addSearchEmbeddableToDashboard = async () => {
await dashboardAddPanel.addSavedSearch('Rendering-Test:-saved-search');
await PageObjects.header.waitUntilLoadingHasFinished();
@ -79,6 +86,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
});
it('should control columns correctly', async () => {
await addSearchEmbeddableToDashboard();
await PageObjects.dashboard.switchToEditMode();
const cell = await dataGrid.getCellElement(0, 2);
@ -95,6 +103,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
});
it('should render duplicate saved search embeddables', async () => {
await addSearchEmbeddableToDashboard();
await addSearchEmbeddableToDashboard();
const [firstGridCell, secondGridCell] = await dataGrid.getAllCellElements();
const firstGridCellContent = await firstGridCell.getVisibleText();
@ -102,5 +111,33 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expect(firstGridCellContent).to.be.equal(secondGridCellContent);
});
it('should display an error', async () => {
await addSearchEmbeddableToDashboard();
await queryBar.setQuery('bytes > 5000');
await queryBar.submitQuery();
await PageObjects.header.waitUntilLoadingHasFinished();
expect(await PageObjects.discover.getSavedSearchDocumentCount()).to.be('2572 documents');
await queryBar.setQuery('this < is not : a valid > query');
await queryBar.submitQuery();
await PageObjects.header.waitUntilLoadingHasFinished();
const embeddableError = await testSubjects.find('embeddableError');
const errorMessage = await embeddableError.findByTestSubject('errorMessageMarkdown');
expect(await errorMessage.getVisibleText()).to.equal(
'Expected AND, OR, end of input, whitespace but "n" found. this < is not : a valid > query ----------^'
);
});
it('should replace a panel with a saved search', async () => {
await dashboardAddPanel.addVisualization('Rendering Test: datatable');
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.dashboard.waitForRenderComplete();
await dashboardPanelActions.replacePanelByTitle('Rendering Test: datatable');
await dashboardReplacePanel.replaceEmbeddable('Rendering-Test:-saved-search');
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.dashboard.waitForRenderComplete();
await testSubjects.missingOrFail('embeddableError');
expect(await PageObjects.discover.getSavedSearchDocumentCount()).to.be('4633 documents');
});
});
}