From bf1af66292ca27df3d9ddf278ec9f12051f12a59 Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Fri, 26 Oct 2018 18:16:00 +0200 Subject: [PATCH] Explore: error handling and time fixes - use global range types - add ErrorBoundary around individual Explore components - fix table merge on empty results - fix TimePicker date parsing on ISO dates - fix TimePicker range string after relative move --- public/app/core/table_model.ts | 4 + public/app/core/utils/rangeutil.ts | 5 +- public/app/features/explore/ErrorBoundary.tsx | 34 +++++ public/app/features/explore/Explore.tsx | 100 +++++++------- public/app/features/explore/Graph.tsx | 4 +- .../app/features/explore/PromQueryField.tsx | 2 +- public/app/features/explore/TimePicker.tsx | 123 +++++++++++------- public/app/features/explore/Wrapper.tsx | 34 +++-- public/app/types/explore.ts | 15 +-- public/sass/pages/_explore.scss | 5 + 10 files changed, 203 insertions(+), 123 deletions(-) create mode 100644 public/app/features/explore/ErrorBoundary.tsx diff --git a/public/app/core/table_model.ts b/public/app/core/table_model.ts index 99395258ba3..91a7cd0c1fb 100644 --- a/public/app/core/table_model.ts +++ b/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]; diff --git a/public/app/core/utils/rangeutil.ts b/public/app/core/utils/rangeutil.ts index 484dd0e3327..2079aa39006 100644 --- a/public/app/core/utils/rangeutil.ts +++ b/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; diff --git a/public/app/features/explore/ErrorBoundary.tsx b/public/app/features/explore/ErrorBoundary.tsx new file mode 100644 index 00000000000..959d15eea80 --- /dev/null +++ b/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 ( +
+

An unexpected error happened.

+
+ {this.state.error && this.state.error.toString()} +
+ {this.state.errorInfo.componentStack} +
+
+ ); + } + // Normally, just render children + return this.props.children; + } +} diff --git a/public/app/features/explore/Explore.tsx b/public/app/features/explore/Explore.tsx index 680cd1e6685..4e7ae676d21 100644 --- a/public/app/features/explore/Explore.tsx +++ b/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 { } } - 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 { 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 { } }; - onChangeTime = nextRange => { - const range = { - from: nextRange.from, - to: nextRange.to, + onChangeTime = (nextRange: RawTimeRange) => { + const range: RawTimeRange = { + ...nextRange, }; this.setState({ range }, () => this.onSubmit()); }; @@ -459,7 +447,7 @@ export class Explore extends React.PureComponent { ) { 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), }; @@ -474,7 +462,7 @@ export class Explore extends React.PureComponent { ]; // Clone range for query request - const queryRange: Range = { ...range }; + const queryRange: RawTimeRange = { ...range }; return { interval, @@ -572,13 +560,28 @@ export class Explore extends React.PureComponent { }); } - 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 = ( + <> + {response.data.error} +
{response.data.response}
+ + ); + } + } + this.setState(state => { // Transaction might have been discarded if (!state.queryTransactions.find(qt => qt.id === transactionId)) { @@ -625,9 +628,7 @@ export class Explore extends React.PureComponent { 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); @@ -657,9 +658,7 @@ export class Explore extends React.PureComponent { 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); @@ -685,9 +684,7 @@ export class Explore extends React.PureComponent { 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); @@ -739,15 +736,16 @@ export class Explore extends React.PureComponent { 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); @@ -856,23 +854,25 @@ export class Explore extends React.PureComponent {
- {supportsGraph && - showingGraph && ( - - )} - {supportsTable && showingTable ? ( -
- - - ) : null} - {supportsLogs && showingLogs ? : null} + + {supportsGraph && + showingGraph && ( + + )} + {supportsTable && showingTable ? ( +
+
+ + ) : null} + {supportsLogs && showingLogs ? : null} + ) : null} diff --git a/public/app/features/explore/Graph.tsx b/public/app/features/explore/Graph.tsx index d57f8d49a43..1f07b9bb46a 100644 --- a/public/app/features/explore/Graph.tsx +++ b/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 }; } diff --git a/public/app/features/explore/PromQueryField.tsx b/public/app/features/explore/PromQueryField.tsx index 58856e3116f..c7afe606734 100644 --- a/public/app/features/explore/PromQueryField.tsx +++ b/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; diff --git a/public/app/features/explore/TimePicker.tsx b/public/app/features/explore/TimePicker.tsx index f9c740073d0..8955fb4aa9b 100644 --- a/public/app/features/explore/TimePicker.tsx +++ b/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 { +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 { 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 { 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) { diff --git a/public/app/features/explore/Wrapper.tsx b/public/app/features/explore/Wrapper.tsx index 7e07aafbf6d..de1eee4c662 100644 --- a/public/app/features/explore/Wrapper.tsx +++ b/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 { const { split, splitState } = this.state; const urlStateLeft = parseUrlState(this.urlStates[STATE_KEY_LEFT]); const urlStateRight = parseUrlState(this.urlStates[STATE_KEY_RIGHT]); + return (
- - {split && ( + + + {split && ( + + + )}
); diff --git a/public/app/types/explore.ts b/public/app/types/explore.ts index 807769212f3..ab62124f60a 100644 --- a/public/app/types/explore.ts +++ b/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; @@ -130,7 +127,7 @@ export interface QueryHint { export interface QueryTransaction { id: string; done: boolean; - error?: string; + error?: string | JSX.Element; hints?: QueryHint[]; latency: number; options: any; @@ -154,7 +151,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. @@ -166,7 +163,7 @@ export interface ExploreState { * Hints gathered for the query row. */ queryTransactions: QueryTransaction[]; - range: Range; + range: RawTimeRange; showingGraph: boolean; showingLogs: boolean; showingTable: boolean; @@ -178,7 +175,7 @@ export interface ExploreState { export interface ExploreUrlState { datasource: string; queries: Query[]; - range: Range; + range: RawTimeRange; } export type ResultType = 'Graph' | 'Logs' | 'Table'; diff --git a/public/sass/pages/_explore.scss b/public/sass/pages/_explore.scss index a3f60f2006b..91e07bc46e6 100644 --- a/public/sass/pages/_explore.scss +++ b/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; + } } }