Skip to content

Commit

Permalink
[8.x] [Discover] Fix issue where KEEP columns are not applied after…
Browse files Browse the repository at this point in the history
… Elasticsearch error (#205833) (#205968)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Discover] Fix issue where `KEEP` columns are not applied after
Elasticsearch error
(#205833)](#205833)

<!--- Backport version: 8.9.8 -->

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

<!--BACKPORT [{"author":{"name":"Davis
McPhee","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-08T18:20:24Z","message":"[Discover]
Fix issue where `KEEP` columns are not applied after Elasticsearch error
(#205833)\n\n## Summary\r\n\r\nThis PR fixes an issue where columns are
not applied correctly when\r\nusing the ES|QL `KEEP` command after an
Elasticsearch error has\r\noccurred.\r\n\r\nFixes #205353.\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/src/platform/packages/shared/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- [ ] 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 was checked for breaking HTTP API changes, and any
breaking\r\nchanges have been approved by the breaking-change committee.
The\r\n`release_note:breaking` label should be applied in these
situations.\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [x] The PR description includes
the appropriate Release Notes section,\r\nand the correct
`release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"8eb326d5961ef377f0a05c98c281eb5b97bd19d3","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","Team:DataDiscovery","backport:version","v8.17.0","v8.18.0"],"number":205833,"url":"https://github.com/elastic/kibana/pull/205833","mergeCommit":{"message":"[Discover]
Fix issue where `KEEP` columns are not applied after Elasticsearch error
(#205833)\n\n## Summary\r\n\r\nThis PR fixes an issue where columns are
not applied correctly when\r\nusing the ES|QL `KEEP` command after an
Elasticsearch error has\r\noccurred.\r\n\r\nFixes #205353.\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/src/platform/packages/shared/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- [ ] 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 was checked for breaking HTTP API changes, and any
breaking\r\nchanges have been approved by the breaking-change committee.
The\r\n`release_note:breaking` label should be applied in these
situations.\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [x] The PR description includes
the appropriate Release Notes section,\r\nand the correct
`release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"8eb326d5961ef377f0a05c98c281eb5b97bd19d3"}},"sourceBranch":"main","suggestedTargetBranches":["8.17","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/205833","number":205833,"mergeCommit":{"message":"[Discover]
Fix issue where `KEEP` columns are not applied after Elasticsearch error
(#205833)\n\n## Summary\r\n\r\nThis PR fixes an issue where columns are
not applied correctly when\r\nusing the ES|QL `KEEP` command after an
Elasticsearch error has\r\noccurred.\r\n\r\nFixes #205353.\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/src/platform/packages/shared/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- [ ] 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 was checked for breaking HTTP API changes, and any
breaking\r\nchanges have been approved by the breaking-change committee.
The\r\n`release_note:breaking` label should be applied in these
situations.\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [x] The PR description includes
the appropriate Release Notes section,\r\nand the correct
`release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"8eb326d5961ef377f0a05c98c281eb5b97bd19d3"}},{"branch":"8.17","label":"v8.17.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.18.0","labelRegex":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
  • Loading branch information
davismcphee authored Jan 9, 2025
1 parent 5106868 commit 886301a
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,10 @@ export function fetchAll(
// Only the document query should send its errors to main$, to cause the full Discover app
// to get into an error state. The other queries will not cause all of Discover to error out
// but their errors will be shown in-place (e.g. of the chart).
.catch(sendErrorTo(dataSubjects.documents$, dataSubjects.main$));
.catch((e) => {
sendErrorMsg(dataSubjects.documents$, e, { query });
sendErrorMsg(dataSubjects.main$, e);
});

// Return a promise that will resolve once all the requests have finished or failed, or no results are found
return firstValueFrom(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export function useEsqlMode({
switchMap(async (next) => {
const { query: nextQuery } = next;

if (!nextQuery || next.fetchStatus === FetchStatus.ERROR) {
if (!nextQuery) {
return;
}

Expand Down Expand Up @@ -104,6 +104,12 @@ export function useEsqlMode({
return;
}

if (next.fetchStatus === FetchStatus.ERROR) {
// An error occurred, but it's still considered an initial fetch
prev.current.initialFetch = false;
return;
}

if (next.fetchStatus !== FetchStatus.PARTIAL) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,12 @@ export function sendLoadingMoreFinishedMsg(
/**
* Send ERROR message
*/
export function sendErrorMsg(data$: DataMain$ | DataDocuments$ | DataTotalHits$, error?: Error) {
data$.next({ fetchStatus: FetchStatus.ERROR, error });
export function sendErrorMsg<T extends DataMsg>(
data$: DataMain$ | DataDocuments$ | DataTotalHits$,
error?: Error,
props?: Omit<T, 'fetchStatus' | 'error'>
) {
data$.next({ fetchStatus: FetchStatus.ERROR, error, ...props });
}

/**
Expand Down
17 changes: 17 additions & 0 deletions test/functional/apps/discover/esql/_esql_columns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,23 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expect(await dataGrid.getHeaderFields()).to.eql(['ip', '@timestamp', 'bytes']);
});

it('should recover from an error and reset columns correctly when a transformational query is used', async () => {
await monacoEditor.setCodeEditorValue('from not_an_index');
await testSubjects.click('querySubmitButton');
await header.waitUntilLoadingHasFinished();
await discover.showsErrorCallout();
await browser.refresh();
await header.waitUntilLoadingHasFinished();
await discover.showsErrorCallout();
await monacoEditor.setCodeEditorValue(
'from logstash-* | keep ip, @timestamp, bytes | limit 10'
);
await testSubjects.click('querySubmitButton');
await header.waitUntilLoadingHasFinished();
await discover.waitUntilSearchingHasFinished();
expect(await dataGrid.getHeaderFields()).to.eql(['ip', '@timestamp', 'bytes']);
});

it('should restore columns correctly when switching between saved searches', async () => {
await discover.loadSavedSearch(SAVED_SEARCH_NON_TRANSFORMATIONAL_INITIAL_COLUMNS);
await header.waitUntilLoadingHasFinished();
Expand Down

0 comments on commit 886301a

Please sign in to comment.