Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Render the error message of failed validation as a string #3195

Open
Tracked by #3556
brokensound77 opened this issue Oct 16, 2023 · 3 comments · Fixed by #4299 · May be fixed by #4329
Open
Tracked by #3556

Render the error message of failed validation as a string #3195

brokensound77 opened this issue Oct 16, 2023 · 3 comments · Fixed by #4299 · May be fixed by #4329
Assignees
Labels

Comments

@brokensound77
Copy link
Contributor

The error message for failed query validation is a json object, which renders improperly, making the type hinting useless. We should render it as a string prior to passing it (json.dumps) to be readable in failed unit tests

ex:
link

E Error in both stack and integrations checks: {'stack': KqlParseError('Error at line:2,column:5\nUnknown field\n(event.dataset:network_traffic.http or (event.category:network_traffic and network.protocol:http)) and\n http.response.status_phrase:ok and destination.port:9200 and network.direction:inbound and\n ^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\nTry adding event.module or event.dataset to specify beats module\n\nstack: 8.12.0, beats: 8.10.3, ecs: 8.10.0'), 'integrations': KqlParseError('Error at line:3,column:9\nUnknown field\n(event.dataset:network_traffic.http or (event.category:network_traffic and network.protocol:http)) and\n http.response.status_phrase:ok and destination.port:9200 and network.direction:inbound and\n not http.response.headers.content-type:"image/x-icon" and\n ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n\tTry adding event.module or event.dataset to specify integration module\n\tWill check against integrations ['network_traffic'] combined.\n\tpackage='network_traffic', integration='http', package_version='1.25.1', stack_version='8.10.0', ecs_version='8.10.0'')}

@botelastic
Copy link

botelastic bot commented Dec 15, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@botelastic botelastic bot added the stale 60 days of inactivity label Dec 15, 2023
@Mikaayenson Mikaayenson added backlog and removed stale 60 days of inactivity labels Dec 15, 2023
@Mikaayenson Mikaayenson removed their assignment Nov 1, 2024
@shashank-elastic shashank-elastic self-assigned this Nov 6, 2024
@shashank-elastic
Copy link
Contributor

Notable Points from @traut

  • The rule of thumb in a good design is to have a static error message which is a unique string and describe the error
  • The details of the error should be logged properly before we throw the error
  • And the format of the error details can happen outside of ValueError scope

@shashank-elastic
Copy link
Contributor

Reopening this to fix in KQL Parser as well, when we get to it

@shashank-elastic shashank-elastic linked a pull request Jan 2, 2025 that will close this issue
5 tasks
@shashank-elastic shashank-elastic linked a pull request Jan 2, 2025 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants