Merge pull request #15424 from grafana/move-error-boundry

Move error boundry from DataPanel to PanelChrome

Merge pull request #15424 from grafana/move-error-boundry
Move error boundry from DataPanel to PanelChrome
7fd89ff7 · Torkel Ödegaard · GitHub · 2c923659 · 19a080a4 · 7fd89ff7
Commit 7fd89ff7 authored Feb 14, 2019 by Torkel Ödegaard Committed by GitHub Feb 14, 2019
9 changed files
--- a/packages/grafana-ui/src/components/EmptySearchResult/EmptySearchResult.tsx
+++ b/packages/grafana-ui/src/components/EmptySearchResult/EmptySearchResult.tsx
@@ -5,9 +5,7 @@ export interface Props {
 }
 const EmptySearchResult: FC<Props> = ({ children }) => {
-  return (
+  return <div className="empty-search-result">{children}</div>;
-    <div className="empty-search-result">{children}</div>
-  );
 };
 export { EmptySearchResult };
--- 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<HTMLDivElement> {
 class Popper extends PureComponent<Props> {
  render() {
-    const { show, placement, onMouseEnter, onMouseLeave, className, wrapperClassName, renderArrow } = this.props;
+    const {
-    const { content } = this.props;
+      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'
+                          {typeof content === 'string' ? content : React.cloneElement(content)}
-                            ? content
-                            : React.cloneElement(content, {
-                                updatePopperPosition: scheduleUpdate,
-                              })}
                          {renderArrow &&
                            renderArrow({
                              arrowProps,

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

--- 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;

--- a/public/app/features/dashboard/dashgrid/DataPanel.tsx
+++ b/public/app/features/dashboard/dashgrid/DataPanel.tsx
 // 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);
+      console.log('DataPanel error', err);
-      this.onError('Request Error');
-    }
+      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 = (errorMessage: string) => {
+      onError(message, err);
-    if (this.state.loading !== LoadingState.Error || this.state.errorMessage !== errorMessage) {
+      this.setState({ isFirstLoad: false });
-      this.setState({
-        loading: LoadingState.Error,
-        isFirstLoad: false,
-        errorMessage: errorMessage,
-      });
    }
  };
@@ -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()}
+        {this.renderLoadingState()}
-        <ErrorBoundary>
+        {this.props.children({ loading, panelData })}
-          {({ error, errorInfo }) => {
-            if (errorInfo) {
-              this.onError(error.message || DEFAULT_PLUGIN_ERROR);
-              return null;
-            }
-            return (
-              <>
-                {this.props.children({
-                  loading,
-                  panelData,
-                })}
-              </>
-            );
-          }}
-        </ErrorBoundary>
      </>
    );
  }
-  private renderLoadingStates(): JSX.Element {
+  private renderLoadingState(): JSX.Element {
-    const { loading, errorMessage } = this.state;
+    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;
  }
 }
--- 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<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>
          );
        }}

--- a/public/app/features/dashboard/dashgrid/PanelHeader/PanelHeader.tsx
+++ b/public/app/features/dashboard/dashgrid/PanelHeader/PanelHeader.tsx
@@ -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 && (

--- 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<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,29 +66,36 @@ 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 (
+    if (infoMode === InfoMode.Error) {
-      <>
+      return this.renderCornerType(infoMode, this.props.error);
-        {infoMode === InfoModes.Info || infoMode === InfoModes.Links ? (
+    }
-          <Tooltip content={this.getInfoContent()} placement="bottom-start">
-            <div className={`panel-info-corner panel-info-corner--${infoMode.toLowerCase()}`}>
+    if (infoMode === InfoMode.Info) {
-              <i className="fa" />
+      return this.renderCornerType(infoMode, this.getInfoContent());
-              <span className="panel-info-corner-inner" />
+    }
-            </div>
-          </Tooltip>
+    return null;
-        ) : null}
-      </>
-    );
  }
 }

--- 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<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) {
+    if (this.angularScope) {
-      this.state.angularScope.range = getTimeSrv().timeRange();
+      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 (this.angularScope && this.angularScope.toggleEditorMode) {
+      this.angularScope.toggleEditorMode();
-    if (angularScope && angularScope.toggleEditorMode) {
-      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 (this.angularScope && this.angularScope.getCollapsedText) {
+      return this.angularScope.getCollapsedText();
-    if (angularScope && angularScope.getCollapsedText) {
-      return 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}