Move error boundry from DataPanel to PanelChrome #15424, rebased and squashed PR

hard move
Revert "hard move"
This reverts commit a2dad6157a.
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)

packages/grafana-ui/src/components/Tooltip/Popper.tsx

@@ -35,8 +35,16 @@ interface Props extends React.HTMLAttributes<HTMLDivElement> {
 
 class Popper extends PureComponent<Props> {
   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 (
       <Manager>
@@ -50,7 +58,7 @@ class Popper extends PureComponent<Props> {
                   // TODO: move modifiers config to popper controller
                   modifiers={{ preventOverflow: { enabled: true, boundariesElement: 'window' } }}
                 >
-                  {({ ref, style, placement, arrowProps, scheduleUpdate }) => {
+                  {({ ref, style, placement, arrowProps }) => {
                     return (
                       <div
                         onMouseEnter={onMouseEnter}
@@ -65,11 +73,7 @@ class Popper extends PureComponent<Props> {
                         className={`${wrapperClassName}`}
                       >
                         <div className={className}>
-                          {typeof content === 'string'
-                            ? content
-                            : React.cloneElement(content, {
-                                updatePopperPosition: scheduleUpdate,
-                              })}
+                          {typeof content === 'string' ? content : React.cloneElement(content)}
                           {renderArrow &&
                             renderArrow({
                               arrowProps,

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<TQuery extends DataQuery = DataQuery> {
   timezone: string;
   range: TimeRange;

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;

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<Props, State> {
 
     this.state = {
       loading: LoadingState.NotStarted,
-      errorMessage: '',
       response: {
         data: [],
       },
@@ -100,6 +96,7 @@ export class DataPanel extends Component<Props, State> {
       widthPixels,
       maxDataPoints,
       onDataResponse,
+      onError,
     } = this.props;
 
     if (!isVisible) {
@@ -111,7 +108,7 @@ export class DataPanel extends Component<Props, State> {
       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<Props, State> {
         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<Props, State> {
   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<Props, State> {
 
     return (
       <>
-        {this.renderLoadingStates()}
-        <ErrorBoundary>
-          {({ error, errorInfo }) => {
-            if (errorInfo) {
-              this.onError(error.message || DEFAULT_PLUGIN_ERROR);
-              return null;
-            }
-            return (
-              <>
-                {this.props.children({
-                  loading,
-                  panelData,
-                })}
-              </>
-            );
-          }}
-        </ErrorBoundary>
+        {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 (
         <div className="panel-loading">
           <i className="fa fa-spinner fa-spin" />
         </div>
       );
-    } else if (loading === LoadingState.Error) {
-      return (
-        <Tooltip content={errorMessage} placement="bottom-start" theme="error">
-          <div className="panel-info-corner panel-info-corner--error">
-            <i className="fa" />
-            <span className="panel-info-corner-inner" />
-          </div>
-        </Tooltip>
-      );
     }
-
     return null;
   }
 }

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<Props, State> {
@@ -45,6 +48,7 @@ export class PanelChrome extends PureComponent<Props, State> {
     this.state = {
       refreshCounter: 0,
       renderCounter: 0,
+      errorMessage: null,
     };
   }
 
@@ -88,8 +92,33 @@ export class PanelChrome extends PureComponent<Props, State> {
     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<Props, State> {
             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<Props, State> {
 
   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<Props, State> {
                 description={panel.description}
                 scopedVars={panel.scopedVars}
                 links={panel.links}
+                error={errorMessage}
               />
-              {this.renderPanelBody(width, height)}
+              <ErrorBoundary>
+                {({ error, errorInfo }) => {
+                  if (errorInfo) {
+                    this.onPanelError(error.message || DEFAULT_PLUGIN_ERROR);
+                    return null;
+                  }
+                  return this.renderPanelBody(width, height);
+                }}
+              </ErrorBoundary>
             </div>
           );
         }}

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<Props, State> {
     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<Props, State> {
           description={panel.description}
           scopedVars={panel.scopedVars}
           links={panel.links}
+          error={error}
         />
         <div className={panelHeaderClass}>
           {isLoading && (

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<Props> {
   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<Props> {
     const interpolatedMarkdown = templateSrv.replace(markdown, panel.scopedVars);
     const remarkableInterpolatedMarkdown = new Remarkable().render(interpolatedMarkdown);
 
-    const html = (
+    return (
       <div className="markdown-html">
         <div dangerouslySetInnerHTML={{ __html: remarkableInterpolatedMarkdown }} />
         {panel.links &&
@@ -62,30 +66,35 @@ export class PanelHeaderCorner extends Component<Props> {
           )}
       </div>
     );
-
-    return html;
   };
 
+  renderCornerType(infoMode: InfoMode, content: string | JSX.Element) {
+    const theme = infoMode === InfoMode.Error ? 'error' : 'info';
+    return (
+      <Tooltip content={content} placement="bottom-start" theme={theme}>
+        <div className={`panel-info-corner panel-info-corner--${infoMode.toLowerCase()}`}>
+          <i className="fa" />
+          <span className="panel-info-corner-inner" />
+        </div>
+      </Tooltip>
+    );
+  }
+
   render() {
-    const infoMode: InfoModes | undefined = this.getInfoMode();
+    const infoMode: InfoMode | undefined = this.getInfoMode();
 
     if (!infoMode) {
       return null;
     }
 
-    return (
-      <>
-        {infoMode === InfoModes.Info || infoMode === InfoModes.Links ? (
-          <Tooltip content={this.getInfoContent()} placement="bottom-start">
-            <div className={`panel-info-corner panel-info-corner--${infoMode.toLowerCase()}`}>
-              <i className="fa" />
-              <span className="panel-info-corner-inner" />
-            </div>
-          </Tooltip>
-        ) : 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;

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<Props, State> {
   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<Props, State> {
     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<Props, State> {
     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<Props, State> {
   }
 
   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<Props, State> {
     }
   };
 
-  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<Props, State> {
   };
 
   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<Props, State> {
 
   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<Props, State> {
             {isCollapsed && <div>{this.renderCollapsedText()}</div>}
           </div>
           <div className="query-editor-row__actions">
-            {this.hasTextEditMode && (
+            {hasTextEditMode && (
               <button
                 className="query-editor-row__action"
                 onClick={this.onToggleEditMode}