From 461a39534dead6dfd46fc2a162ce460048d617b1 Mon Sep 17 00:00:00 2001 From: Joshua Li Date: Wed, 11 Sep 2024 11:24:21 -0700 Subject: [PATCH] [query assist] use badge to show agent errors (#7998) - enable query assist for index patterns without data source - add agent error parsing utils - update ml-commons response schema processing - previously ml-commons returns error.body as a string, now it is a JSON object. Ideally frontend should keep it as is to reduce serializing/deserializing, but since older version of ml-commons can be used through MDS, we'll keep it as a string for consistency - use badge to show query assist error if possible - add unit tests Signed-off-by: Joshua Li --- .../query_string/query_string_manager.mock.ts | 7 +- .../query_enhancements/public/plugin.tsx | 6 +- .../public/query_assist/_index.scss | 25 ++ .../query_assist_bar.test.tsx.snap | 47 ++++ .../components/call_outs.test.tsx | 4 +- .../components/query_assist_bar.test.tsx | 218 ++++++++++++++++++ .../components/query_assist_bar.tsx | 9 +- .../components/query_assist_input.test.tsx | 13 +- .../components/query_assist_input.tsx | 18 +- .../components/warning_badge.test.tsx | 62 +++++ .../query_assist/components/warning_badge.tsx | 145 ++++++++++++ .../query_assist/utils/create_extension.tsx | 5 +- .../public/query_assist/utils/errors.test.ts | 20 +- .../public/query_assist/utils/errors.ts | 34 ++- .../query_assist/utils/get_persisted_log.ts | 5 +- .../public/services/index.ts | 6 +- .../server/routes/query_assist/routes.ts | 15 +- .../utils/get_top_nav_config.test.tsx | 5 +- 18 files changed, 613 insertions(+), 31 deletions(-) create mode 100644 src/plugins/query_enhancements/public/query_assist/components/__snapshots__/query_assist_bar.test.tsx.snap create mode 100644 src/plugins/query_enhancements/public/query_assist/components/query_assist_bar.test.tsx create mode 100644 src/plugins/query_enhancements/public/query_assist/components/warning_badge.test.tsx create mode 100644 src/plugins/query_enhancements/public/query_assist/components/warning_badge.tsx diff --git a/src/plugins/data/public/query/query_string/query_string_manager.mock.ts b/src/plugins/data/public/query/query_string/query_string_manager.mock.ts index 95374d1885f0..2fe1e7a7eff4 100644 --- a/src/plugins/data/public/query/query_string/query_string_manager.mock.ts +++ b/src/plugins/data/public/query/query_string/query_string_manager.mock.ts @@ -28,6 +28,7 @@ * under the License. */ +import { of } from 'rxjs'; import { QueryStringContract } from '.'; import { Query, Dataset } from '../../../common'; import { datasetServiceMock } from './dataset_service/dataset_service.mock'; @@ -44,10 +45,10 @@ const createSetupContractMock = (isEnhancementsEnabled: boolean = false) => { }; const queryStringManagerMock: jest.Mocked = { - getQuery: jest.fn(), + getQuery: jest.fn().mockReturnValue(defaultQuery), setQuery: jest.fn(), - getUpdates$: jest.fn(), - getDefaultQuery: jest.fn(), + getUpdates$: jest.fn().mockReturnValue(of(defaultQuery)), + getDefaultQuery: jest.fn().mockReturnValue(defaultQuery), formatQuery: jest.fn(), clearQuery: jest.fn(), addToQueryHistory: jest.fn(), diff --git a/src/plugins/query_enhancements/public/plugin.tsx b/src/plugins/query_enhancements/public/plugin.tsx index b6578e1e8fc3..acd3e6f75109 100644 --- a/src/plugins/query_enhancements/public/plugin.tsx +++ b/src/plugins/query_enhancements/public/plugin.tsx @@ -4,7 +4,6 @@ */ import { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from '../../../core/public'; -import { IStorageWrapper, Storage } from '../../opensearch_dashboards_utils/public'; import { ConfigSchema } from '../common/config'; import { setData, setStorage } from './services'; import { createQueryAssistExtension } from './query_assist'; @@ -19,6 +18,7 @@ import { LanguageConfig, Query } from '../../data/public'; import { s3TypeConfig } from './datasets'; import { createEditor, DefaultInput, SingleLineInput } from '../../data/public'; import { QueryLanguageReference } from './query_editor/query_language_reference'; +import { DataStorage } from '../../data/common'; export class QueryEnhancementsPlugin implements @@ -28,12 +28,12 @@ export class QueryEnhancementsPlugin QueryEnhancementsPluginSetupDependencies, QueryEnhancementsPluginStartDependencies > { - private readonly storage: IStorageWrapper; + private readonly storage: DataStorage; private readonly config: ConfigSchema; constructor(initializerContext: PluginInitializerContext) { this.config = initializerContext.config.get(); - this.storage = new Storage(window.localStorage); + this.storage = new DataStorage(window.localStorage, 'discover.queryAssist.'); } public setup( diff --git a/src/plugins/query_enhancements/public/query_assist/_index.scss b/src/plugins/query_enhancements/public/query_assist/_index.scss index 65ed6aa60884..9094e54afa43 100644 --- a/src/plugins/query_enhancements/public/query_assist/_index.scss +++ b/src/plugins/query_enhancements/public/query_assist/_index.scss @@ -7,6 +7,31 @@ &.queryAssist__callout { margin-top: $euiSizeXS; } + + .queryAssist__popoverWrapper { + display: flex; + align-items: center; + padding-right: $euiSizeS; + } + + // Hide the error badge when the input is focused + .queryAssist__inputWrapper:has(.queryAssist__input:focus) .queryAssist__popoverWrapper { + display: none; + } + + .queryAssist__input { + text-overflow: ellipsis; + } +} + +.queryAssist__popoverText { + padding: 0 $euiSizeM; +} + +#queryAssistErrorTitle { + // there is a medium size icon with two extra-small gutter between the icon + // and the title pushing to the left so title is aligned with other content + margin-left: -($euiSizeM + $euiSizeS); } .osdQueryEditorExtensionComponent__query-assist { diff --git a/src/plugins/query_enhancements/public/query_assist/components/__snapshots__/query_assist_bar.test.tsx.snap b/src/plugins/query_enhancements/public/query_assist/components/__snapshots__/query_assist_bar.test.tsx.snap new file mode 100644 index 000000000000..5255a51f4b5b --- /dev/null +++ b/src/plugins/query_enhancements/public/query_assist/components/__snapshots__/query_assist_bar.test.tsx.snap @@ -0,0 +1,47 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`QueryAssistBar matches snapshot 1`] = ` +
+
+
+
+
+
+ +
+
+
+ +
+
+
+
+ +
+`; diff --git a/src/plugins/query_enhancements/public/query_assist/components/call_outs.test.tsx b/src/plugins/query_enhancements/public/query_assist/components/call_outs.test.tsx index 4ac107d49c6b..6cb2f55b5a29 100644 --- a/src/plugins/query_enhancements/public/query_assist/components/call_outs.test.tsx +++ b/src/plugins/query_enhancements/public/query_assist/components/call_outs.test.tsx @@ -4,13 +4,13 @@ */ import { render } from '@testing-library/react'; -import React, { ComponentProps } from 'react'; +import React, { ComponentProps, PropsWithChildren } from 'react'; import { IntlProvider } from 'react-intl'; import { QueryAssistCallOut } from './call_outs'; type Props = ComponentProps; -const IntlWrapper = ({ children }: { children: unknown }) => ( +const IntlWrapper = ({ children }: PropsWithChildren) => ( {children} ); diff --git a/src/plugins/query_enhancements/public/query_assist/components/query_assist_bar.test.tsx b/src/plugins/query_enhancements/public/query_assist/components/query_assist_bar.test.tsx new file mode 100644 index 000000000000..44036a11c859 --- /dev/null +++ b/src/plugins/query_enhancements/public/query_assist/components/query_assist_bar.test.tsx @@ -0,0 +1,218 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import React, { ComponentProps, PropsWithChildren } from 'react'; +import { IntlProvider } from 'react-intl'; +import { of } from 'rxjs'; +import { QueryAssistBar } from '.'; +import { notificationServiceMock, uiSettingsServiceMock } from '../../../../../core/public/mocks'; +import { DataStorage } from '../../../../data/common'; +import { QueryEditorExtensionDependencies, QueryStringContract } from '../../../../data/public'; +import { dataPluginMock } from '../../../../data/public/mocks'; +import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public'; +import { setData, setStorage } from '../../services'; +import { useGenerateQuery } from '../hooks'; +import { AgentError, ProhibitedQueryError } from '../utils'; +import { QueryAssistInput } from './query_assist_input'; + +jest.mock('../../../../opensearch_dashboards_react/public', () => ({ + useOpenSearchDashboards: jest.fn(), + withOpenSearchDashboards: jest.fn((component: React.Component) => component), +})); + +jest.mock('../hooks', () => ({ + useGenerateQuery: jest.fn().mockReturnValue({ generateQuery: jest.fn(), loading: false }), +})); + +jest.mock('./query_assist_input', () => ({ + QueryAssistInput: ({ inputRef, error }: ComponentProps) => ( + <> + +
{JSON.stringify(error)}
+ + ), +})); + +const dataMock = dataPluginMock.createStartContract(true); +const queryStringMock = dataMock.query.queryString as jest.Mocked; +const uiSettingsMock = uiSettingsServiceMock.createStartContract(); +const notificationsMock = notificationServiceMock.createStartContract(); + +setData(dataMock); +setStorage(new DataStorage(window.localStorage, 'mock-prefix')); + +const dependencies: QueryEditorExtensionDependencies = { + language: 'PPL', + onSelectLanguage: jest.fn(), + isCollapsed: false, + setIsCollapsed: jest.fn(), +}; + +type Props = ComponentProps; + +const IntlWrapper = ({ children }: PropsWithChildren) => ( + {children} +); + +const renderQueryAssistBar = (overrideProps: Partial = {}) => { + const props: Props = Object.assign>({ dependencies }, overrideProps); + const component = render(, { + wrapper: IntlWrapper, + }); + return { component, props: props as jest.MockedObjectDeep }; +}; + +describe('QueryAssistBar', () => { + beforeEach(() => { + (useOpenSearchDashboards as jest.Mock).mockReturnValue({ + services: { + data: dataMock, + uiSettings: uiSettingsMock, + notifications: notificationsMock, + }, + }); + }); + afterEach(() => { + jest.clearAllMocks(); + }); + + it('renders null if collapsed', () => { + const { component } = renderQueryAssistBar({ + dependencies: { ...dependencies, isCollapsed: true }, + }); + expect(component.container).toBeEmptyDOMElement(); + }); + + it('matches snapshot', () => { + const { component } = renderQueryAssistBar(); + expect(component.container).toMatchSnapshot(); + }); + + it('displays callout when query input is empty on submit', async () => { + renderQueryAssistBar(); + + fireEvent.click(screen.getByRole('button')); + + await waitFor(() => { + expect(screen.getByTestId('query-assist-empty-query-callout')).toBeInTheDocument(); + }); + }); + + it('displays callout when dataset is not selected on submit', async () => { + queryStringMock.getQuery.mockReturnValueOnce({ query: '', language: 'kuery' }); + queryStringMock.getUpdates$.mockReturnValueOnce(of({ query: '', language: 'kuery' })); + renderQueryAssistBar(); + + fireEvent.change(screen.getByRole('textbox'), { target: { value: 'test query' } }); + fireEvent.click(screen.getByRole('button')); + + await waitFor(() => { + expect(screen.getByTestId('query-assist-empty-index-callout')).toBeInTheDocument(); + }); + }); + + it('displays callout for guardrail errors', async () => { + const generateQueryMock = jest.fn().mockResolvedValue({ error: new ProhibitedQueryError() }); + (useGenerateQuery as jest.Mock).mockReturnValue({ + generateQuery: generateQueryMock, + loading: false, + }); + + renderQueryAssistBar(); + + fireEvent.change(screen.getByRole('textbox'), { target: { value: 'test query' } }); + fireEvent.click(screen.getByRole('button')); + + await waitFor(() => { + expect(screen.getByTestId('query-assist-guard-callout')).toBeInTheDocument(); + }); + }); + + it('passes agent errors to input', async () => { + const generateQueryMock = jest.fn().mockResolvedValue({ + error: new AgentError({ + error: { type: 'mock-type', reason: 'mock-reason', details: 'mock-details' }, + status: 303, + }), + }); + (useGenerateQuery as jest.Mock).mockReturnValue({ + generateQuery: generateQueryMock, + loading: false, + }); + + renderQueryAssistBar(); + + fireEvent.change(screen.getByRole('textbox'), { target: { value: 'test query' } }); + fireEvent.click(screen.getByRole('button')); + + await waitFor(() => { + expect(screen.getByText(/mock-reason/)).toBeInTheDocument(); + }); + }); + + it('displays toast for other unknown errors', async () => { + const mockError = new Error('mock-error'); + const generateQueryMock = jest.fn().mockResolvedValue({ + error: mockError, + }); + (useGenerateQuery as jest.Mock).mockReturnValue({ + generateQuery: generateQueryMock, + loading: false, + }); + + renderQueryAssistBar(); + + fireEvent.change(screen.getByRole('textbox'), { target: { value: 'test query' } }); + fireEvent.click(screen.getByRole('button')); + + await waitFor(() => { + expect(notificationsMock.toasts.addError).toHaveBeenCalledWith(mockError, { + title: 'Failed to generate results', + }); + }); + }); + + it('submits a valid query and updates services', async () => { + const generateQueryMock = jest + .fn() + .mockResolvedValue({ response: { query: 'generated query' } }); + (useGenerateQuery as jest.Mock).mockReturnValue({ + generateQuery: generateQueryMock, + loading: false, + }); + + renderQueryAssistBar(); + + fireEvent.change(screen.getByRole('textbox'), { target: { value: 'test query' } }); + fireEvent.click(screen.getByRole('button')); + + await waitFor(() => { + expect(generateQueryMock).toHaveBeenCalledWith({ + question: 'test query', + index: 'Default Index Pattern', + language: 'PPL', + dataSourceId: 'mock-data-source-id', + }); + }); + + expect(queryStringMock.setQuery).toHaveBeenCalledWith({ + dataset: { + dataSource: { + id: 'mock-data-source-id', + title: 'Default Data Source', + type: 'OpenSearch', + }, + id: 'default-index-pattern', + timeFieldName: '@timestamp', + title: 'Default Index Pattern', + type: 'INDEX_PATTERN', + }, + language: 'PPL', + query: 'generated query', + }); + expect(screen.getByTestId('query-assist-query-generated-callout')).toBeInTheDocument(); + }); +}); diff --git a/src/plugins/query_enhancements/public/query_assist/components/query_assist_bar.tsx b/src/plugins/query_enhancements/public/query_assist/components/query_assist_bar.tsx index 85f951fc7515..884068070dab 100644 --- a/src/plugins/query_enhancements/public/query_assist/components/query_assist_bar.tsx +++ b/src/plugins/query_enhancements/public/query_assist/components/query_assist_bar.tsx @@ -15,7 +15,7 @@ import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react import { QueryAssistParameters } from '../../../common/query_assist'; import { getStorage } from '../../services'; import { useGenerateQuery } from '../hooks'; -import { getPersistedLog, ProhibitedQueryError } from '../utils'; +import { getPersistedLog, AgentError, ProhibitedQueryError } from '../utils'; import { QueryAssistCallOut, QueryAssistCallOutType } from './call_outs'; import { QueryAssistInput } from './query_assist_input'; import { QueryAssistSubmitButton } from './submit_button'; @@ -36,6 +36,7 @@ export const QueryAssistBar: React.FC = (props) => { const { generateQuery, loading } = useGenerateQuery(); const [callOutType, setCallOutType] = useState(); const dismissCallout = () => setCallOutType(undefined); + const [agentError, setAgentError] = useState(); const [selectedDataset, setSelectedDataset] = useState( queryString.getQuery().dataset ); @@ -60,6 +61,7 @@ export const QueryAssistBar: React.FC = (props) => { return; } dismissCallout(); + setAgentError(undefined); previousQuestionRef.current = inputRef.current.value; persistedLog.add(inputRef.current.value); const params: QueryAssistParameters = { @@ -72,6 +74,8 @@ export const QueryAssistBar: React.FC = (props) => { if (error) { if (error instanceof ProhibitedQueryError) { setCallOutType('invalid_query'); + } else if (error instanceof AgentError) { + setAgentError(error); } else { services.notifications.toasts.addError(error, { title: 'Failed to generate results' }); } @@ -91,7 +95,7 @@ export const QueryAssistBar: React.FC = (props) => { return ( - + = (props) => { isDisabled={loading} selectedIndex={selectedIndex} previousQuestion={previousQuestionRef.current} + error={agentError} /> diff --git a/src/plugins/query_enhancements/public/query_assist/components/query_assist_input.test.tsx b/src/plugins/query_enhancements/public/query_assist/components/query_assist_input.test.tsx index 1e6eb89da1f8..fa2c393e9b9a 100644 --- a/src/plugins/query_enhancements/public/query_assist/components/query_assist_input.test.tsx +++ b/src/plugins/query_enhancements/public/query_assist/components/query_assist_input.test.tsx @@ -4,9 +4,10 @@ */ import { I18nProvider } from '@osd/i18n/react'; -import { fireEvent, render } from '@testing-library/react'; +import { fireEvent, render, screen } from '@testing-library/react'; import React, { ComponentProps } from 'react'; import { SuggestionsComponentProps } from '../../../../data/public/ui/typeahead/suggestions_component'; +import { AgentError } from '../utils'; import { QueryAssistInput } from './query_assist_input'; jest.mock('../../services', () => ({ @@ -73,4 +74,14 @@ describe(' spec', () => { fireEvent.click(suggestionButton); expect(inputElement.value).toBe('mock suggestion 1'); }); + + it('should show error badge if there is an error', async () => { + renderQueryAssistInput({ + error: new AgentError({ + error: { type: 'mock-type', reason: 'mock-reason', details: 'mock-details' }, + status: 303, + }), + }); + expect(screen.getByTestId('queryAssistErrorBadge')).toBeInTheDocument(); + }); }); diff --git a/src/plugins/query_enhancements/public/query_assist/components/query_assist_input.tsx b/src/plugins/query_enhancements/public/query_assist/components/query_assist_input.tsx index 5ea5caaa5f49..26baec03c408 100644 --- a/src/plugins/query_enhancements/public/query_assist/components/query_assist_input.tsx +++ b/src/plugins/query_enhancements/public/query_assist/components/query_assist_input.tsx @@ -4,10 +4,13 @@ */ import { EuiFieldText, EuiIcon, EuiOutsideClickDetector, EuiPortal } from '@elastic/eui'; +import { i18n } from '@osd/i18n'; import React, { useMemo, useState } from 'react'; import { PersistedLog, QuerySuggestionTypes } from '../../../../data/public'; import assistantMark from '../../assets/query_assist_mark.svg'; import { getData } from '../../services'; +import { AgentError } from '../utils'; +import { WarningBadge } from './warning_badge'; interface QueryAssistInputProps { inputRef: React.RefObject; @@ -16,6 +19,7 @@ interface QueryAssistInputProps { initialValue?: string; selectedIndex?: string; previousQuestion?: string; + error?: AgentError; } export const QueryAssistInput: React.FC = (props) => { @@ -72,8 +76,9 @@ export const QueryAssistInput: React.FC = (props) => { return ( setIsSuggestionsVisible(false)}> -
+
= (props) => { placeholder={ props.previousQuestion || (props.selectedIndex - ? `Ask a natural language question about ${props.selectedIndex} to generate a query` - : 'Select an index to ask a question') + ? i18n.translate('queryEnhancements.queryAssist.input.placeholderWithIndex', { + defaultMessage: + 'Ask a natural language question about {selectedIndex} to generate a query', + values: { selectedIndex: props.selectedIndex }, + }) + : i18n.translate('queryEnhancements.queryAssist.input.placeholderWithoutIndex', { + defaultMessage: 'Select an index to ask a question', + })) } prepend={} + append={} fullWidth /> diff --git a/src/plugins/query_enhancements/public/query_assist/components/warning_badge.test.tsx b/src/plugins/query_enhancements/public/query_assist/components/warning_badge.test.tsx new file mode 100644 index 000000000000..d9de2f824d92 --- /dev/null +++ b/src/plugins/query_enhancements/public/query_assist/components/warning_badge.test.tsx @@ -0,0 +1,62 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { fireEvent, render, waitFor } from '@testing-library/react'; +import React, { ComponentProps } from 'react'; +import { IntlProvider } from 'react-intl'; +import { WarningBadge } from './warning_badge'; + +type WarningBadgeProps = ComponentProps; + +const renderWarningBadge = (overrideProps: Partial = {}) => { + const props: WarningBadgeProps = Object.assign>( + { + error: { + error: { + error: { + details: 'mock-details', + reason: 'mock-reason', + type: 'mock-type', + }, + status: 303, + }, + name: 'mock-name', + message: 'mock-message', + }, + }, + overrideProps + ); + const component = render( + + + + ); + return { component, props: props as jest.MockedObjectDeep }; +}; + +describe(' spec', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should render null if no error', () => { + const { component } = renderWarningBadge({ error: undefined }); + expect(component.container).toBeEmptyDOMElement(); + }); + + it('should render error details', async () => { + const { component } = renderWarningBadge(); + + fireEvent.click(component.getByTestId('queryAssistErrorBadge')); + await waitFor(() => { + expect(component.getByText(/mock-reason/)).toBeInTheDocument(); + }); + + fireEvent.click(component.getByTestId('queryAssistErrorMore')); + await waitFor(() => { + expect(component.getByText(/mock-details/)).toBeInTheDocument(); + }); + }); +}); diff --git a/src/plugins/query_enhancements/public/query_assist/components/warning_badge.tsx b/src/plugins/query_enhancements/public/query_assist/components/warning_badge.tsx new file mode 100644 index 000000000000..53b1c5944480 --- /dev/null +++ b/src/plugins/query_enhancements/public/query_assist/components/warning_badge.tsx @@ -0,0 +1,145 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + EuiBadge, + EuiFlexGroup, + EuiFlexItem, + EuiIcon, + EuiLink, + EuiPopover, + EuiText, +} from '@elastic/eui'; +import { i18n } from '@osd/i18n'; +import { FormattedMessage } from '@osd/i18n/react'; +import React, { SyntheticEvent, useEffect, useState } from 'react'; +import { AgentError } from '../utils'; + +interface WarningBadgeProps { + error: AgentError | undefined; +} + +export const WarningBadge: React.FC = (props) => { + const [isPopoverOpen, setIsPopoverOpen] = useState(false); + const [showMore, setShowMore] = useState(false); + + useEffect(() => { + const badgeContainer = document.querySelector('.queryAssist__badgeContainer'); + if (badgeContainer === null) return; + const elementsWithBadge = document.querySelectorAll('.queryAssist__withBadge'); + elementsWithBadge.forEach((element) => + element.style.setProperty('--badge-width', `${badgeContainer.offsetWidth}px`) + ); + }, [props.error]); + + if (!props.error) return null; + const { + error: { error, status }, + } = props.error; + + return ( +
+ { + e.preventDefault(); + setIsPopoverOpen(!isPopoverOpen); + }} + onClickAriaLabel={i18n.translate('queryEnhancements.queryAssist.badge.ariaLabel', { + defaultMessage: 'Click to show details', + })} + data-test-subj="queryAssistErrorBadge" + > + + + } + isOpen={isPopoverOpen} + closePopover={() => setIsPopoverOpen(false)} + className="queryAssist__popover" + > + +
+
+ + + + + + + + + + +
+
+ + + + : {error.reason} +
+ {showMore && ( + <> +
+ + + + : {error.details} +
+
+ + + + : {error.type} +
+
+ + + + : {status} +
+ + )} + setShowMore(!showMore)} data-test-subj="queryAssistErrorMore"> + {showMore ? ( + + ) : ( + + )} + +
+ + +
+ ); +}; diff --git a/src/plugins/query_enhancements/public/query_assist/utils/create_extension.tsx b/src/plugins/query_enhancements/public/query_assist/utils/create_extension.tsx index 2b86245cc43d..701a7ca94e7e 100644 --- a/src/plugins/query_enhancements/public/query_assist/utils/create_extension.tsx +++ b/src/plugins/query_enhancements/public/query_assist/utils/create_extension.tsx @@ -65,8 +65,9 @@ const getAvailableLanguages$ = (http: HttpSetup, data: DataPublicPluginSetup) => // currently query assist tool relies on opensearch API to get index // mappings, external data source types (e.g. s3) are not supported if ( - query.dataset?.dataSource?.type !== DEFAULT_DATA.SOURCE_TYPES.OPENSEARCH && - query.dataset?.dataSource?.type !== 'DATA_SOURCE' + query.dataset?.dataSource?.type !== DEFAULT_DATA.SOURCE_TYPES.OPENSEARCH && // datasource is MDS OpenSearch + query.dataset?.dataSource?.type !== 'DATA_SOURCE' && // datasource is MDS OpenSearch when using indexes + query.dataset?.type !== DEFAULT_DATA.SET_TYPES.INDEX_PATTERN // dataset is index pattern ) return []; diff --git a/src/plugins/query_enhancements/public/query_assist/utils/errors.test.ts b/src/plugins/query_enhancements/public/query_assist/utils/errors.test.ts index 1adbae11b462..df34cea89829 100644 --- a/src/plugins/query_enhancements/public/query_assist/utils/errors.test.ts +++ b/src/plugins/query_enhancements/public/query_assist/utils/errors.test.ts @@ -5,7 +5,7 @@ import { ResponseError } from '@opensearch-project/opensearch/lib/errors'; import { ERROR_DETAILS } from '../../../common'; -import { formatError, ProhibitedQueryError } from './errors'; +import { AgentError, formatError, ProhibitedQueryError } from './errors'; describe('formatError', () => { it('should return an error with a custom message for status code 429', () => { @@ -42,6 +42,24 @@ describe('formatError', () => { expect(formattedError.message).toEqual(error.body.message); }); + it('should return an AgentError if possible', () => { + const error = new ResponseError({ + statusCode: 400, + body: { + statusCode: 400, + message: `{"error":{"reason":"Invalid Request","details":"PPLTool doesn't support searching indexes starting with '.' since it could be system index, current searching index name: .opensearch-sap-log-types-config","type":"IllegalArgumentException"},"status":400}`, + }, + warnings: [], + headers: null, + meta: {} as any, + }); + const formattedError = formatError(error); + expect(formattedError).toBeInstanceOf(AgentError); + expect((formattedError as AgentError).error.error.details).toEqual( + "PPLTool doesn't support searching indexes starting with '.' since it could be system index, current searching index name: .opensearch-sap-log-types-config" + ); + }); + it('should return the original error body for other errors', () => { const error = new ResponseError({ statusCode: 500, diff --git a/src/plugins/query_enhancements/public/query_assist/utils/errors.ts b/src/plugins/query_enhancements/public/query_assist/utils/errors.ts index dbd0a9ef8fc7..f473395b336b 100644 --- a/src/plugins/query_enhancements/public/query_assist/utils/errors.ts +++ b/src/plugins/query_enhancements/public/query_assist/utils/errors.ts @@ -3,6 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +// eslint-disable-next-line max-classes-per-file import { ResponseError } from '@opensearch-project/opensearch/lib/errors'; import { ERROR_DETAILS } from '../../../common'; @@ -12,6 +13,33 @@ export class ProhibitedQueryError extends Error { } } +interface IAgentError { + error: { + reason: string; + details: string; + type: string; + }; + status: number; +} + +export class AgentError extends Error { + public readonly error: IAgentError; + constructor(error: IAgentError) { + super(error.error.details); + if ( + !( + 'status' in error && + 'reason' in error.error && + 'details' in error.error && + 'type' in error.error + ) + ) { + throw new Error('Failed to parse error'); + } + this.error = error; + } +} + export const formatError = (error: ResponseError | Error): Error => { if ('body' in error) { if (error.body.statusCode === 429) @@ -24,7 +52,11 @@ export const formatError = (error: ResponseError | Error): Error => { error.body.message.includes(ERROR_DETAILS.GUARDRAILS_TRIGGERED) ) return new ProhibitedQueryError(error.body.message); - return error.body as Error; + try { + return new AgentError(JSON.parse(error.body.message)); + } catch (parseError) { + return error.body as Error; + } } return error; }; diff --git a/src/plugins/query_enhancements/public/query_assist/utils/get_persisted_log.ts b/src/plugins/query_enhancements/public/query_assist/utils/get_persisted_log.ts index c86edcf125c0..17a0f286f377 100644 --- a/src/plugins/query_enhancements/public/query_assist/utils/get_persisted_log.ts +++ b/src/plugins/query_enhancements/public/query_assist/utils/get_persisted_log.ts @@ -4,13 +4,12 @@ */ import { IUiSettingsClient } from 'opensearch-dashboards/public'; -import { UI_SETTINGS } from '../../../../data/common'; +import { DataStorage, UI_SETTINGS } from '../../../../data/common'; import { PersistedLog } from '../../../../data/public'; -import { IStorageWrapper } from '../../../../opensearch_dashboards_utils/public'; export function getPersistedLog( uiSettings: IUiSettingsClient, - storage: IStorageWrapper, + storage: DataStorage, language: string ) { return new PersistedLog( diff --git a/src/plugins/query_enhancements/public/services/index.ts b/src/plugins/query_enhancements/public/services/index.ts index bb0284408faa..23071cdbc86f 100644 --- a/src/plugins/query_enhancements/public/services/index.ts +++ b/src/plugins/query_enhancements/public/services/index.ts @@ -3,11 +3,11 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { createGetterSetter } from '../../../opensearch_dashboards_utils/common'; -import { IStorageWrapper } from '../../../opensearch_dashboards_utils/public'; +import { DataStorage } from '../../../data/common'; import { DataPublicPluginStart } from '../../../data/public'; +import { createGetterSetter } from '../../../opensearch_dashboards_utils/common'; -export const [getStorage, setStorage] = createGetterSetter('storage'); +export const [getStorage, setStorage] = createGetterSetter('storage'); export const [getData, setData] = createGetterSetter('data'); export { ConnectionsService } from './connections_service'; diff --git a/src/plugins/query_enhancements/server/routes/query_assist/routes.ts b/src/plugins/query_enhancements/server/routes/query_assist/routes.ts index d30396719533..07e48961dd81 100644 --- a/src/plugins/query_enhancements/server/routes/query_assist/routes.ts +++ b/src/plugins/query_enhancements/server/routes/query_assist/routes.ts @@ -78,13 +78,16 @@ export function registerQueryAssistRoutes(router: IRouter) { return response.ok({ body: responseBody }); } catch (error) { if (isResponseError(error)) { - if (error.statusCode === 400 && error.body.includes(ERROR_DETAILS.GUARDRAILS_TRIGGERED)) + if ( + error.statusCode === 400 && + // on opensearch >= 2.17, error.body is an object https://github.com/opensearch-project/ml-commons/pull/2858 + JSON.stringify(error.body).includes(ERROR_DETAILS.GUARDRAILS_TRIGGERED) + ) return response.badRequest({ body: ERROR_DETAILS.GUARDRAILS_TRIGGERED }); - return response.badRequest({ - body: - typeof error.meta.body === 'string' - ? error.meta.body - : JSON.stringify(error.meta.body), + return response.custom({ + statusCode: error.statusCode, + // for consistency, frontend will always receive the actual error in error.body.message as a JSON string + body: typeof error.body === 'string' ? error.body : JSON.stringify(error.body), }); } return response.custom({ statusCode: error.statusCode || 500, body: error.message }); diff --git a/src/plugins/vis_builder/public/application/utils/get_top_nav_config.test.tsx b/src/plugins/vis_builder/public/application/utils/get_top_nav_config.test.tsx index 536596cc9e1f..a911982f638b 100644 --- a/src/plugins/vis_builder/public/application/utils/get_top_nav_config.test.tsx +++ b/src/plugins/vis_builder/public/application/utils/get_top_nav_config.test.tsx @@ -97,7 +97,10 @@ describe('getOnSave', () => { }, "searchSourceFields": Object { "filter": null, - "query": null, + "query": Object { + "language": "kuery", + "query": "", + }, }, "styleState": "", "title": "new title",