Browse Source

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
kay delaney 6 years ago
parent
commit
477d5643b6

+ 41 - 35
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<PromQuery, PromOptions>
           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<PromQuery, PromOptions>
       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<PromQuery, PromOptions>
       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<PromQuery, PromOptions>
     error.status = err.status;
     error.statusText = err.statusText;
 
-    throw error;
+    return error;
   };
 
   performSuggestQuery(query: string, cache = false) {

+ 73 - 33
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';
+    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',
+      };
+      // 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',
+      beforeEach(async () => {
+        const response = {
           data: {
-            resultType: 'matrix',
-            result: [
-              {
-                metric: { __name__: 'test', job: 'testjob' },
-                values: [[60, '3846']],
-              },
-            ],
+            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"}');
+      });
+    });
+
+    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,
         },
       };
-      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;
+      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 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 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"}');