From b0ec86a7bd7d451cec177c09f233996880d1c96b Mon Sep 17 00:00:00 2001 From: Vadim Kibana <82822460+vadimkibana@users.noreply.github.com> Date: Wed, 16 Oct 2024 14:04:04 +0200 Subject: [PATCH] [ES|QL] Catch internal parsing errors (#196249) ## Summary Addresses this comment https://github.com/elastic/kibana/issues/191683#issuecomment-2338181503 Catches parser internal errors, and reports them the same as the regular parsing errors. (cherry picked from commit 1f56f2102fd2108dc4469710e1a2765f918451b6) --- .../src/parser/__tests__/columns.test.ts | 2 +- .../src/parser/__tests__/commands.test.ts | 2 +- .../src/parser/__tests__/from.test.ts | 2 +- .../src/parser/__tests__/function.test.ts | 2 +- .../src/parser/__tests__/inlinecast.test.ts | 2 +- .../src/parser/__tests__/literal.test.ts | 2 +- .../src/parser/__tests__/metrics.test.ts | 2 +- .../src/parser/__tests__/params.test.ts | 2 +- .../src/parser/__tests__/rename.test.ts | 2 +- .../src/parser/__tests__/sort.test.ts | 2 +- .../src/parser/__tests__/where.test.ts | 2 +- packages/kbn-esql-ast/src/parser/parser.ts | 85 +++++++++++++------ 12 files changed, 70 insertions(+), 37 deletions(-) diff --git a/packages/kbn-esql-ast/src/parser/__tests__/columns.test.ts b/packages/kbn-esql-ast/src/parser/__tests__/columns.test.ts index e79c418eeb3c..38e98104d41b 100644 --- a/packages/kbn-esql-ast/src/parser/__tests__/columns.test.ts +++ b/packages/kbn-esql-ast/src/parser/__tests__/columns.test.ts @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { getAstAndSyntaxErrors as parse } from '..'; +import { parse } from '..'; describe('Column Identifier Expressions', () => { it('can parse un-quoted identifiers', () => { diff --git a/packages/kbn-esql-ast/src/parser/__tests__/commands.test.ts b/packages/kbn-esql-ast/src/parser/__tests__/commands.test.ts index 30d44d447387..6fb176c9624f 100644 --- a/packages/kbn-esql-ast/src/parser/__tests__/commands.test.ts +++ b/packages/kbn-esql-ast/src/parser/__tests__/commands.test.ts @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { getAstAndSyntaxErrors as parse } from '..'; +import { parse } from '..'; describe('commands', () => { describe('correctly formatted, basic usage', () => { diff --git a/packages/kbn-esql-ast/src/parser/__tests__/from.test.ts b/packages/kbn-esql-ast/src/parser/__tests__/from.test.ts index 101661973a69..f2f0fded57ca 100644 --- a/packages/kbn-esql-ast/src/parser/__tests__/from.test.ts +++ b/packages/kbn-esql-ast/src/parser/__tests__/from.test.ts @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { getAstAndSyntaxErrors as parse } from '..'; +import { parse } from '..'; describe('FROM', () => { describe('correctly formatted', () => { diff --git a/packages/kbn-esql-ast/src/parser/__tests__/function.test.ts b/packages/kbn-esql-ast/src/parser/__tests__/function.test.ts index 8ec533816a56..9d822f78f933 100644 --- a/packages/kbn-esql-ast/src/parser/__tests__/function.test.ts +++ b/packages/kbn-esql-ast/src/parser/__tests__/function.test.ts @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { getAstAndSyntaxErrors as parse } from '..'; +import { parse } from '..'; import { Walker } from '../../walker'; describe('function AST nodes', () => { diff --git a/packages/kbn-esql-ast/src/parser/__tests__/inlinecast.test.ts b/packages/kbn-esql-ast/src/parser/__tests__/inlinecast.test.ts index d0650ab3f321..889ca2a2ecf3 100644 --- a/packages/kbn-esql-ast/src/parser/__tests__/inlinecast.test.ts +++ b/packages/kbn-esql-ast/src/parser/__tests__/inlinecast.test.ts @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { getAstAndSyntaxErrors as parse } from '..'; +import { parse } from '..'; import { ESQLFunction, ESQLInlineCast, ESQLSingleAstItem } from '../../types'; describe('Inline cast (::)', () => { diff --git a/packages/kbn-esql-ast/src/parser/__tests__/literal.test.ts b/packages/kbn-esql-ast/src/parser/__tests__/literal.test.ts index 514d769d5c45..7f50198c9604 100644 --- a/packages/kbn-esql-ast/src/parser/__tests__/literal.test.ts +++ b/packages/kbn-esql-ast/src/parser/__tests__/literal.test.ts @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { getAstAndSyntaxErrors as parse } from '..'; +import { parse } from '..'; import { ESQLLiteral } from '../../types'; describe('literal expression', () => { diff --git a/packages/kbn-esql-ast/src/parser/__tests__/metrics.test.ts b/packages/kbn-esql-ast/src/parser/__tests__/metrics.test.ts index 54ddc49c5d04..d33c94e8903a 100644 --- a/packages/kbn-esql-ast/src/parser/__tests__/metrics.test.ts +++ b/packages/kbn-esql-ast/src/parser/__tests__/metrics.test.ts @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { getAstAndSyntaxErrors as parse } from '..'; +import { parse } from '..'; describe('METRICS', () => { describe('correctly formatted', () => { diff --git a/packages/kbn-esql-ast/src/parser/__tests__/params.test.ts b/packages/kbn-esql-ast/src/parser/__tests__/params.test.ts index 8586236eeb2f..e4b1a892d32d 100644 --- a/packages/kbn-esql-ast/src/parser/__tests__/params.test.ts +++ b/packages/kbn-esql-ast/src/parser/__tests__/params.test.ts @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { getAstAndSyntaxErrors as parse } from '..'; +import { parse } from '..'; import { Walker } from '../../walker'; /** diff --git a/packages/kbn-esql-ast/src/parser/__tests__/rename.test.ts b/packages/kbn-esql-ast/src/parser/__tests__/rename.test.ts index 4acad891150b..214e5c1d3688 100644 --- a/packages/kbn-esql-ast/src/parser/__tests__/rename.test.ts +++ b/packages/kbn-esql-ast/src/parser/__tests__/rename.test.ts @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { getAstAndSyntaxErrors as parse } from '..'; +import { parse } from '..'; describe('RENAME', () => { /** diff --git a/packages/kbn-esql-ast/src/parser/__tests__/sort.test.ts b/packages/kbn-esql-ast/src/parser/__tests__/sort.test.ts index cfaec0a6e39e..981eac40b68a 100644 --- a/packages/kbn-esql-ast/src/parser/__tests__/sort.test.ts +++ b/packages/kbn-esql-ast/src/parser/__tests__/sort.test.ts @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { getAstAndSyntaxErrors as parse } from '..'; +import { parse } from '..'; describe('SORT', () => { describe('correctly formatted', () => { diff --git a/packages/kbn-esql-ast/src/parser/__tests__/where.test.ts b/packages/kbn-esql-ast/src/parser/__tests__/where.test.ts index d507b559fd40..f3f6aeb886ec 100644 --- a/packages/kbn-esql-ast/src/parser/__tests__/where.test.ts +++ b/packages/kbn-esql-ast/src/parser/__tests__/where.test.ts @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { getAstAndSyntaxErrors as parse } from '..'; +import { parse } from '..'; describe('WHERE', () => { describe('correctly formatted', () => { diff --git a/packages/kbn-esql-ast/src/parser/parser.ts b/packages/kbn-esql-ast/src/parser/parser.ts index ad263a49ebd0..f99e00e92d1e 100644 --- a/packages/kbn-esql-ast/src/parser/parser.ts +++ b/packages/kbn-esql-ast/src/parser/parser.ts @@ -100,33 +100,66 @@ export interface ParseResult { } export const parse = (text: string | undefined, options: ParseOptions = {}): ParseResult => { - if (text == null) { - const commands: ESQLAstQueryExpression['commands'] = []; - return { ast: commands, root: Builder.expression.query(commands), errors: [], tokens: [] }; + try { + if (text == null) { + const commands: ESQLAstQueryExpression['commands'] = []; + return { ast: commands, root: Builder.expression.query(commands), errors: [], tokens: [] }; + } + const errorListener = new ESQLErrorListener(); + const parseListener = new ESQLAstBuilderListener(); + const { tokens, parser } = getParser( + CharStreams.fromString(text), + errorListener, + parseListener + ); + + parser[GRAMMAR_ROOT_RULE](); + + const errors = errorListener.getErrors().filter((error) => { + return !SYNTAX_ERRORS_TO_IGNORE.includes(error.message); + }); + const { ast: commands } = parseListener.getAst(); + const root = Builder.expression.query(commands, { + location: { + min: 0, + max: text.length - 1, + }, + }); + + if (options.withFormatting) { + const decorations = collectDecorations(tokens); + attachDecorations(root, tokens.tokens, decorations.lines); + } + + return { root, ast: commands, errors, tokens: tokens.tokens }; + } catch (error) { + /** + * Parsing should never fail, meaning this branch should never execute. But + * if it does fail, we want to log the error message for easier debugging. + */ + // eslint-disable-next-line no-console + console.error(error); + + const root = Builder.expression.query(); + + return { + root, + ast: root.commands, + errors: [ + { + startLineNumber: 0, + endLineNumber: 0, + startColumn: 0, + endColumn: 0, + message: + 'Parsing internal error: ' + + (!!error && typeof error === 'object' ? String(error.message) : String(error)), + severity: 'error', + }, + ], + tokens: [], + }; } - const errorListener = new ESQLErrorListener(); - const parseListener = new ESQLAstBuilderListener(); - const { tokens, parser } = getParser(CharStreams.fromString(text), errorListener, parseListener); - - parser[GRAMMAR_ROOT_RULE](); - - const errors = errorListener.getErrors().filter((error) => { - return !SYNTAX_ERRORS_TO_IGNORE.includes(error.message); - }); - const { ast: commands } = parseListener.getAst(); - const root = Builder.expression.query(commands, { - location: { - min: 0, - max: text.length - 1, - }, - }); - - if (options.withFormatting) { - const decorations = collectDecorations(tokens); - attachDecorations(root, tokens.tokens, decorations.lines); - } - - return { root, ast: commands, errors, tokens: tokens.tokens }; }; export const parseErrors = (text: string) => {