From caa82a124daaf87c648fb194f3fc1770373f824c Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Wed, 6 Apr 2022 11:17:49 +0200 Subject: [PATCH] Loki,Prometheus: Fix of showing error message for empty query (#47379) * Loki,Prometheus: Dont show error on empty query * Add tests --- .../loki/querybuilder/parsing.test.ts | 9 ++++++ .../datasource/loki/querybuilder/parsing.ts | 28 ++++++++++++++++--- .../prometheus/querybuilder/parsing.test.ts | 9 ++++++ .../prometheus/querybuilder/parsing.ts | 12 ++++++++ .../querybuilder/shared/parsingUtils.ts | 2 +- 5 files changed, 55 insertions(+), 5 deletions(-) diff --git a/public/app/plugins/datasource/loki/querybuilder/parsing.test.ts b/public/app/plugins/datasource/loki/querybuilder/parsing.test.ts index c3b94525519..51201da9d36 100644 --- a/public/app/plugins/datasource/loki/querybuilder/parsing.test.ts +++ b/public/app/plugins/datasource/loki/querybuilder/parsing.test.ts @@ -2,6 +2,15 @@ import { buildVisualQueryFromString } from './parsing'; import { LokiVisualQuery } from './types'; describe('buildVisualQueryFromString', () => { + it('creates no errors for empty query', () => { + expect(buildVisualQueryFromString('')).toEqual( + noErrors({ + labels: [], + operations: [], + }) + ); + }); + it('parses simple query with label-values', () => { expect(buildVisualQueryFromString('{app="frontend"}')).toEqual( noErrors({ diff --git a/public/app/plugins/datasource/loki/querybuilder/parsing.ts b/public/app/plugins/datasource/loki/querybuilder/parsing.ts index 3b19dbfe863..43168019c2e 100644 --- a/public/app/plugins/datasource/loki/querybuilder/parsing.ts +++ b/public/app/plugins/datasource/loki/querybuilder/parsing.ts @@ -20,8 +20,8 @@ interface Context { interface ParsingError { text: string; - from: number; - to: number; + from?: number; + to?: number; parentType?: string; } @@ -36,12 +36,25 @@ export function buildVisualQueryFromString(expr: string): Context { operations: [], }; - const context = { + const context: Context = { query: visQuery, errors: [], }; - handleExpression(replacedExpr, node, context); + try { + handleExpression(replacedExpr, node, context); + } catch (err) { + // Not ideal to log it here, but otherwise we would lose the stack trace. + console.error(err); + context.errors.push({ + text: err.message, + }); + } + + // If we have empty query, we want to reset errors + if (isEmptyQuery(context.query)) { + context.errors = []; + } return context; } @@ -496,3 +509,10 @@ function createNotSupportedError(expr: string, node: SyntaxNode, error: string) err.text = `${error}: ${err.text}`; return err; } + +function isEmptyQuery(query: LokiVisualQuery) { + if (query.labels.length === 0 && query.operations.length === 0) { + return true; + } + return false; +} diff --git a/public/app/plugins/datasource/prometheus/querybuilder/parsing.test.ts b/public/app/plugins/datasource/prometheus/querybuilder/parsing.test.ts index fc7ffb8142c..a340f44fb63 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/parsing.test.ts +++ b/public/app/plugins/datasource/prometheus/querybuilder/parsing.test.ts @@ -2,6 +2,15 @@ import { buildVisualQueryFromString } from './parsing'; import { PromVisualQuery } from './types'; describe('buildVisualQueryFromString', () => { + it('creates no errors for empty query', () => { + expect(buildVisualQueryFromString('')).toEqual( + noErrors({ + labels: [], + operations: [], + metric: '', + }) + ); + }); it('parses simple query', () => { expect(buildVisualQueryFromString('counters_logins{app="frontend"}')).toEqual( noErrors({ diff --git a/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts b/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts index 0db720b4b0f..0db7f39daed 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts +++ b/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts @@ -46,6 +46,11 @@ export function buildVisualQueryFromString(expr: string): Context { text: err.message, }); } + + // If we have empty query, we want to reset errors + if (isEmptyQuery(context.query)) { + context.errors = []; + } return context; } @@ -373,3 +378,10 @@ function getBinaryModifier( }; } } + +function isEmptyQuery(query: PromVisualQuery) { + if (query.labels.length === 0 && query.operations.length === 0 && !query.metric) { + return true; + } + return false; +} diff --git a/public/app/plugins/datasource/prometheus/querybuilder/shared/parsingUtils.ts b/public/app/plugins/datasource/prometheus/querybuilder/shared/parsingUtils.ts index 9895860e27e..e2d89d4db6d 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/shared/parsingUtils.ts +++ b/public/app/plugins/datasource/prometheus/querybuilder/shared/parsingUtils.ts @@ -30,7 +30,7 @@ export function makeError(expr: string, node: SyntaxNode) { const variableRegex = /\$(\w+)|\[\[([\s\S]+?)(?::(\w+))?\]\]|\${(\w+)(?:\.([^:^\}]+))?(?::([^\}]+))?}/g; /** - * As variables with $ are creating parsing errors, we first replace them with magic string that is parseable and at + * As variables with $ are creating parsing errors, we first replace them with magic string that is parsable and at * the same time we can get the variable and it's format back from it. * @param expr */