Skip to content

CLIENT-4526 Fix error listener's silent omitting#67

Merged
agrgr merged 5 commits intomainfrom
CLIENT-4526-fix-error-handling
Mar 27, 2026
Merged

CLIENT-4526 Fix error listener's silent omitting#67
agrgr merged 5 commits intomainfrom
CLIENT-4526-fix-error-handling

Conversation

@agrgr
Copy link
Copy Markdown
Collaborator

@agrgr agrgr commented Mar 26, 2026

No description provided.

@agrgr agrgr requested a review from tim-aero March 26, 2026 21:52
@agrgr agrgr added the bug Something isn't working label Mar 26, 2026
@agrgr agrgr marked this pull request as ready for review March 26, 2026 21:53
Comment on lines +67 to +69
if (errorListener.hasErrors()) {
throw new DslParseException("Could not parse given DSL expression input");
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would seem to lose any useful information about what the errors are, which is critical. Does the DSLParserErrorListener get information from the parse about what the error is?

Copy link
Copy Markdown
Collaborator Author

@agrgr agrgr Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, DSLParserErrorListener does receive detailed error information from ANTLR. The listener only used it for the special float-literal paths, where it threw immediately with a specific message, while error message was printed to stderr. It makes sense now to propagate error message from ANTLR, will push a change soon.

@agrgr
Copy link
Copy Markdown
Collaborator Author

agrgr commented Mar 26, 2026

Now a combined error message from lexer and parser is propagated via an exception rather than to stderr.

@agrgr agrgr requested a review from tim-aero March 26, 2026 23:30
Copy link
Copy Markdown

@tim-aero tim-aero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@agrgr agrgr merged commit 56c60ee into main Mar 27, 2026
12 checks passed
@agrgr agrgr deleted the CLIENT-4526-fix-error-handling branch March 27, 2026 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants