From 15aae5e8a92604712531ac0e77cee6f3dea8e877 Mon Sep 17 00:00:00 2001 From: Andres Martinez Gotor Date: Wed, 8 Mar 2023 16:11:38 +0100 Subject: [PATCH] Panel: Show multiple errors info in the inspector (#64340) --- .../src/utils/queryResponse.test.ts | 3 + .../src/utils/queryResponse.ts | 6 +- .../components/Inspector/InspectContent.tsx | 7 ++- .../PanelEditor/PanelEditorTableView.test.tsx | 61 ++++++++++++++++++- .../PanelEditor/PanelEditorTableView.tsx | 8 ++- .../dashgrid/PanelStateWrapper.test.tsx | 43 ++++++++++++- .../dashboard/dashgrid/PanelStateWrapper.tsx | 10 ++- .../explore/ExploreQueryInspector.tsx | 9 ++- .../inspector/InspectErrorTab.test.tsx | 49 +++++++++++++-- .../features/inspector/InspectErrorTab.tsx | 35 ++++++++--- 10 files changed, 207 insertions(+), 24 deletions(-) diff --git a/packages/grafana-runtime/src/utils/queryResponse.test.ts b/packages/grafana-runtime/src/utils/queryResponse.test.ts index 3cd421ac4be..56be442fd27 100644 --- a/packages/grafana-runtime/src/utils/queryResponse.test.ts +++ b/packages/grafana-runtime/src/utils/queryResponse.test.ts @@ -54,6 +54,7 @@ const resWithError = { results: { A: { error: 'Hello Error', + status: 400, frames: [ { schema: { @@ -354,12 +355,14 @@ describe('Query Response parser', () => { { "message": "Hello Error", "refId": "A", + "status": 400, } `); expect(res.errors).toEqual([ { message: 'Hello Error', refId: 'A', + status: 400, }, ]); diff --git a/packages/grafana-runtime/src/utils/queryResponse.ts b/packages/grafana-runtime/src/utils/queryResponse.ts index 902c9b28da7..919c003ef03 100644 --- a/packages/grafana-runtime/src/utils/queryResponse.ts +++ b/packages/grafana-runtime/src/utils/queryResponse.ts @@ -33,6 +33,7 @@ export interface DataResponse { error?: string; refId?: string; frames?: DataFrameJSON[]; + status?: number; // Legacy TSDB format... series?: TimeSeries[]; @@ -86,12 +87,13 @@ export function toDataQueryResponse( rsp.error = { refId: dr.refId, message: dr.error, + status: dr.status, }; } if (rsp.errors) { - rsp.errors.push({ refId: dr.refId, message: dr.error }); + rsp.errors.push({ refId: dr.refId, message: dr.error, status: dr.status }); } else { - rsp.errors = [{ refId: dr.refId, message: dr.error }]; + rsp.errors = [{ refId: dr.refId, message: dr.error, status: dr.status }]; } rsp.state = LoadingState.Error; } diff --git a/public/app/features/dashboard/components/Inspector/InspectContent.tsx b/public/app/features/dashboard/components/Inspector/InspectContent.tsx index a5388e9c753..198dfec4f51 100644 --- a/public/app/features/dashboard/components/Inspector/InspectContent.tsx +++ b/public/app/features/dashboard/components/Inspector/InspectContent.tsx @@ -50,7 +50,10 @@ export const InspectContent = ({ return null; } - const error = data?.error; + let errors = data?.errors; + if (!errors?.length && data?.error) { + errors = [data.error]; + } // Validate that the active tab is actually valid and allowed let activeTab = currentTab; @@ -102,7 +105,7 @@ export const InspectContent = ({ {activeTab === InspectTab.JSON && ( )} - {activeTab === InspectTab.Error && } + {activeTab === InspectTab.Error && } {data && activeTab === InspectTab.Stats && } {data && activeTab === InspectTab.Query && ( panel.refresh()} /> diff --git a/public/app/features/dashboard/components/PanelEditor/PanelEditorTableView.test.tsx b/public/app/features/dashboard/components/PanelEditor/PanelEditorTableView.test.tsx index 1bdc4169ed9..e303ba5671a 100644 --- a/public/app/features/dashboard/components/PanelEditor/PanelEditorTableView.test.tsx +++ b/public/app/features/dashboard/components/PanelEditor/PanelEditorTableView.test.tsx @@ -1,4 +1,4 @@ -import { act, render, screen } from '@testing-library/react'; +import { act, fireEvent, render, screen } from '@testing-library/react'; import React, { FC } from 'react'; import { Provider } from 'react-redux'; import configureMockStore from 'redux-mock-store'; @@ -15,6 +15,7 @@ import { PanelProps, TimeRange, } from '@grafana/data'; +import { selectors } from '@grafana/e2e-selectors'; import { getTimeSrv, TimeSrv, setTimeSrv } from 'app/features/dashboard/services/TimeSrv'; import { PanelQueryRunner } from '../../../query/state/PanelQueryRunner'; @@ -174,6 +175,64 @@ describe('PanelEditorTableView', () => { width: 100, }); }); + + it('should render an error', async () => { + const { rerender, props, subject, store } = setupTestContext({}); + + // only render the panel when loading is done + act(() => { + subject.next({ state: LoadingState.Loading, series: [], timeRange: getDefaultTimeRange() }); + subject.next({ + state: LoadingState.Error, + series: [], + errors: [{ message: 'boom!' }], + timeRange: getDefaultTimeRange(), + }); + }); + + const newProps = { ...props, isInView: true }; + rerender( + + + + ); + + const button = screen.getByRole('button', { name: selectors.components.Panels.Panel.headerCornerInfo('error') }); + expect(button).toBeInTheDocument(); + await act(async () => { + fireEvent.focus(button); + }); + expect(await screen.findByText('boom!')).toBeInTheDocument(); + }); + + it('should render a description for multiple errors', async () => { + const { rerender, props, subject, store } = setupTestContext({}); + + // only render the panel when loading is done + act(() => { + subject.next({ state: LoadingState.Loading, series: [], timeRange: getDefaultTimeRange() }); + subject.next({ + state: LoadingState.Error, + series: [], + errors: [{ message: 'boom 1!' }, { message: 'boom 2!' }], + timeRange: getDefaultTimeRange(), + }); + }); + + const newProps = { ...props, isInView: true }; + rerender( + + + + ); + + const button = screen.getByRole('button', { name: selectors.components.Panels.Panel.headerCornerInfo('error') }); + expect(button).toBeInTheDocument(); + await act(async () => { + fireEvent.focus(button); + }); + expect(await screen.findByText('Multiple errors found. Click for more details')).toBeInTheDocument(); + }); }); const TestPanelComponent: FC = () =>
Plugin Panel to Render
; diff --git a/public/app/features/dashboard/components/PanelEditor/PanelEditorTableView.tsx b/public/app/features/dashboard/components/PanelEditor/PanelEditorTableView.tsx index b116db21f21..904ae31ec5d 100644 --- a/public/app/features/dashboard/components/PanelEditor/PanelEditorTableView.tsx +++ b/public/app/features/dashboard/components/PanelEditor/PanelEditorTableView.tsx @@ -49,11 +49,17 @@ export function PanelEditorTableView({ width, height, panel, dashboard }: Props) if (!data) { return null; } + + const errorMessage = data?.errors + ? data.errors.length > 1 + ? 'Multiple errors found. Click for more details' + : data.errors[0].message + : data?.error?.message; return ( {(innerWidth, innerHeight) => ( <> - + { expect(screen.getByText(/plugin panel to render/i)).toBeInTheDocument(); }); }); + + describe('when there are error(s)', () => { + [ + { errors: [{ message: 'boom!' }], expectedMessage: 'boom!' }, + { + errors: [{ message: 'boom!' }, { message: 'boom2!' }], + expectedMessage: 'Multiple errors found. Click for more details', + }, + ].forEach((scenario) => { + it(`then it should show the error message: ${scenario.expectedMessage}`, async () => { + const { rerender, props, subject, store } = setupTestContext({}); + + act(() => { + subject.next({ state: LoadingState.Loading, series: [], timeRange: getDefaultTimeRange() }); + subject.next({ + state: LoadingState.Error, + series: [], + errors: scenario.errors, + timeRange: getDefaultTimeRange(), + }); + }); + + const newProps = { ...props, isInView: true }; + rerender( + + + + ); + + const button = screen.getByRole('button', { + name: selectors.components.Panels.Panel.headerCornerInfo('error'), + }); + expect(button).toBeInTheDocument(); + await act(async () => { + fireEvent.focus(button); + }); + expect(await screen.findByText(scenario.expectedMessage)).toBeInTheDocument(); + }); + }); + }); }); const TestPanelComponent: FC = () =>
Plugin Panel to Render
; diff --git a/public/app/features/dashboard/dashgrid/PanelStateWrapper.tsx b/public/app/features/dashboard/dashgrid/PanelStateWrapper.tsx index e6d50b68379..6e2c72fc71a 100644 --- a/public/app/features/dashboard/dashgrid/PanelStateWrapper.tsx +++ b/public/app/features/dashboard/dashgrid/PanelStateWrapper.tsx @@ -300,8 +300,14 @@ export class PanelStateWrapper extends PureComponent { } break; case LoadingState.Error: - const { error } = data; - if (error) { + const { error, errors } = data; + if (errors?.length) { + if (errors.length === 1) { + errorMessage = errors[0].message; + } else { + errorMessage = 'Multiple errors found. Click for more details'; + } + } else if (error) { if (errorMessage !== error.message) { errorMessage = error.message; } diff --git a/public/app/features/explore/ExploreQueryInspector.tsx b/public/app/features/explore/ExploreQueryInspector.tsx index 681f7e7a747..6084734bbf8 100644 --- a/public/app/features/explore/ExploreQueryInspector.tsx +++ b/public/app/features/explore/ExploreQueryInspector.tsx @@ -26,7 +26,10 @@ type Props = DispatchProps & ConnectedProps; export function ExploreQueryInspector(props: Props) { const { loading, width, onClose, queryResponse, timeZone } = props; const dataFrames = queryResponse?.series || []; - const error = queryResponse?.error; + let errors = queryResponse?.errors; + if (!errors?.length && queryResponse?.error) { + errors = [queryResponse.error]; + } useEffect(() => { reportInteraction('grafana_explore_query_inspector_opened'); @@ -69,12 +72,12 @@ export function ExploreQueryInspector(props: Props) { }; const tabs = [statsTab, queryTab, jsonTab, dataTab]; - if (error) { + if (errors?.length) { const errorTab: TabConfig = { label: 'Error', value: 'error', icon: 'exclamation-triangle', - content: , + content: , }; tabs.push(errorTab); } diff --git a/public/app/features/inspector/InspectErrorTab.test.tsx b/public/app/features/inspector/InspectErrorTab.test.tsx index d86db092516..222cbb09304 100644 --- a/public/app/features/inspector/InspectErrorTab.test.tsx +++ b/public/app/features/inspector/InspectErrorTab.test.tsx @@ -16,7 +16,7 @@ describe('InspectErrorTab', () => { error: 'my error', }, }; - render(); + render(); expect(screen.getByText('This is an error')).toBeInTheDocument(); expect(screen.getByText('error:')).toBeInTheDocument(); expect(screen.getByText('"my error"')).toBeInTheDocument(); @@ -27,7 +27,7 @@ describe('InspectErrorTab', () => { message: '{ "error": { "code": "BadRequest", "message": "Please provide below info when asking for support.", "details": [] } }', }; - const { container } = render(); + const { container } = render(); expect(container.childElementCount).toEqual(1); expect(screen.getByText('code:')).toBeInTheDocument(); expect(screen.getByText('"BadRequest"')).toBeInTheDocument(); @@ -39,7 +39,7 @@ describe('InspectErrorTab', () => { message: '400 BadRequest, Error from Azure: { "error": { "code": "BadRequest", "message": "Please provide below info when asking for support.", "details": [] } }', }; - const { container } = render(); + const { container } = render(); expect(container.childElementCount).toEqual(2); expect(screen.getByRole('heading', { name: '400 BadRequest, Error from Azure:' })).toBeInTheDocument(); expect(screen.getByText('code:')).toBeInTheDocument(); @@ -55,7 +55,7 @@ describe('InspectErrorTab', () => { const error = { message: errMsg, }; - render(); + render(); expect(screen.queryByRole('heading')).toBeNull(); expect(screen.getByText(errMsg)).toBeInTheDocument(); }); @@ -65,9 +65,48 @@ describe('InspectErrorTab', () => { const error = { status: 400, }; - const { container } = render(); + const { container } = render(); expect(container.childElementCount).toEqual(1); expect(screen.getByText('status:')).toBeInTheDocument(); expect(screen.getByText('400')).toBeInTheDocument(); }); + + it('should return a message along with a status', () => { + const error = { + status: 400, + message: 'This is an error', + }; + render(); + expect(screen.getByText(/This is an error/)).toBeInTheDocument(); + expect(screen.getByText(/Status: 400/)).toBeInTheDocument(); + }); + + it('should return a JSON encoded object along with a status', () => { + const error = { + status: 400, + message: + '{ "error": { "code": "BadRequest", "message": "Please provide below info when asking for support.", "details": [] } }', + }; + render(); + expect(screen.getByText('"BadRequest"')).toBeInTheDocument(); + expect(screen.getByText(/Status: 400/)).toBeInTheDocument(); + }); + + it('should return multiple errors', () => { + const errors = [ + { + status: 400, + message: 'This is one error', + }, + { + status: 401, + message: 'This is another error', + }, + ]; + render(); + expect(screen.getByText(/This is one error/)).toBeInTheDocument(); + expect(screen.getByText(/Status: 400/)).toBeInTheDocument(); + expect(screen.getByText(/This is another error/)).toBeInTheDocument(); + expect(screen.getByText(/Status: 401/)).toBeInTheDocument(); + }); }); diff --git a/public/app/features/inspector/InspectErrorTab.tsx b/public/app/features/inspector/InspectErrorTab.tsx index d149e66c5cf..9c9b4b71aa1 100644 --- a/public/app/features/inspector/InspectErrorTab.tsx +++ b/public/app/features/inspector/InspectErrorTab.tsx @@ -1,10 +1,10 @@ import React from 'react'; import { DataQueryError } from '@grafana/data'; -import { JSONFormatter } from '@grafana/ui'; +import { Alert, JSONFormatter } from '@grafana/ui'; interface InspectErrorTabProps { - error?: DataQueryError; + errors?: DataQueryError[]; } const parseErrorMessage = (message: string): { msg: string; json?: any } => { @@ -20,10 +20,7 @@ const parseErrorMessage = (message: string): { msg: string; json?: any } => { } }; -export const InspectErrorTab = ({ error }: InspectErrorTabProps) => { - if (!error) { - return null; - } +function renderError(error: DataQueryError) { if (error.data) { return ( <> @@ -35,15 +32,39 @@ export const InspectErrorTab = ({ error }: InspectErrorTabProps) => { if (error.message) { const { msg, json } = parseErrorMessage(error.message); if (!json) { - return
{msg}
; + return ( + <> + {error.status && <>Status: {error.status}. Message: } + {msg} + + ); } else { return ( <> {msg !== '' &&

{msg}

} + {error.status && <>Status: {error.status}} ); } } return ; +} + +export const InspectErrorTab = ({ errors }: InspectErrorTabProps) => { + if (!errors?.length) { + return null; + } + if (errors.length === 1) { + return renderError(errors[0]); + } + return ( + <> + {errors.map((error, index) => ( + + {renderError(error)} + + ))} + + ); };