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

## Summary

This PR fixes how aborted requests are handled in saved search
embeddables. Previously all failed requests after the first request were
considered aborted by the embeddable which resulted in at least two
incorrect behaviours:
- Any failed requests after the first request we're being swallowed by
the embeddable since it considered them aborted, meaning no error state
was ever shown. Now the embeddable correctly changes to an error state
on failed (non-aborted) requests.
- Replacing a Dashboard with a saved search panel previously resulted in
an aborted error state. This no longer happens and the saved search
panel successfully loads.

The issue was originally introduced in #137690 with work aimed at
eliminating aborted error states in the saved search embeddable.

Fixes #151067.
Fixes #153797.

### Checklist

- [ ] ~Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~
- [ ]
~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials~
- [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
- [ ] ~Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard
accessibility](https://webaim.org/techniques/keyboard/))~
- [ ] ~Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~
- [ ] ~If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~
- [ ] ~This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~
- [ ] ~This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)~

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
This commit is contained in:
Davis McPhee 2023-03-30 10:41:04 -03:00 committed by GitHub
parent 102aadf0d2
commit 1126da53d5
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('2,572 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('4,633 documents');
});
});
}