From 652ea3c08ec033773d4ff1b4038c9f0b0531269f Mon Sep 17 00:00:00 2001 From: Peter Holmberg Date: Mon, 11 Feb 2019 16:44:09 +0100 Subject: [PATCH] Move error boundry from DataPanel to PanelChrome #15424, rebased and squashed PR hard move Revert "hard move" This reverts commit a2dad6157a0e77dbdae2f6c7440b55d6a40e3864. bubble error from datapanel to panelchrome remove comments implement show error in panelcorner fixed issue with updatePopperPosition using error callback from datapanel instead Fixes to error handling and clearing, also publishing of legacy events so old query editors work with react panels fully added another error message scenario Restored loading spinner to DataPanel Fixed merge issues (cherry picked from commit 125c087aab64b434dac03e577b39574fe841fed7) --- .../src/components/Tooltip/Popper.tsx | 20 +++-- packages/grafana-ui/src/types/datasource.ts | 10 +++ public/app/core/services/AngularLoader.ts | 4 +- .../features/dashboard/dashgrid/DataPanel.tsx | 73 ++++++------------ .../dashboard/dashgrid/PanelChrome.tsx | 47 +++++++++++- .../dashgrid/PanelHeader/PanelHeader.tsx | 6 +- .../PanelHeader/PanelHeaderCorner.tsx | 53 +++++++------ .../dashboard/panel_editor/QueryEditorRow.tsx | 75 ++++++++++++------- 8 files changed, 173 insertions(+), 115 deletions(-) diff --git a/packages/grafana-ui/src/components/Tooltip/Popper.tsx b/packages/grafana-ui/src/components/Tooltip/Popper.tsx index 93a896d97ed..cf4b9cdd653 100644 --- a/packages/grafana-ui/src/components/Tooltip/Popper.tsx +++ b/packages/grafana-ui/src/components/Tooltip/Popper.tsx @@ -35,8 +35,16 @@ interface Props extends React.HTMLAttributes { class Popper extends PureComponent { render() { - const { show, placement, onMouseEnter, onMouseLeave, className, wrapperClassName, renderArrow } = this.props; - const { content } = this.props; + const { + content, + show, + placement, + onMouseEnter, + onMouseLeave, + className, + wrapperClassName, + renderArrow, + } = this.props; return ( @@ -50,7 +58,7 @@ class Popper extends PureComponent { // TODO: move modifiers config to popper controller modifiers={{ preventOverflow: { enabled: true, boundariesElement: 'window' } }} > - {({ ref, style, placement, arrowProps, scheduleUpdate }) => { + {({ ref, style, placement, arrowProps }) => { return (
{ className={`${wrapperClassName}`} >
- {typeof content === 'string' - ? content - : React.cloneElement(content, { - updatePopperPosition: scheduleUpdate, - })} + {typeof content === 'string' ? content : React.cloneElement(content)} {renderArrow && renderArrow({ arrowProps, diff --git a/packages/grafana-ui/src/types/datasource.ts b/packages/grafana-ui/src/types/datasource.ts index e34cf25dc01..a34f39b59c6 100644 --- a/packages/grafana-ui/src/types/datasource.ts +++ b/packages/grafana-ui/src/types/datasource.ts @@ -29,6 +29,16 @@ export interface DataQuery { datasource?: string | null; } +export interface DataQueryError { + data?: { + message?: string; + error?: string; + }; + message?: string; + status?: string; + statusText?: string; +} + export interface DataQueryOptions { timezone: string; range: TimeRange; diff --git a/public/app/core/services/AngularLoader.ts b/public/app/core/services/AngularLoader.ts index 54dd9a35767..d9b78e66cba 100644 --- a/public/app/core/services/AngularLoader.ts +++ b/public/app/core/services/AngularLoader.ts @@ -27,7 +27,9 @@ export class AngularLoader { compiledElem.remove(); }, digest: () => { - scope.$digest(); + if (!scope.$$phase) { + scope.$digest(); + } }, getScope: () => { return scope; diff --git a/public/app/features/dashboard/dashgrid/DataPanel.tsx b/public/app/features/dashboard/dashgrid/DataPanel.tsx index b81d66fa7f5..0675c7afa60 100644 --- a/public/app/features/dashboard/dashgrid/DataPanel.tsx +++ b/public/app/features/dashboard/dashgrid/DataPanel.tsx @@ -1,8 +1,6 @@ // Library import React, { Component } from 'react'; -import { Tooltip } from '@grafana/ui'; -import ErrorBoundary from 'app/core/components/ErrorBoundary/ErrorBoundary'; // Services import { DatasourceSrv, getDatasourceSrv } from 'app/features/plugins/datasource_srv'; // Utils @@ -11,6 +9,7 @@ import kbn from 'app/core/utils/kbn'; import { DataQueryOptions, DataQueryResponse, + DataQueryError, LoadingState, PanelData, TableData, @@ -18,8 +17,6 @@ import { TimeSeries, } from '@grafana/ui'; -const DEFAULT_PLUGIN_ERROR = 'Error in plugin'; - interface RenderProps { loading: LoadingState; panelData: PanelData; @@ -38,12 +35,12 @@ export interface Props { maxDataPoints?: number; children: (r: RenderProps) => JSX.Element; onDataResponse?: (data: DataQueryResponse) => void; + onError: (message: string, error: DataQueryError) => void; } export interface State { isFirstLoad: boolean; loading: LoadingState; - errorMessage: string; response: DataQueryResponse; } @@ -61,7 +58,6 @@ export class DataPanel extends Component { this.state = { loading: LoadingState.NotStarted, - errorMessage: '', response: { data: [], }, @@ -100,6 +96,7 @@ export class DataPanel extends Component { widthPixels, maxDataPoints, onDataResponse, + onError, } = this.props; if (!isVisible) { @@ -111,7 +108,7 @@ export class DataPanel extends Component { return; } - this.setState({ loading: LoadingState.Loading, errorMessage: '' }); + this.setState({ loading: LoadingState.Loading }); try { const ds = await this.dataSourceSrv.get(datasource); @@ -150,18 +147,22 @@ export class DataPanel extends Component { isFirstLoad: false, }); } catch (err) { - console.log('Loading error', err); - this.onError('Request Error'); - } - }; + console.log('DataPanel error', err); - onError = (errorMessage: string) => { - if (this.state.loading !== LoadingState.Error || this.state.errorMessage !== errorMessage) { - this.setState({ - loading: LoadingState.Error, - isFirstLoad: false, - errorMessage: errorMessage, - }); + let message = 'Query error'; + + if (err.message) { + message = err.message; + } else if (err.data && err.data.message) { + message = err.data.message; + } else if (err.data && err.data.error) { + message = err.data.error; + } else if (err.status) { + message = `Query error: ${err.status} ${err.statusText}`; + } + + onError(message, err); + this.setState({ isFirstLoad: false }); } }; @@ -184,11 +185,10 @@ export class DataPanel extends Component { render() { const { queries } = this.props; const { loading, isFirstLoad } = this.state; - const panelData = this.getPanelData(); if (isFirstLoad && loading === LoadingState.Loading) { - return this.renderLoadingStates(); + return this.renderLoadingState(); } if (!queries.length) { @@ -201,46 +201,21 @@ export class DataPanel extends Component { return ( <> - {this.renderLoadingStates()} - - {({ error, errorInfo }) => { - if (errorInfo) { - this.onError(error.message || DEFAULT_PLUGIN_ERROR); - return null; - } - return ( - <> - {this.props.children({ - loading, - panelData, - })} - - ); - }} - + {this.renderLoadingState()} + {this.props.children({ loading, panelData })} ); } - private renderLoadingStates(): JSX.Element { - const { loading, errorMessage } = this.state; + private renderLoadingState(): JSX.Element { + const { loading } = this.state; if (loading === LoadingState.Loading) { return (
); - } else if (loading === LoadingState.Error) { - return ( - -
- - -
- - ); } - return null; } } diff --git a/public/app/features/dashboard/dashgrid/PanelChrome.tsx b/public/app/features/dashboard/dashgrid/PanelChrome.tsx index 5c4ef430ce4..5a993293946 100644 --- a/public/app/features/dashboard/dashgrid/PanelChrome.tsx +++ b/public/app/features/dashboard/dashgrid/PanelChrome.tsx @@ -8,6 +8,7 @@ import { getTimeSrv, TimeSrv } from '../services/TimeSrv'; // Components import { PanelHeader } from './PanelHeader/PanelHeader'; import { DataPanel } from './DataPanel'; +import ErrorBoundary from '../../../core/components/ErrorBoundary/ErrorBoundary'; // Utils import { applyPanelTimeOverrides, snapshotDataToPanelData } from 'app/features/dashboard/utils/panel'; @@ -17,11 +18,12 @@ import { profiler } from 'app/core/profiler'; // Types import { DashboardModel, PanelModel } from '../state'; import { PanelPlugin } from 'app/types'; -import { TimeRange, LoadingState, PanelData } from '@grafana/ui'; +import { DataQueryResponse, TimeRange, LoadingState, PanelData, DataQueryError } from '@grafana/ui'; import variables from 'sass/_variables.scss'; import templateSrv from 'app/features/templating/template_srv'; -import { DataQueryResponse } from '@grafana/ui/src'; + +const DEFAULT_PLUGIN_ERROR = 'Error in plugin'; export interface Props { panel: PanelModel; @@ -34,6 +36,7 @@ export interface State { renderCounter: number; timeInfo?: string; timeRange?: TimeRange; + errorMessage: string | null; } export class PanelChrome extends PureComponent { @@ -45,6 +48,7 @@ export class PanelChrome extends PureComponent { this.state = { refreshCounter: 0, renderCounter: 0, + errorMessage: null, }; } @@ -88,8 +92,33 @@ export class PanelChrome extends PureComponent { if (this.props.dashboard.isSnapshot()) { this.props.panel.snapshotData = dataQueryResponse.data; } + // clear error state (if any) + this.clearErrorState(); + + // This event is used by old query editors and panel editor options + this.props.panel.events.emit('data-received', dataQueryResponse.data); }; + onDataError = (message: string, error: DataQueryError) => { + if (this.state.errorMessage !== message) { + this.setState({ errorMessage: message }); + } + // this event is used by old query editors + this.props.panel.events.emit('data-error', error); + }; + + onPanelError = (message: string) => { + if (this.state.errorMessage !== message) { + this.setState({ errorMessage: message }); + } + }; + + clearErrorState() { + if (this.state.errorMessage) { + this.setState({ errorMessage: null }); + } + } + get isVisible() { return !this.props.dashboard.otherPanelInFullscreen(this.props.panel); } @@ -150,6 +179,7 @@ export class PanelChrome extends PureComponent { widthPixels={width} refreshCounter={refreshCounter} onDataResponse={this.onDataResponse} + onError={this.onDataError} > {({ loading, panelData }) => { return this.renderPanelPlugin(loading, panelData, width, height); @@ -164,7 +194,7 @@ export class PanelChrome extends PureComponent { render() { const { dashboard, panel } = this.props; - const { timeInfo } = this.state; + const { errorMessage, timeInfo } = this.state; const { transparent } = panel; const containerClassNames = `panel-container panel-container--absolute ${transparent ? 'panel-transparent' : ''}`; @@ -185,8 +215,17 @@ export class PanelChrome extends PureComponent { description={panel.description} scopedVars={panel.scopedVars} links={panel.links} + error={errorMessage} /> - {this.renderPanelBody(width, height)} + + {({ error, errorInfo }) => { + if (errorInfo) { + this.onPanelError(error.message || DEFAULT_PLUGIN_ERROR); + return null; + } + return this.renderPanelBody(width, height); + }} +
); }} diff --git a/public/app/features/dashboard/dashgrid/PanelHeader/PanelHeader.tsx b/public/app/features/dashboard/dashgrid/PanelHeader/PanelHeader.tsx index 22766b4c8e6..2be32cd9955 100644 --- a/public/app/features/dashboard/dashgrid/PanelHeader/PanelHeader.tsx +++ b/public/app/features/dashboard/dashgrid/PanelHeader/PanelHeader.tsx @@ -2,7 +2,7 @@ import React, { Component } from 'react'; import classNames from 'classnames'; import { isEqual } from 'lodash'; -import PanelHeaderCorner from './PanelHeaderCorner'; +import { PanelHeaderCorner } from './PanelHeaderCorner'; import { PanelHeaderMenu } from './PanelHeaderMenu'; import templateSrv from 'app/features/templating/template_srv'; @@ -18,6 +18,7 @@ export interface Props { description?: string; scopedVars?: string; links?: []; + error?: string; } interface ClickCoordinates { @@ -71,7 +72,7 @@ export class PanelHeader extends Component { const isFullscreen = false; const isLoading = false; const panelHeaderClass = classNames({ 'panel-header': true, 'grid-drag-handle': !isFullscreen }); - const { panel, dashboard, timeInfo, scopedVars } = this.props; + const { panel, dashboard, timeInfo, scopedVars, error } = this.props; const title = templateSrv.replaceWithText(panel.title, scopedVars); return ( @@ -82,6 +83,7 @@ export class PanelHeader extends Component { description={panel.description} scopedVars={panel.scopedVars} links={panel.links} + error={error} />
{isLoading && ( diff --git a/public/app/features/dashboard/dashgrid/PanelHeader/PanelHeaderCorner.tsx b/public/app/features/dashboard/dashgrid/PanelHeader/PanelHeaderCorner.tsx index e1c21315bd2..63ebed1632c 100644 --- a/public/app/features/dashboard/dashgrid/PanelHeader/PanelHeaderCorner.tsx +++ b/public/app/features/dashboard/dashgrid/PanelHeader/PanelHeaderCorner.tsx @@ -6,7 +6,7 @@ import templateSrv from 'app/features/templating/template_srv'; import { LinkSrv } from 'app/features/panel/panellinks/link_srv'; import { getTimeSrv, TimeSrv } from 'app/features/dashboard/services/TimeSrv'; -enum InfoModes { +enum InfoMode { Error = 'Error', Info = 'Info', Links = 'Links', @@ -18,18 +18,22 @@ interface Props { description?: string; scopedVars?: string; links?: []; + error?: string; } export class PanelHeaderCorner extends Component { timeSrv: TimeSrv = getTimeSrv(); getInfoMode = () => { - const { panel } = this.props; + const { panel, error } = this.props; + if (error) { + return InfoMode.Error; + } if (!!panel.description) { - return InfoModes.Info; + return InfoMode.Info; } if (panel.links && panel.links.length) { - return InfoModes.Links; + return InfoMode.Links; } return undefined; @@ -42,7 +46,7 @@ export class PanelHeaderCorner extends Component { const interpolatedMarkdown = templateSrv.replace(markdown, panel.scopedVars); const remarkableInterpolatedMarkdown = new Remarkable().render(interpolatedMarkdown); - const html = ( + return (
{panel.links && @@ -62,30 +66,35 @@ export class PanelHeaderCorner extends Component { )}
); - - return html; }; + renderCornerType(infoMode: InfoMode, content: string | JSX.Element) { + const theme = infoMode === InfoMode.Error ? 'error' : 'info'; + return ( + +
+ + +
+ + ); + } + render() { - const infoMode: InfoModes | undefined = this.getInfoMode(); + const infoMode: InfoMode | undefined = this.getInfoMode(); if (!infoMode) { return null; } - return ( - <> - {infoMode === InfoModes.Info || infoMode === InfoModes.Links ? ( - -
- - -
- - ) : null} - - ); + if (infoMode === InfoMode.Error) { + return this.renderCornerType(infoMode, this.props.error); + } + + if (infoMode === InfoMode.Info) { + return this.renderCornerType(infoMode, this.getInfoContent()); + } + + return null; } } - -export default PanelHeaderCorner; diff --git a/public/app/features/dashboard/panel_editor/QueryEditorRow.tsx b/public/app/features/dashboard/panel_editor/QueryEditorRow.tsx index cf89175a5ee..0b8d2c39908 100644 --- a/public/app/features/dashboard/panel_editor/QueryEditorRow.tsx +++ b/public/app/features/dashboard/panel_editor/QueryEditorRow.tsx @@ -28,28 +28,57 @@ interface State { loadedDataSourceValue: string | null | undefined; datasource: DataSourceApi | null; isCollapsed: boolean; - angularScope: AngularQueryComponentScope | null; + hasTextEditMode: boolean; } export class QueryEditorRow extends PureComponent { element: HTMLElement | null = null; + angularScope: AngularQueryComponentScope | null; angularQueryEditor: AngularComponent | null = null; state: State = { datasource: null, isCollapsed: false, - angularScope: null, loadedDataSourceValue: undefined, + hasTextEditMode: false, }; componentDidMount() { this.loadDatasource(); this.props.panel.events.on('refresh', this.onPanelRefresh); + this.props.panel.events.on('data-error', this.onPanelDataError); + this.props.panel.events.on('data-received', this.onPanelDataReceived); } + componentWillUnmount() { + this.props.panel.events.off('refresh', this.onPanelRefresh); + this.props.panel.events.off('data-error', this.onPanelDataError); + this.props.panel.events.off('data-received', this.onPanelDataReceived); + + if (this.angularQueryEditor) { + this.angularQueryEditor.destroy(); + } + } + + onPanelDataError = () => { + // Some query controllers listen to data error events and need a digest + if (this.angularQueryEditor) { + // for some reason this needs to be done in next tick + setTimeout(this.angularQueryEditor.digest); + } + }; + + onPanelDataReceived = () => { + // Some query controllers listen to data error events and need a digest + if (this.angularQueryEditor) { + // for some reason this needs to be done in next tick + setTimeout(this.angularQueryEditor.digest); + } + }; + onPanelRefresh = () => { - if (this.state.angularScope) { - this.state.angularScope.range = getTimeSrv().timeRange(); + if (this.angularScope) { + this.angularScope.range = getTimeSrv().timeRange(); } }; @@ -73,7 +102,11 @@ export class QueryEditorRow extends PureComponent { const dataSourceSrv = getDatasourceSrv(); const datasource = await dataSourceSrv.get(query.datasource || panel.datasource); - this.setState({ datasource, loadedDataSourceValue: this.props.dataSourceValue }); + this.setState({ + datasource, + loadedDataSourceValue: this.props.dataSourceValue, + hasTextEditMode: false, + }); } componentDidUpdate() { @@ -98,21 +131,14 @@ export class QueryEditorRow extends PureComponent { const scopeProps = { ctrl: this.getAngularQueryComponentScope() }; this.angularQueryEditor = loader.load(this.element, scopeProps, template); + this.angularScope = scopeProps.ctrl; // give angular time to compile setTimeout(() => { - this.setState({ angularScope: scopeProps.ctrl }); + this.setState({ hasTextEditMode: !!this.angularScope.toggleEditorMode }); }, 10); } - componentWillUnmount() { - this.props.panel.events.off('refresh', this.onPanelRefresh); - - if (this.angularQueryEditor) { - this.angularQueryEditor.destroy(); - } - } - onToggleCollapse = () => { this.setState({ isCollapsed: !this.state.isCollapsed }); }; @@ -138,10 +164,8 @@ export class QueryEditorRow extends PureComponent { } onToggleEditMode = () => { - const { angularScope } = this.state; - - if (angularScope && angularScope.toggleEditorMode) { - angularScope.toggleEditorMode(); + if (this.angularScope && this.angularScope.toggleEditorMode) { + this.angularScope.toggleEditorMode(); this.angularQueryEditor.digest(); } @@ -150,11 +174,6 @@ export class QueryEditorRow extends PureComponent { } }; - get hasTextEditMode() { - const { angularScope } = this.state; - return angularScope && angularScope.toggleEditorMode; - } - onRemoveQuery = () => { this.props.onRemoveQuery(this.props.query); }; @@ -171,10 +190,8 @@ export class QueryEditorRow extends PureComponent { }; renderCollapsedText(): string | null { - const { angularScope } = this.state; - - if (angularScope && angularScope.getCollapsedText) { - return angularScope.getCollapsedText(); + if (this.angularScope && this.angularScope.getCollapsedText) { + return this.angularScope.getCollapsedText(); } return null; @@ -182,7 +199,7 @@ export class QueryEditorRow extends PureComponent { render() { const { query, inMixedMode } = this.props; - const { datasource, isCollapsed } = this.state; + const { datasource, isCollapsed, hasTextEditMode } = this.state; const isDisabled = query.hide; const bodyClasses = classNames('query-editor-row__body gf-form-query', { @@ -212,7 +229,7 @@ export class QueryEditorRow extends PureComponent { {isCollapsed &&
{this.renderCollapsedText()}
}
- {this.hasTextEditMode && ( + {hasTextEditMode && (