From 477d5643b64edac67bb041a69c58938b8030b2b0 Mon Sep 17 00:00:00 2001 From: kay delaney <45561153+kaydelaney@users.noreply.github.com> Date: Tue, 18 Jun 2019 11:07:09 +0100 Subject: [PATCH] Explore: Prometheus query errors now show (#17470) * Explore: Prometheus query errors now show Fixes: #17435 * Explore: Adds test to ensure prometheus query errors are reported * Explore: removes implicit anys introduced previously --- .../datasource/prometheus/datasource.ts | 76 ++++++------ .../prometheus/specs/datasource.test.ts | 112 ++++++++++++------ 2 files changed, 117 insertions(+), 71 deletions(-) diff --git a/public/app/plugins/datasource/prometheus/datasource.ts b/public/app/plugins/datasource/prometheus/datasource.ts index c12aef1cfd6..7f62b7e6897 100644 --- a/public/app/plugins/datasource/prometheus/datasource.ts +++ b/public/app/plugins/datasource/prometheus/datasource.ts @@ -1,8 +1,8 @@ // Libraries import _ from 'lodash'; import $ from 'jquery'; -import { from, Observable } from 'rxjs'; -import { single, map, filter } from 'rxjs/operators'; +import { from, of, Observable } from 'rxjs'; +import { single, map, filter, catchError } from 'rxjs/operators'; // Services & Utils import kbn from 'app/core/utils/kbn'; @@ -25,6 +25,7 @@ import { DataQueryError, DataStreamObserver, LoadingState, + DataStreamState, DataQueryResponseData, } from '@grafana/ui/src/types'; import { ExploreUrlState } from 'app/types/explore'; @@ -199,31 +200,32 @@ export class PrometheusDatasource extends DataSourceApi single(), // unsubscribes automatically after first result filter((response: any) => (response.cancelled ? false : true)), map((response: any) => { - return this.processResult(response, query, target, queries.length); + const delta = this.processResult(response, query, target, queries.length); + const state: DataStreamState = { + key: `prometheus-${target.refId}`, + state: query.instant ? LoadingState.Loading : LoadingState.Done, + request: options, + delta, + unsubscribe: () => undefined, + }; + + return state; + }), + catchError(err => { + const error = this.handleErrors(err, target); + const state: DataStreamState = { + key: `prometheus-${target.refId}`, + request: options, + state: LoadingState.Error, + error, + unsubscribe: () => undefined, + }; + + return of(state); }) ) .subscribe({ - next: series => { - if (query.instant) { - observer({ - key: `prometheus-${target.refId}`, - state: LoadingState.Loading, - request: options, - series: null, - delta: series, - unsubscribe: () => undefined, - }); - } else { - observer({ - key: `prometheus-${target.refId}`, - state: LoadingState.Done, - request: options, - series: null, - delta: series, - unsubscribe: () => undefined, - }); - } - }, + next: state => observer(state), }); } }; @@ -399,9 +401,13 @@ export class PrometheusDatasource extends DataSourceApi data['timeout'] = this.queryTimeout; } - return this._request(url, data, { requestId: query.requestId, headers: query.headers }).catch((err: any) => - this.handleErrors(err, query) - ); + return this._request(url, data, { requestId: query.requestId, headers: query.headers }).catch((err: any) => { + if (err.cancelled) { + return err; + } + + throw this.handleErrors(err, query); + }); } performInstantQuery(query: PromQueryRequest, time: number) { @@ -415,16 +421,16 @@ export class PrometheusDatasource extends DataSourceApi data['timeout'] = this.queryTimeout; } - return this._request(url, data, { requestId: query.requestId, headers: query.headers }).catch((err: any) => - this.handleErrors(err, query) - ); + return this._request(url, data, { requestId: query.requestId, headers: query.headers }).catch((err: any) => { + if (err.cancelled) { + return err; + } + + throw this.handleErrors(err, query); + }); } handleErrors = (err: any, target: PromQuery) => { - if (err.cancelled) { - return err; - } - const error: DataQueryError = { message: 'Unknown error during query transaction. Please check JS console logs.', refId: target.refId, @@ -445,7 +451,7 @@ export class PrometheusDatasource extends DataSourceApi error.status = err.status; error.statusText = err.statusText; - throw error; + return error; }; performSuggestQuery(query: string, cache = false) { diff --git a/public/app/plugins/datasource/prometheus/specs/datasource.test.ts b/public/app/plugins/datasource/prometheus/specs/datasource.test.ts index 91949c69bd0..a70cdf90803 100644 --- a/public/app/plugins/datasource/prometheus/specs/datasource.test.ts +++ b/public/app/plugins/datasource/prometheus/specs/datasource.test.ts @@ -425,49 +425,84 @@ const timeSrv = ({ describe('PrometheusDatasource', () => { describe('When querying prometheus with one target using query editor target spec', () => { - let results: any; - const query = { - range: { from: time({ seconds: 63 }), to: time({ seconds: 183 }) }, - targets: [{ expr: 'test{job="testjob"}', format: 'time_series' }], - interval: '60s', - }; - // Interval alignment with step - const urlExpected = - 'proxied/api/v1/query_range?query=' + encodeURIComponent('test{job="testjob"}') + '&start=60&end=180&step=60'; - - beforeEach(async () => { - const response = { - data: { - status: 'success', - data: { - resultType: 'matrix', - result: [ - { - metric: { __name__: 'test', job: 'testjob' }, - values: [[60, '3846']], - }, - ], - }, - }, + describe('and query syntax is valid', () => { + let results: any; + const query = { + range: { from: time({ seconds: 63 }), to: time({ seconds: 183 }) }, + targets: [{ expr: 'test{job="testjob"}', format: 'time_series' }], + interval: '60s', }; - backendSrv.datasourceRequest = jest.fn(() => Promise.resolve(response)); - ctx.ds = new PrometheusDatasource(instanceSettings, q, backendSrv as any, templateSrv as any, timeSrv as any); + // Interval alignment with step + const urlExpected = + 'proxied/api/v1/query_range?query=' + encodeURIComponent('test{job="testjob"}') + '&start=60&end=180&step=60'; - await ctx.ds.query(query).then((data: any) => { - results = data; + beforeEach(async () => { + const response = { + data: { + status: 'success', + data: { + resultType: 'matrix', + result: [ + { + metric: { __name__: 'test', job: 'testjob' }, + values: [[60, '3846']], + }, + ], + }, + }, + }; + backendSrv.datasourceRequest = jest.fn(() => Promise.resolve(response)); + ctx.ds = new PrometheusDatasource(instanceSettings, q, backendSrv as any, templateSrv as any, timeSrv as any); + + await ctx.ds.query(query).then((data: any) => { + results = data; + }); + }); + + it('should generate the correct query', () => { + const res = backendSrv.datasourceRequest.mock.calls[0][0]; + expect(res.method).toBe('GET'); + expect(res.url).toBe(urlExpected); + }); + + it('should return series list', async () => { + expect(results.data.length).toBe(1); + expect(results.data[0].target).toBe('test{job="testjob"}'); }); }); - it('should generate the correct query', () => { - const res = backendSrv.datasourceRequest.mock.calls[0][0]; - expect(res.method).toBe('GET'); - expect(res.url).toBe(urlExpected); - }); - it('should return series list', async () => { - expect(results.data.length).toBe(1); - expect(results.data[0].target).toBe('test{job="testjob"}'); + describe('and query syntax is invalid', () => { + let results: string; + const query = { + range: { from: time({ seconds: 63 }), to: time({ seconds: 183 }) }, + targets: [{ expr: 'tes;;t{job="testjob"}', format: 'time_series' }], + interval: '60s', + }; + + const errMessage = 'parse error at char 25: could not parse remaining input'; + const response = { + data: { + status: 'error', + errorType: 'bad_data', + error: errMessage, + }, + }; + + beforeEach(async () => { + backendSrv.datasourceRequest = jest.fn(() => Promise.reject(response)); + ctx.ds = new PrometheusDatasource(instanceSettings, q, backendSrv as any, templateSrv as any, timeSrv as any); + + await ctx.ds.query(query).catch((e: any) => { + results = e.message; + }); + }); + + it('should generate an error', () => { + expect(results).toBe(`"${errMessage}"`); + }); }); }); + describe('When querying prometheus with one target which returns multiple series', () => { let results: any; const start = 60; @@ -536,6 +571,7 @@ describe('PrometheusDatasource', () => { expect(results.data[1].datapoints[3][0]).toBe(null); }); }); + describe('When querying prometheus with one target and instant = true', () => { let results: any; const urlExpected = 'proxied/api/v1/query?query=' + encodeURIComponent('test{job="testjob"}') + '&time=123'; @@ -578,6 +614,7 @@ describe('PrometheusDatasource', () => { expect(results.data[0].target).toBe('test{job="testjob"}'); }); }); + describe('When performing annotationQuery', () => { let results: any; @@ -1227,6 +1264,7 @@ describe('PrometheusDatasource', () => { }); }); }); + describe('The __range, __range_s and __range_ms variables', () => { const response = { status: 'success', @@ -1329,12 +1367,14 @@ describe('PrometheusDatasource for POST', () => { results = data; }); }); + it('should generate the correct query', () => { const res = backendSrv.datasourceRequest.mock.calls[0][0]; expect(res.method).toBe('POST'); expect(res.url).toBe(urlExpected); expect(res.data).toEqual(dataExpected); }); + it('should return series list', () => { expect(results.data.length).toBe(1); expect(results.data[0].target).toBe('test{job="testjob"}');