Skip to content

Commit

Permalink
[8.x] [ES|QL] Catch internal parsing errors (#196249) (#196540)
Browse files Browse the repository at this point in the history
# Backport

This will backport the following commits from `main` to `8.x`:
- [[ES|QL] Catch internal parsing errors
(#196249)](#196249)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Vadim
Kibana","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-16T12:04:04Z","message":"[ES|QL]
Catch internal parsing errors (#196249)\n\n## Summary\r\n\r\nAddresses
this
comment\r\nhttps://github.com//issues/191683#issuecomment-2338181503\r\n\r\nCatches
parser internal errors, and reports them the same as the
regular\r\nparsing
errors.","sha":"1f56f2102fd2108dc4469710e1a2765f918451b6","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["review","release_note:skip","v9.0.0","backport:prev-minor","Feature:ES|QL","Team:ESQL","v8.16.0"],"title":"[ES|QL]
Catch internal parsing
errors","number":196249,"url":"https://github.com/elastic/kibana/pull/196249","mergeCommit":{"message":"[ES|QL]
Catch internal parsing errors (#196249)\n\n## Summary\r\n\r\nAddresses
this
comment\r\nhttps://github.com//issues/191683#issuecomment-2338181503\r\n\r\nCatches
parser internal errors, and reports them the same as the
regular\r\nparsing
errors.","sha":"1f56f2102fd2108dc4469710e1a2765f918451b6"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196249","number":196249,"mergeCommit":{"message":"[ES|QL]
Catch internal parsing errors (#196249)\n\n## Summary\r\n\r\nAddresses
this
comment\r\nhttps://github.com//issues/191683#issuecomment-2338181503\r\n\r\nCatches
parser internal errors, and reports them the same as the
regular\r\nparsing
errors.","sha":"1f56f2102fd2108dc4469710e1a2765f918451b6"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Vadim Kibana <[email protected]>
Co-authored-by: Stratoula Kalafateli <[email protected]>
  • Loading branch information
3 people authored Oct 18, 2024
1 parent 9424b3d commit 21e0bc9
Show file tree
Hide file tree
Showing 12 changed files with 70 additions and 37 deletions.
2 changes: 1 addition & 1 deletion packages/kbn-esql-ast/src/parser/__tests__/columns.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-esql-ast/src/parser/__tests__/from.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 (::)', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-esql-ast/src/parser/__tests__/literal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-esql-ast/src/parser/__tests__/metrics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-esql-ast/src/parser/__tests__/params.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-esql-ast/src/parser/__tests__/rename.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
/**
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-esql-ast/src/parser/__tests__/sort.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-esql-ast/src/parser/__tests__/where.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
85 changes: 59 additions & 26 deletions packages/kbn-esql-ast/src/parser/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down

0 comments on commit 21e0bc9

Please sign in to comment.