Skip to content

Conversation

Twixes
Copy link
Member

@Twixes Twixes commented Jun 8, 2023

Problem

Users are confused when their HogQL isn't right and the problem is only caught in the ClickHouse layer, and not HogQL.

Changes

We now expose selected CH error types in the HogQL full query endpoint.

Screenshot 2023-06-08 at 14 45 35

How did you test this code?

API test.

@Twixes Twixes requested a review from mariusandra June 8, 2023 12:59
@PostHog PostHog deleted a comment from posthog-bot Jun 8, 2023
@@ -100,6 +101,8 @@
return JsonResponse(process_query(self.team, query_json), safe=False)
except HogQLException as e:
raise ValidationError(str(e))
except ExposedCHQueryError as e:
raise ValidationError(str(e), e.code_name)

Check warning

Code scanning / CodeQL

Information exposure through an exception

[Stack trace information](1) flows to this location and may be exposed to an external user.
Copy link
Member Author

Choose a reason for hiding this comment

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

We're only exposing the error message for error we've deemed safe, so this is intentional

@@ -110,6 +113,8 @@
return JsonResponse(process_query(self.team, query_json), safe=False)
except HogQLException as e:
raise ValidationError(str(e))
except ExposedCHQueryError as e:
raise ValidationError(str(e), e.code_name)

Check warning

Code scanning / CodeQL

Information exposure through an exception

[Stack trace information](1) flows to this location and may be exposed to an external user.
Copy link
Member Author

Choose a reason for hiding this comment

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

Same as the other one

Copy link
Collaborator

@mariusandra mariusandra left a comment

Choose a reason for hiding this comment

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

Love it! ❤️

@Twixes Twixes merged commit 6404ed2 into master Jun 8, 2023
@Twixes Twixes deleted the expose-hogql-errors branch June 8, 2023 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants