diff --git a/public/app/core/utils/explore.test.ts b/public/app/core/utils/explore.test.ts index 156b59b035d..2b65bb9e309 100644 --- a/public/app/core/utils/explore.test.ts +++ b/public/app/core/utils/explore.test.ts @@ -2,7 +2,6 @@ import { buildQueryTransaction, clearHistory, DEFAULT_RANGE, - getFirstQueryErrorWithoutRefId, getRefIds, getValueWithRefId, hasNonEmptyQuery, @@ -14,7 +13,7 @@ import { getTimeRangeFromUrl, } from './explore'; import store from 'app/core/store'; -import { DataQueryError, dateTime, ExploreUrlState, LogsSortOrder } from '@grafana/data'; +import { dateTime, ExploreUrlState, LogsSortOrder } from '@grafana/data'; import { RefreshPicker } from '@grafana/ui'; import { serializeStateToUrlParam } from '@grafana/data/src/utils/url'; @@ -323,40 +322,6 @@ describe('getTimeRangeFromUrl', () => { }); }); -describe('getFirstQueryErrorWithoutRefId', () => { - describe('when called with a null value', () => { - it('then it should return undefined', () => { - const errors: DataQueryError[] | undefined = undefined; - const result = getFirstQueryErrorWithoutRefId(errors); - - expect(result).toBeUndefined(); - }); - }); - - describe('when called with an array with only refIds', () => { - it('then it should return undefined', () => { - const errors: DataQueryError[] = [{ refId: 'A' }, { refId: 'B' }]; - const result = getFirstQueryErrorWithoutRefId(errors); - - expect(result).toBeUndefined(); - }); - }); - - describe('when called with an array with and without refIds', () => { - it('then it should return undefined', () => { - const errors: DataQueryError[] = [ - { refId: 'A' }, - { message: 'A message' }, - { refId: 'B' }, - { message: 'B message' }, - ]; - const result = getFirstQueryErrorWithoutRefId(errors); - - expect(result).toBe(errors[1]); - }); - }); -}); - describe('getRefIds', () => { describe('when called with a null value', () => { it('then it should return empty array', () => { diff --git a/public/app/core/utils/explore.ts b/public/app/core/utils/explore.ts index 6bdf4b0a8ed..da1a0817149 100644 --- a/public/app/core/utils/explore.ts +++ b/public/app/core/utils/explore.ts @@ -5,7 +5,6 @@ import { Unsubscribable } from 'rxjs'; import { CoreApp, DataQuery, - DataQueryError, DataQueryRequest, DataSourceApi, dateMath, @@ -422,14 +421,6 @@ export const getValueWithRefId = (value?: any): any => { return undefined; }; -export const getFirstQueryErrorWithoutRefId = (errors?: DataQueryError[]): DataQueryError | undefined => { - if (!errors) { - return undefined; - } - - return errors.filter((error) => (error && error.refId ? false : true))[0]; -}; - export const getRefIds = (value: any): string[] => { if (!value) { return []; @@ -479,11 +470,6 @@ export function getIntervals(range: TimeRange, lowLimit?: string, resolution?: n return rangeUtil.calculateInterval(range, resolution, lowLimit); } -export const getFirstNonQueryRowSpecificError = (queryErrors?: DataQueryError[]): DataQueryError | undefined => { - const refId = getValueWithRefId(queryErrors); - return refId ? undefined : getFirstQueryErrorWithoutRefId(queryErrors); -}; - export const copyStringToClipboard = (string: string) => { const el = document.createElement('textarea'); el.value = string; diff --git a/public/app/features/explore/ErrorContainer.tsx b/public/app/features/explore/ErrorContainer.tsx index 332ef98cb3a..9acd6f5efb9 100644 --- a/public/app/features/explore/ErrorContainer.tsx +++ b/public/app/features/explore/ErrorContainer.tsx @@ -11,7 +11,7 @@ export const ErrorContainer: FunctionComponent = (props) => const { queryError } = props; const showError = queryError ? true : false; const duration = showError ? 100 : 10; - const message = queryError ? queryError.message : null; + const message = queryError?.message || queryError?.data?.message || 'Unknown error'; return ( diff --git a/public/app/features/explore/Explore.test.tsx b/public/app/features/explore/Explore.test.tsx index 921127e870e..916d957a0d9 100644 --- a/public/app/features/explore/Explore.test.tsx +++ b/public/app/features/explore/Explore.test.tsx @@ -1,6 +1,5 @@ import React from 'react'; import { DataSourceApi, LoadingState, toUtc, DataQueryError, DataQueryRequest, CoreApp } from '@grafana/data'; -import { getFirstNonQueryRowSpecificError } from 'app/core/utils/explore'; import { ExploreId } from 'app/types/explore'; import { shallow } from 'enzyme'; import { Explore, ExploreProps } from './Explore'; @@ -85,17 +84,6 @@ const dummyProps: ExploreProps = { splitOpen: (() => {}) as any, }; -const setupErrors = (hasRefId?: boolean) => { - return [ - { - message: 'Error message', - status: '400', - statusText: 'Bad Request', - refId: hasRefId ? 'A' : '', - }, - ]; -}; - describe('Explore', () => { it('should render component', () => { const wrapper = shallow(); @@ -107,21 +95,4 @@ describe('Explore', () => { expect(wrapper.find(SecondaryActions)).toHaveLength(1); expect(wrapper.find(SecondaryActions).props().addQueryRowButtonHidden).toBe(false); }); - - it('should filter out a query-row-specific error when looking for non-query-row-specific errors', async () => { - const queryErrors = setupErrors(true); - const queryError = getFirstNonQueryRowSpecificError(queryErrors); - expect(queryError).toBeUndefined(); - }); - - it('should not filter out a generic error when looking for non-query-row-specific errors', async () => { - const queryErrors = setupErrors(); - const queryError = getFirstNonQueryRowSpecificError(queryErrors); - expect(queryError).toEqual({ - message: 'Error message', - status: '400', - statusText: 'Bad Request', - refId: '', - }); - }); }); diff --git a/public/app/features/explore/Explore.tsx b/public/app/features/explore/Explore.tsx index 5f42faa061a..da71073a882 100644 --- a/public/app/features/explore/Explore.tsx +++ b/public/app/features/explore/Explore.tsx @@ -31,17 +31,15 @@ import { updateTimeRange } from './state/time'; import { scanStopAction, addQueryRow, modifyQueries, setQueries, scanStart } from './state/query'; import { ExploreId, ExploreItemState } from 'app/types/explore'; import { StoreState } from 'app/types'; -import { getFirstNonQueryRowSpecificError } from 'app/core/utils/explore'; import { ExploreToolbar } from './ExploreToolbar'; import { NoDataSourceCallToAction } from './NoDataSourceCallToAction'; import { getTimeZone } from '../profile/state/selectors'; -import { ErrorContainer } from './ErrorContainer'; -//TODO:unification import { TraceView } from './TraceView/TraceView'; import { SecondaryActions } from './SecondaryActions'; import { FILTER_FOR_OPERATOR, FILTER_OUT_OPERATOR, FilterItem } from '@grafana/ui/src/components/Table/types'; import { ExploreGraphNGPanel } from './ExploreGraphNGPanel'; import { NodeGraphContainer } from './NodeGraphContainer'; +import { ResponseErrorContainer } from './ResponseErrorContainer'; const getStyles = stylesFactory((theme: GrafanaTheme) => { return { @@ -315,12 +313,6 @@ export class Explore extends React.PureComponent { const { openDrawer } = this.state; const styles = getStyles(theme); const showPanels = queryResponse && queryResponse.state !== LoadingState.NotStarted; - - // gets an error without a refID, so non-query-row-related error, like a connection error - const queryErrors = - queryResponse.state === LoadingState.Error && queryResponse.error ? [queryResponse.error] : undefined; - const queryError = getFirstNonQueryRowSpecificError(queryErrors); - const showRichHistory = openDrawer === ExploreDrawer.RichHistory; const showQueryInspector = openDrawer === ExploreDrawer.QueryInspector; @@ -344,7 +336,7 @@ export class Explore extends React.PureComponent { onClickQueryInspectorButton={this.toggleShowQueryInspector} /> - + {({ width }) => { if (width === 0) { diff --git a/public/app/features/explore/QueryRow.tsx b/public/app/features/explore/QueryRow.tsx index 1ef490558bc..2531977bff0 100644 --- a/public/app/features/explore/QueryRow.tsx +++ b/public/app/features/explore/QueryRow.tsx @@ -177,6 +177,8 @@ export class QueryRow extends PureComponent { const canToggleEditorModes = has(datasourceInstance, 'components.QueryCtrl.prototype.toggleEditorMode'); const isNotStarted = queryResponse.state === LoadingState.NotStarted; + + // We show error without refId in ResponseErrorContainer so this condition needs to match se we don't loose errors. const queryErrors = queryResponse.error && queryResponse.error.refId === query.refId ? [queryResponse.error] : []; return ( diff --git a/public/app/features/explore/ResponseErrorContainer.test.tsx b/public/app/features/explore/ResponseErrorContainer.test.tsx new file mode 100644 index 00000000000..a4372358659 --- /dev/null +++ b/public/app/features/explore/ResponseErrorContainer.test.tsx @@ -0,0 +1,56 @@ +import React from 'react'; +import { configureStore } from '../../store/configureStore'; +import { ResponseErrorContainer } from './ResponseErrorContainer'; +import { Provider } from 'react-redux'; +import { render, screen } from '@testing-library/react'; +import { ExploreId } from '../../types'; +import { DataQueryError, LoadingState } from '@grafana/data'; + +describe('ResponseErrorContainer', () => { + it('shows error message if it does not contain refId', async () => { + setup({ + message: 'test error', + }); + expect(screen.getByText('test error')).toBeInTheDocument(); + }); + + it('shows error.data.message if error.message does not exist', async () => { + setup({ + data: { + message: 'test error', + }, + }); + expect(screen.getByText('test error')).toBeInTheDocument(); + }); + + it('does not show error if there is refID', async () => { + setup({ + refId: 'someId', + message: 'test error', + }); + expect(screen.queryByText('test error')).not.toBeInTheDocument(); + }); + + it('does not show error if there is refID', async () => { + setup({ + refId: 'someId', + message: 'test error', + }); + expect(screen.queryByText('test error')).not.toBeInTheDocument(); + }); +}); + +function setup(error: DataQueryError) { + const store = configureStore(); + store.getState().explore[ExploreId.left].queryResponse = { + timeRange: {} as any, + series: [], + state: LoadingState.Error, + error, + }; + render( + + + + ); +} diff --git a/public/app/features/explore/ResponseErrorContainer.tsx b/public/app/features/explore/ResponseErrorContainer.tsx new file mode 100644 index 00000000000..f144180b689 --- /dev/null +++ b/public/app/features/explore/ResponseErrorContainer.tsx @@ -0,0 +1,21 @@ +import React from 'react'; +import { useSelector } from 'react-redux'; +import { ExploreId, StoreState } from '../../types'; +import { LoadingState } from '@grafana/data'; +import { ErrorContainer } from './ErrorContainer'; + +interface Props { + exploreId: ExploreId; +} +export function ResponseErrorContainer(props: Props) { + const queryResponse = useSelector((state: StoreState) => state.explore[props.exploreId]?.queryResponse); + + // Only show error if it does not have refId. Otherwise let query row to handle it so this condition has to be matched + // with QueryRow.tsx so we don't loose errors. + const queryError = + queryResponse?.state === LoadingState.Error && queryResponse?.error && !queryResponse.error.refId + ? queryResponse.error + : undefined; + + return ; +} diff --git a/public/app/features/explore/__snapshots__/Explore.test.tsx.snap b/public/app/features/explore/__snapshots__/Explore.test.tsx.snap index 5898b8cd187..4b33a28eb09 100644 --- a/public/app/features/explore/__snapshots__/Explore.test.tsx.snap +++ b/public/app/features/explore/__snapshots__/Explore.test.tsx.snap @@ -26,7 +26,9 @@ exports[`Explore should render component 1`] = ` richHistoryButtonActive={false} /> - +