From 651ec36ea453ef8f77a741e235ec036f40cc74a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Jamr=C3=B3z?= Date: Fri, 2 Apr 2021 14:27:05 +0200 Subject: [PATCH 1/3] Display an error when finding metrics fails --- .../plugins/datasource/graphite/query_ctrl.ts | 14 ++++- .../graphite/specs/query_ctrl.test.ts | 53 ++++++++++++++++++- 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/public/app/plugins/datasource/graphite/query_ctrl.ts b/public/app/plugins/datasource/graphite/query_ctrl.ts index 3a6a24f5cee00..749527eecc9df 100644 --- a/public/app/plugins/datasource/graphite/query_ctrl.ts +++ b/public/app/plugins/datasource/graphite/query_ctrl.ts @@ -24,8 +24,9 @@ export class GraphiteQueryCtrl extends QueryCtrl { supportsTags: boolean; paused: boolean; - // to avoid error flooding, it's shown only once per session + // to avoid error flooding, these errors are shown only once per session private _tagsAutoCompleteErrorShown = false; + private _metricAutoCompleteErrorShown = false; /** @ngInject */ constructor( @@ -110,7 +111,7 @@ export class GraphiteQueryCtrl extends QueryCtrl { } }) .catch((err: any) => { - dispatch(notifyApp(createErrorNotification('Error', err))); + this.handleMetricsAutoCompleteError(err); }); } @@ -183,6 +184,7 @@ export class GraphiteQueryCtrl extends QueryCtrl { } }) .catch((err: any): any[] => { + this.handleMetricsAutoCompleteError(err); return []; }); } @@ -437,6 +439,14 @@ export class GraphiteQueryCtrl extends QueryCtrl { dispatch(notifyApp(createErrorNotification(`Fetching tags failed: ${error.message}.`))); } } + + private handleMetricsAutoCompleteError(error: Error): void { + console.log(error); + if (!this._metricAutoCompleteErrorShown) { + this._metricAutoCompleteErrorShown = true; + dispatch(notifyApp(createErrorNotification(`Fetching metrics failed: ${error.message}.`))); + } + } } function mapToDropdownOptions(results: any[]) { diff --git a/public/app/plugins/datasource/graphite/specs/query_ctrl.test.ts b/public/app/plugins/datasource/graphite/specs/query_ctrl.test.ts index dedf20e18c0b7..322f078d780ca 100644 --- a/public/app/plugins/datasource/graphite/specs/query_ctrl.test.ts +++ b/public/app/plugins/datasource/graphite/specs/query_ctrl.test.ts @@ -193,10 +193,61 @@ describe('GraphiteQueryCtrl', () => { }); }); + describe('when autocomplete for metric names is not available', () => { + silenceConsoleOutput(); + beforeEach(() => { + ctx.ctrl.datasource.getTagsAutoComplete = jest.fn().mockReturnValue(Promise.resolve([])); + ctx.ctrl.datasource.metricFindQuery = jest.fn().mockReturnValue( + new Promise(() => { + throw new Error(); + }) + ); + }); + + it('getAltSegments should handle autocomplete errors', async () => { + await expect(async () => { + await ctx.ctrl.getAltSegments(0, 'any'); + expect(mockDispatch).toBeCalledWith( + expect.objectContaining({ + type: 'appNotifications/notifyApp', + }) + ); + }).not.toThrow(); + }); + + it('getAltSegments should display the error message only once', async () => { + await ctx.ctrl.getAltSegments(0, 'any'); + expect(mockDispatch.mock.calls.length).toBe(1); + + await ctx.ctrl.getAltSegments(0, 'any'); + expect(mockDispatch.mock.calls.length).toBe(1); + }); + + it('checkOtherSegments should handle autocomplete errors', async () => { + await expect(async () => { + await ctx.ctrl.checkOtherSegments(1, false); + expect(mockDispatch).toBeCalledWith( + expect.objectContaining({ + type: 'appNotifications/notifyApp', + }) + ); + }).not.toThrow(); + }); + + it('checkOtherSegments should display the error message only once', async () => { + await ctx.ctrl.checkOtherSegments(1, false); + expect(mockDispatch.mock.calls.length).toBe(1); + + await ctx.ctrl.checkOtherSegments(1, false); + expect(mockDispatch.mock.calls.length).toBe(1); + }); + }); + describe('when autocomplete for tags is not available', () => { silenceConsoleOutput(); beforeEach(() => { - ctx.ctrl.datasource.getTagsAutoComplete.mockReturnValue( + ctx.datasource.metricFindQuery = jest.fn().mockReturnValue(Promise.resolve([])); + ctx.datasource.getTagsAutoComplete = jest.fn().mockReturnValue( new Promise(() => { throw new Error(); }) From 8793c05a872cbb892cb58233b0b4228a91901065 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Jamr=C3=B3z?= Date: Fri, 9 Apr 2021 15:45:59 +0200 Subject: [PATCH 2/3] Update public/app/plugins/datasource/graphite/query_ctrl.ts Co-authored-by: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> --- public/app/plugins/datasource/graphite/query_ctrl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/app/plugins/datasource/graphite/query_ctrl.ts b/public/app/plugins/datasource/graphite/query_ctrl.ts index 749527eecc9df..f04a26b523ccc 100644 --- a/public/app/plugins/datasource/graphite/query_ctrl.ts +++ b/public/app/plugins/datasource/graphite/query_ctrl.ts @@ -441,7 +441,7 @@ export class GraphiteQueryCtrl extends QueryCtrl { } private handleMetricsAutoCompleteError(error: Error): void { - console.log(error); + console.error(error); if (!this._metricAutoCompleteErrorShown) { this._metricAutoCompleteErrorShown = true; dispatch(notifyApp(createErrorNotification(`Fetching metrics failed: ${error.message}.`))); From cd96253d76e7e20fef98dfeec503c841bbf1415a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Jamr=C3=B3z?= Date: Fri, 9 Apr 2021 15:46:45 +0200 Subject: [PATCH 3/3] Make the level of the log message more accurate --- public/app/plugins/datasource/graphite/query_ctrl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/app/plugins/datasource/graphite/query_ctrl.ts b/public/app/plugins/datasource/graphite/query_ctrl.ts index f04a26b523ccc..31582f487a513 100644 --- a/public/app/plugins/datasource/graphite/query_ctrl.ts +++ b/public/app/plugins/datasource/graphite/query_ctrl.ts @@ -433,7 +433,7 @@ export class GraphiteQueryCtrl extends QueryCtrl { } private handleTagsAutoCompleteError(error: Error): void { - console.log(error); + console.error(error); if (!this._tagsAutoCompleteErrorShown) { this._tagsAutoCompleteErrorShown = true; dispatch(notifyApp(createErrorNotification(`Fetching tags failed: ${error.message}.`)));