Merge pull request #13842 from grafana/davkal/explore-error-handling

Explore: error handling and time fixes

public/app/core/table_model.ts

@@ -83,6 +83,10 @@ function areRowsMatching(columns, row, otherRow) {
 export function mergeTablesIntoModel(dst?: TableModel, ...tables: TableModel[]): TableModel {
   const model = dst || new TableModel();
 
+  if (arguments.length === 1) {
+    return model;
+  }
+
   // Single query returns data columns and rows as is
   if (arguments.length === 2) {
     model.columns = [...tables[0].columns];

public/app/core/utils/rangeutil.ts

@@ -1,5 +1,8 @@
 import _ from 'lodash';
 import moment from 'moment';
+
+import { RawTimeRange } from 'app/types/series';
+
 import * as dateMath from './datemath';
 
 const spans = {
@@ -129,7 +132,7 @@ export function describeTextRange(expr: any) {
   return opt;
 }
 
-export function describeTimeRange(range) {
+export function describeTimeRange(range: RawTimeRange): string {
   const option = rangeIndex[range.from.toString() + ' to ' + range.to.toString()];
   if (option) {
     return option.display;

public/app/features/explore/ErrorBoundary.tsx

@@ -0,0 +1,34 @@
+import React, { Component } from 'react';
+
+export default class ErrorBoundary extends Component<{}, any> {
+  constructor(props) {
+    super(props);
+    this.state = { error: null, errorInfo: null };
+  }
+
+  componentDidCatch(error, errorInfo) {
+    // Catch errors in any components below and re-render with error message
+    this.setState({
+      error: error,
+      errorInfo: errorInfo,
+    });
+  }
+
+  render() {
+    if (this.state.errorInfo) {
+      // Error path
+      return (
+        <div className="explore-container">
+          <h3>An unexpected error happened.</h3>
+          <details style={{ whiteSpace: 'pre-wrap' }}>
+            {this.state.error && this.state.error.toString()}
+            <br />
+            {this.state.errorInfo.componentStack}
+          </details>
+        </div>
+      );
+    }
+    // Normally, just render children
+    return this.props.children;
+  }
+}

public/app/features/explore/Explore.tsx

@@ -3,15 +3,7 @@ import { hot } from 'react-hot-loader';
 import Select from 'react-select';
 import _ from 'lodash';
 
-import {
-  ExploreState,
-  ExploreUrlState,
-  HistoryItem,
-  Query,
-  QueryTransaction,
-  Range,
-  ResultType,
-} from 'app/types/explore';
+import { ExploreState, ExploreUrlState, HistoryItem, Query, QueryTransaction, ResultType } from 'app/types/explore';
 import kbn from 'app/core/utils/kbn';
 import colors from 'app/core/utils/colors';
 import store from 'app/core/store';
@@ -24,12 +16,14 @@ import IndicatorsContainer from 'app/core/components/Picker/IndicatorsContainer'
 import NoOptionsMessage from 'app/core/components/Picker/NoOptionsMessage';
 import TableModel, { mergeTablesIntoModel } from 'app/core/table_model';
 
+import ErrorBoundary from './ErrorBoundary';
 import QueryRows from './QueryRows';
 import Graph from './Graph';
 import Logs from './Logs';
 import Table from './Table';
 import TimePicker from './TimePicker';
 import { ensureQueries, generateQueryKey, hasQuery } from './utils/query';
+import { RawTimeRange } from 'app/types/series';
 
 const MAX_HISTORY_ITEMS = 100;
 
@@ -154,11 +148,6 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
     }
   }
 
-  componentDidCatch(error) {
-    this.setState({ datasourceError: error });
-    console.error(error);
-  }
-
   async setDatasource(datasource) {
     const supportsGraph = datasource.meta.metrics;
     const supportsLogs = datasource.meta.logs;
@@ -170,7 +159,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
       const testResult = await datasource.testDatasource();
       datasourceError = testResult.status === 'success' ? null : testResult.message;
     } catch (error) {
-      datasourceError = (error && error.statusText) || error;
+      datasourceError = (error && error.statusText) || 'Network error';
     }
 
     const historyKey = `grafana.explore.history.${datasourceId}`;
@@ -278,10 +267,9 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
     }
   };
 
-  onChangeTime = nextRange => {
-    const range = {
-      from: nextRange.from,
-      to: nextRange.to,
+  onChangeTime = (nextRange: RawTimeRange) => {
+    const range: RawTimeRange = {
+      ...nextRange,
     };
     this.setState({ range }, () => this.onSubmit());
   };
@@ -471,7 +459,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
   ) {
     const { datasource, range } = this.state;
     const resolution = this.el.offsetWidth;
-    const absoluteRange = {
+    const absoluteRange: RawTimeRange = {
       from: parseDate(range.from, false),
       to: parseDate(range.to, true),
     };
@@ -486,7 +474,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
     ];
 
     // Clone range for query request
-    const queryRange: Range = { ...range };
+    const queryRange: RawTimeRange = { ...range };
 
     return {
       interval,
@@ -584,13 +572,28 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
     });
   }
 
-  failQueryTransaction(transactionId: string, error: string, datasourceId: string) {
+  failQueryTransaction(transactionId: string, response: any, datasourceId: string) {
     const { datasource } = this.state;
     if (datasource.meta.id !== datasourceId) {
       // Navigated away, queries did not matter
       return;
     }
 
+    console.error(response);
+
+    let error: string | JSX.Element = response;
+    if (response.data) {
+      error = response.data.error;
+      if (response.data.response) {
+        error = (
+          <>
+            <span>{response.data.error}</span>
+            <details>{response.data.response}</details>
+          </>
+        );
+      }
+    }
+
     this.setState(state => {
       // Transaction might have been discarded
       if (!state.queryTransactions.find(qt => qt.id === transactionId)) {
@@ -637,9 +640,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
           this.completeQueryTransaction(transaction.id, results, latency, queries, datasourceId);
           this.setState({ graphRange: transaction.options.range });
         } catch (response) {
-          console.error(response);
-          const queryError = response.data ? response.data.error : response;
-          this.failQueryTransaction(transaction.id, queryError, datasourceId);
+          this.failQueryTransaction(transaction.id, response, datasourceId);
         }
       } else {
         this.discardTransactions(rowIndex);
@@ -669,9 +670,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
           const results = res.data[0];
           this.completeQueryTransaction(transaction.id, results, latency, queries, datasourceId);
         } catch (response) {
-          console.error(response);
-          const queryError = response.data ? response.data.error : response;
-          this.failQueryTransaction(transaction.id, queryError, datasourceId);
+          this.failQueryTransaction(transaction.id, response, datasourceId);
         }
       } else {
         this.discardTransactions(rowIndex);
@@ -697,9 +696,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
           const results = res.data;
           this.completeQueryTransaction(transaction.id, results, latency, queries, datasourceId);
         } catch (response) {
-          console.error(response);
-          const queryError = response.data ? response.data.error : response;
-          this.failQueryTransaction(transaction.id, queryError, datasourceId);
+          this.failQueryTransaction(transaction.id, response, datasourceId);
         }
       } else {
         this.discardTransactions(rowIndex);
@@ -751,15 +748,16 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
     const graphLoading = queryTransactions.some(qt => qt.resultType === 'Graph' && !qt.done);
     const tableLoading = queryTransactions.some(qt => qt.resultType === 'Table' && !qt.done);
     const logsLoading = queryTransactions.some(qt => qt.resultType === 'Logs' && !qt.done);
+    // TODO don't recreate those on each re-render
     const graphResult = _.flatten(
       queryTransactions.filter(qt => qt.resultType === 'Graph' && qt.done && qt.result).map(qt => qt.result)
     );
     const tableResult = mergeTablesIntoModel(
       new TableModel(),
-      ...queryTransactions.filter(qt => qt.resultType === 'Table' && qt.done).map(qt => qt.result)
+      ...queryTransactions.filter(qt => qt.resultType === 'Table' && qt.done && qt.result).map(qt => qt.result)
     );
     const logsResult = _.flatten(
-      queryTransactions.filter(qt => qt.resultType === 'Logs' && qt.done).map(qt => qt.result)
+      queryTransactions.filter(qt => qt.resultType === 'Logs' && qt.done && qt.result).map(qt => qt.result)
     );
     const loading = queryTransactions.some(qt => !qt.done);
 
@@ -868,23 +866,25 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
             </div>
 
             <main className="m-t-2">
-              {supportsGraph &&
-                showingGraph && (
-                  <Graph
-                    data={graphResult}
-                    height={graphHeight}
-                    loading={graphLoading}
-                    id={`explore-graph-${position}`}
-                    range={graphRange}
-                    split={split}
-                  />
-                )}
-              {supportsTable && showingTable ? (
-                <div className="panel-container m-t-2">
-                  <Table data={tableResult} loading={tableLoading} onClickCell={this.onClickTableCell} />
-                </div>
-              ) : null}
-              {supportsLogs && showingLogs ? <Logs data={logsResult} loading={logsLoading} /> : null}
+              <ErrorBoundary>
+                {supportsGraph &&
+                  showingGraph && (
+                    <Graph
+                      data={graphResult}
+                      height={graphHeight}
+                      loading={graphLoading}
+                      id={`explore-graph-${position}`}
+                      range={graphRange}
+                      split={split}
+                    />
+                  )}
+                {supportsTable && showingTable ? (
+                  <div className="panel-container m-t-2">
+                    <Table data={tableResult} loading={tableLoading} onClickCell={this.onClickTableCell} />
+                  </div>
+                ) : null}
+                {supportsLogs && showingLogs ? <Logs data={logsResult} loading={logsLoading} /> : null}
+              </ErrorBoundary>
             </main>
           </div>
         ) : null}

public/app/features/explore/Graph.tsx

@@ -6,7 +6,7 @@ import { withSize } from 'react-sizeme';
 import 'vendor/flot/jquery.flot';
 import 'vendor/flot/jquery.flot.time';
 
-import { Range } from 'app/types/explore';
+import { RawTimeRange } from 'app/types/series';
 import * as dateMath from 'app/core/utils/datemath';
 import TimeSeries from 'app/core/time_series2';
 
@@ -76,7 +76,7 @@ interface GraphProps {
   height?: string; // e.g., '200px'
   id?: string;
   loading?: boolean;
-  range: Range;
+  range: RawTimeRange;
   split?: boolean;
   size?: { width: number; height: number };
 }

public/app/features/explore/PromQueryField.tsx

@@ -90,7 +90,7 @@ interface CascaderOption {
 
 interface PromQueryFieldProps {
   datasource: any;
-  error?: string;
+  error?: string | JSX.Element;
   hint?: any;
   history?: any[];
   initialQuery?: string | null;

public/app/features/explore/TimePicker.tsx

@@ -3,6 +3,7 @@ import moment from 'moment';
 
 import * as dateMath from 'app/core/utils/datemath';
 import * as rangeUtil from 'app/core/utils/rangeutil';
+import { RawTimeRange } from 'app/types/series';
 
 const DATE_FORMAT = 'YYYY-MM-DD HH:mm:ss';
 export const DEFAULT_RANGE = {
@@ -10,77 +11,104 @@ export const DEFAULT_RANGE = {
   to: 'now',
 };
 
-export function parseTime(value, isUtc = false, asString = false) {
+/**
+ * Return a human-editable string of either relative (inludes "now") or absolute local time (in the shape of DATE_FORMAT).
+ * @param value Epoch or relative time
+ */
+export function parseTime(value: string, isUtc = false): string {
   if (value.indexOf('now') !== -1) {
     return value;
   }
-  if (!isNaN(value)) {
-    const epoch = parseInt(value, 10);
-    const m = isUtc ? moment.utc(epoch) : moment(epoch);
-    return asString ? m.format(DATE_FORMAT) : m;
+  let time: any = value;
+  // Possible epoch
+  if (!isNaN(time)) {
+    time = parseInt(time, 10);
   }
-  return undefined;
+  time = isUtc ? moment.utc(time) : moment(time);
+  return time.format(DATE_FORMAT);
 }
 
-export default class TimePicker extends PureComponent<any, any> {
+interface TimePickerProps {
+  isOpen?: boolean;
+  isUtc?: boolean;
+  range?: RawTimeRange;
+  onChangeTime?: (Range) => void;
+}
+
+interface TimePickerState {
+  isOpen: boolean;
+  isUtc: boolean;
+  rangeString: string;
+  refreshInterval: string;
+
+  // Input-controlled text, keep these in a shape that is human-editable
+  fromRaw: string;
+  toRaw: string;
+}
+
+export default class TimePicker extends PureComponent<TimePickerProps, TimePickerState> {
   dropdownEl: any;
+
   constructor(props) {
     super(props);
 
-    const fromRaw = props.range ? props.range.from : DEFAULT_RANGE.from;
-    const toRaw = props.range ? props.range.to : DEFAULT_RANGE.to;
+    const from = props.range ? props.range.from : DEFAULT_RANGE.from;
+    const to = props.range ? props.range.to : DEFAULT_RANGE.to;
+
+    // Ensure internal format
+    const fromRaw = parseTime(from, props.isUtc);
+    const toRaw = parseTime(to, props.isUtc);
     const range = {
-      from: parseTime(fromRaw),
-      to: parseTime(toRaw),
+      from: fromRaw,
+      to: toRaw,
     };
+
     this.state = {
-      fromRaw: parseTime(fromRaw, props.isUtc, true),
+      fromRaw,
+      toRaw,
       isOpen: props.isOpen,
       isUtc: props.isUtc,
       rangeString: rangeUtil.describeTimeRange(range),
       refreshInterval: '',
-      toRaw: parseTime(toRaw, props.isUtc, true),
     };
   }
 
-  move(direction) {
+  move(direction: number) {
     const { onChangeTime } = this.props;
     const { fromRaw, toRaw } = this.state;
-    const range = {
-      from: dateMath.parse(fromRaw, false),
-      to: dateMath.parse(toRaw, true),
-    };
+    const from = dateMath.parse(fromRaw, false);
+    const to = dateMath.parse(toRaw, true);
+    const timespan = (to.valueOf() - from.valueOf()) / 2;
 
-    const timespan = (range.to.valueOf() - range.from.valueOf()) / 2;
-    let to, from;
+    let nextTo, nextFrom;
     if (direction === -1) {
-      to = range.to.valueOf() - timespan;
-      from = range.from.valueOf() - timespan;
+      nextTo = to.valueOf() - timespan;
+      nextFrom = from.valueOf() - timespan;
     } else if (direction === 1) {
-      to = range.to.valueOf() + timespan;
-      from = range.from.valueOf() + timespan;
-      if (to > Date.now() && range.to < Date.now()) {
-        to = Date.now();
-        from = range.from.valueOf();
+      nextTo = to.valueOf() + timespan;
+      nextFrom = from.valueOf() + timespan;
+      if (nextTo > Date.now() && to < Date.now()) {
+        nextTo = Date.now();
+        nextFrom = from.valueOf();
       }
     } else {
-      to = range.to.valueOf();
-      from = range.from.valueOf();
+      nextTo = to.valueOf();
+      nextFrom = from.valueOf();
     }
 
-    const rangeString = rangeUtil.describeTimeRange(range);
-    // No need to convert to UTC again
-    to = moment(to);
-    from = moment(from);
+    const nextRange = {
+      from: moment(nextFrom),
+      to: moment(nextTo),
+    };
 
     this.setState(
       {
-        rangeString,
-        fromRaw: from.format(DATE_FORMAT),
-        toRaw: to.format(DATE_FORMAT),
+        rangeString: rangeUtil.describeTimeRange(nextRange),
+        fromRaw: nextRange.from.format(DATE_FORMAT),
+        toRaw: nextRange.to.format(DATE_FORMAT),
       },
       () => {
-        onChangeTime({ to, from });
+        onChangeTime(nextRange);
       }
     );
   }
@@ -99,16 +127,19 @@ export default class TimePicker extends PureComponent<any, any> {
 
   handleClickApply = () => {
     const { onChangeTime } = this.props;
-    const { toRaw, fromRaw } = this.state;
-    const range = {
-      from: dateMath.parse(fromRaw, false),
-      to: dateMath.parse(toRaw, true),
-    };
-    const rangeString = rangeUtil.describeTimeRange(range);
+    let range;
     this.setState(
-      {
-        isOpen: false,
-        rangeString,
+      state => {
+        const { toRaw, fromRaw } = this.state;
+        range = {
+          from: dateMath.parse(fromRaw, false),
+          to: dateMath.parse(toRaw, true),
+        };
+        const rangeString = rangeUtil.describeTimeRange(range);
+        return {
+          isOpen: false,
+          rangeString,
+        };
       },
       () => {
         if (onChangeTime) {

public/app/features/explore/Wrapper.tsx

@@ -7,6 +7,7 @@ import { serializeStateToUrlParam, parseUrlState } from 'app/core/utils/explore'
 import { StoreState } from 'app/types';
 import { ExploreState } from 'app/types/explore';
 
+import ErrorBoundary from './ErrorBoundary';
 import Explore from './Explore';
 
 interface WrapperProps {
@@ -61,28 +62,33 @@ export class Wrapper extends Component<WrapperProps, WrapperState> {
     const { split, splitState } = this.state;
     const urlStateLeft = parseUrlState(this.urlStates[STATE_KEY_LEFT]);
     const urlStateRight = parseUrlState(this.urlStates[STATE_KEY_RIGHT]);
+
     return (
       <div className="explore-wrapper">
-        <Explore
-          datasourceSrv={datasourceSrv}
-          onChangeSplit={this.onChangeSplit}
-          onSaveState={this.onSaveState}
-          position="left"
-          split={split}
-          stateKey={STATE_KEY_LEFT}
-          urlState={urlStateLeft}
-        />
-        {split && (
+        <ErrorBoundary>
           <Explore
             datasourceSrv={datasourceSrv}
             onChangeSplit={this.onChangeSplit}
             onSaveState={this.onSaveState}
-            position="right"
+            position="left"
             split={split}
-            splitState={splitState}
-            stateKey={STATE_KEY_RIGHT}
-            urlState={urlStateRight}
+            stateKey={STATE_KEY_LEFT}
+            urlState={urlStateLeft}
           />
+        </ErrorBoundary>
+        {split && (
+          <ErrorBoundary>
+            <Explore
+              datasourceSrv={datasourceSrv}
+              onChangeSplit={this.onChangeSplit}
+              onSaveState={this.onSaveState}
+              position="right"
+              split={split}
+              splitState={splitState}
+              stateKey={STATE_KEY_RIGHT}
+              urlState={urlStateRight}
+            />
+          </ErrorBoundary>
         )}
       </div>
     );

public/app/types/explore.ts

@@ -1,5 +1,7 @@
 import { Value } from 'slate';
 
+import { RawTimeRange } from './series';
+
 export interface CompletionItem {
   /**
    * The label of this completion item. By default
@@ -100,11 +102,6 @@ export interface TypeaheadOutput {
   suggestions: CompletionItemGroup[];
 }
 
-export interface Range {
-  from: string;
-  to: string;
-}
-
 export interface Query {
   query: string;
   key?: string;
@@ -131,7 +128,7 @@ export interface QueryHint {
 export interface QueryTransaction {
   id: string;
   done: boolean;
-  error?: string;
+  error?: string | JSX.Element;
   hints?: QueryHint[];
   latency: number;
   options: any;
@@ -155,7 +152,7 @@ export interface ExploreState {
   datasourceMissing: boolean;
   datasourceName?: string;
   exploreDatasources: ExploreDatasource[];
-  graphRange: Range;
+  graphRange: RawTimeRange;
   history: HistoryItem[];
   /**
    * Initial rows of queries to push down the tree.
@@ -167,7 +164,7 @@ export interface ExploreState {
    * Hints gathered for the query row.
    */
   queryTransactions: QueryTransaction[];
-  range: Range;
+  range: RawTimeRange;
   showingGraph: boolean;
   showingLogs: boolean;
   showingTable: boolean;
@@ -179,7 +176,7 @@ export interface ExploreState {
 export interface ExploreUrlState {
   datasource: string;
   queries: Query[];
-  range: Range;
+  range: RawTimeRange;
 }
 
 export type ResultType = 'Graph' | 'Logs' | 'Table';

public/sass/pages/_explore.scss

@@ -258,6 +258,11 @@
 
   .prom-query-field-info {
     margin: 0.25em 0.5em 0.5em;
+    display: flex;
+
+    details {
+      margin-left: 1em;
+    }
   }
 }