From 2c97f8bd7ae9f9febb14e3afa696cdc24bd95a5d Mon Sep 17 00:00:00 2001 From: Alexander Spicer Date: Thu, 5 Sep 2024 11:30:38 -0700 Subject: [PATCH] exposed funnel errors --- posthog/hogql_queries/insights/funnels/base.py | 18 +++++++++--------- .../hogql_queries/insights/funnels/funnel.py | 4 ++-- .../funnels/funnel_correlation_query_runner.py | 12 ++++++------ .../insights/funnels/funnel_event_query.py | 6 +++--- .../insights/funnels/funnel_time_to_convert.py | 5 ++--- .../insights/funnels/funnel_trends_persons.py | 12 +++++++----- .../insights/funnels/funnel_unordered.py | 4 ++-- .../hogql_queries/insights/funnels/utils.py | 4 ++-- requirements.txt | 4 ++-- 9 files changed, 35 insertions(+), 34 deletions(-) diff --git a/posthog/hogql_queries/insights/funnels/base.py b/posthog/hogql_queries/insights/funnels/base.py index 84ddf66eafccd..330ad3bf804e7 100644 --- a/posthog/hogql_queries/insights/funnels/base.py +++ b/posthog/hogql_queries/insights/funnels/base.py @@ -3,11 +3,11 @@ from functools import cached_property from typing import Any, Optional, Union, cast -from rest_framework.exceptions import ValidationError from posthog.clickhouse.materialized_columns.column import ColumnName from posthog.hogql import ast from posthog.hogql.constants import get_breakdown_limit_for_context +from posthog.hogql.errors import ExposedHogQLError from posthog.hogql.parser import parse_expr, parse_select from posthog.hogql.property import action_to_expr, property_to_expr from posthog.hogql_queries.insights.funnels.funnel_aggregation_operations import FirstTimeForUserAggregationQuery @@ -61,23 +61,23 @@ def __init__(self, context: FunnelQueryContext): if self.context.funnelsFilter.exclusions is not None: for exclusion in self.context.funnelsFilter.exclusions: if exclusion.funnelFromStep >= exclusion.funnelToStep: - raise ValidationError( + raise ExposedHogQLError( "Exclusion event range is invalid. End of range should be greater than start." ) if exclusion.funnelFromStep >= len(self.context.query.series) - 1: - raise ValidationError( + raise ExposedHogQLError( "Exclusion event range is invalid. Start of range is greater than number of steps." ) if exclusion.funnelToStep > len(self.context.query.series) - 1: - raise ValidationError( + raise ExposedHogQLError( "Exclusion event range is invalid. End of range is greater than number of steps." ) for entity in self.context.query.series[exclusion.funnelFromStep : exclusion.funnelToStep + 1]: if is_equal(entity, exclusion) or is_superset(entity, exclusion): - raise ValidationError("Exclusion steps cannot contain an event that's part of funnel steps.") + raise ExposedHogQLError("Exclusion steps cannot contain an event that's part of funnel steps.") def get_query(self) -> ast.SelectQuery: raise NotImplementedError() @@ -195,7 +195,7 @@ def _get_breakdown_expr(self) -> ast.Expr: elif breakdownType == "data_warehouse_person_property" and isinstance(breakdown, str): return ast.Field(chain=["person", *breakdown.split(".")]) else: - raise ValidationError(detail=f"Unsupported breakdown type: {breakdownType}") + raise ExposedHogQLError(detail=f"Unsupported breakdown type: {breakdownType}") def _format_results( self, results @@ -299,7 +299,7 @@ def _serialize_step( action_id = step.event type = "events" elif isinstance(step, DataWarehouseNode): - raise ValidationError( + raise ExposedHogQLError( "Data warehouse tables are not supported in funnels just yet. For now, please try this funnel without the data warehouse-based step." ) else: @@ -396,7 +396,7 @@ def _get_inner_event_query( if breakdown and breakdownType == BreakdownType.COHORT: if funnel_events_query.select_from is None: - raise ValidationError("Apologies, there was an error adding cohort breakdowns to the query.") + raise ExposedHogQLError("Apologies, there was an error adding cohort breakdowns to the query.") funnel_events_query.select_from.next_join = self._get_cohort_breakdown_join() if not skip_step_filter: @@ -586,7 +586,7 @@ def _build_step_query( action = Action.objects.get(pk=int(entity.id), team=self.context.team) event_expr = action_to_expr(action) elif isinstance(entity, DataWarehouseNode): - raise ValidationError( + raise ExposedHogQLError( "Data warehouse tables are not supported in funnels just yet. For now, please try this funnel without the data warehouse-based step." ) elif entity.event is None: diff --git a/posthog/hogql_queries/insights/funnels/funnel.py b/posthog/hogql_queries/insights/funnels/funnel.py index 470979d32b26d..8521908ff3c99 100644 --- a/posthog/hogql_queries/insights/funnels/funnel.py +++ b/posthog/hogql_queries/insights/funnels/funnel.py @@ -1,8 +1,8 @@ from posthog.hogql import ast +from posthog.hogql.errors import ExposedHogQLError from posthog.hogql.parser import parse_expr from posthog.hogql_queries.insights.funnels.base import FunnelBase -from rest_framework.exceptions import ValidationError from posthog.schema import BreakdownType @@ -68,7 +68,7 @@ def get_step_counts_query(self): def get_step_counts_without_aggregation_query(self): max_steps = self.context.max_steps if max_steps < 2: - raise ValidationError("Funnels require at least two steps before calculating.") + raise ExposedHogQLError("Funnels require at least two steps before calculating.") formatted_query = self._build_step_subquery(2, max_steps) breakdown_exprs = self._get_breakdown_prop_expr() diff --git a/posthog/hogql_queries/insights/funnels/funnel_correlation_query_runner.py b/posthog/hogql_queries/insights/funnels/funnel_correlation_query_runner.py index e37ee95ce6b48..9b8432931cba3 100644 --- a/posthog/hogql_queries/insights/funnels/funnel_correlation_query_runner.py +++ b/posthog/hogql_queries/insights/funnels/funnel_correlation_query_runner.py @@ -2,6 +2,7 @@ from typing import Literal, Optional, Any, TypedDict, cast from posthog.constants import AUTOCAPTURE_EVENT +from posthog.hogql.errors import ExposedHogQLError from posthog.hogql.parser import parse_select from posthog.hogql.property import property_to_expr from posthog.hogql_queries.insights.funnels.funnel_event_query import FunnelEventQuery @@ -11,7 +12,6 @@ from posthog.models.action.action import Action from posthog.models.element.element import chain_to_elements from posthog.models.event.util import ElementSerializer -from rest_framework.exceptions import ValidationError from posthog.hogql import ast from posthog.hogql.constants import LimitContext @@ -359,7 +359,7 @@ def events_actor_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: assert self.correlation_actors_query is not None if not self.correlation_actors_query.funnelCorrelationPersonEntity: - raise ValidationError("No entity for persons specified") + raise ExposedHogQLError("No entity for persons specified") assert isinstance(self.correlation_actors_query.funnelCorrelationPersonEntity, EventsNode) @@ -428,7 +428,7 @@ def properties_actor_query( assert self.correlation_actors_query is not None if not self.correlation_actors_query.funnelCorrelationPropertyValues: - raise ValidationError("Property Correlation expects atleast one Property to get persons for") + raise ExposedHogQLError("Property Correlation expects at least one Property to get persons for") target_step = self.context.max_steps funnel_persons_query = self.get_funnel_actors_cte() @@ -543,7 +543,7 @@ def get_event_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: def get_event_property_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: if not self.query.funnelCorrelationEventNames: - raise ValidationError("Event Property Correlation expects atleast one event name to run correlation on") + raise ExposedHogQLError("Event Property Correlation expects at least one event name to run correlation on") funnel_persons_query = self.get_funnel_actors_cte() event_join_query = self._get_events_join_query() @@ -641,7 +641,7 @@ def get_event_property_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: def get_properties_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: if not self.query.funnelCorrelationNames: - raise ValidationError("Property Correlation expects atleast one Property to run correlation on") + raise ExposedHogQLError("Property Correlation expects atleast one Property to run correlation on") funnel_persons_query = self.get_funnel_actors_cte() target_step = self.context.max_steps @@ -829,7 +829,7 @@ def _get_funnel_step_names(self) -> list[str]: if entity.event is not None: events.add(entity.event) else: - raise ValidationError("Data warehouse nodes are not supported here") + raise ExposedHogQLError("Data warehouse nodes are not supported here") return sorted(events) diff --git a/posthog/hogql_queries/insights/funnels/funnel_event_query.py b/posthog/hogql_queries/insights/funnels/funnel_event_query.py index c4cb9507534ef..499d3e0cc6ed2 100644 --- a/posthog/hogql_queries/insights/funnels/funnel_event_query.py +++ b/posthog/hogql_queries/insights/funnels/funnel_event_query.py @@ -1,6 +1,7 @@ from typing import Union, Optional from posthog.clickhouse.materialized_columns.column import ColumnName from posthog.hogql import ast +from posthog.hogql.errors import ExposedHogQLError from posthog.hogql.parser import parse_expr from posthog.hogql_queries.insights.funnels.funnel_query_context import FunnelQueryContext from posthog.hogql_queries.insights.utils.properties import Properties @@ -14,7 +15,6 @@ FunnelExclusionActionsNode, FunnelExclusionEventsNode, ) -from rest_framework.exceptions import ValidationError class FunnelEventQuery: @@ -148,11 +148,11 @@ def _entity_expr(self, skip_entity_filter: bool) -> ast.Expr | None: action = Action.objects.get(pk=int(node.id), team=team) events.update(action.get_step_events()) except Action.DoesNotExist: - raise ValidationError(f"Action ID {node.id} does not exist!") + raise ExposedHogQLError(f"Action ID {node.id} does not exist!") elif isinstance(node, DataWarehouseNode): continue # Data warehouse nodes aren't based on events else: - raise ValidationError("Series and exclusions must be compose of action and event nodes") + raise ExposedHogQLError("Series and exclusions must be compose of action and event nodes") # Disable entity pre-filtering for "All events" if None in events: diff --git a/posthog/hogql_queries/insights/funnels/funnel_time_to_convert.py b/posthog/hogql_queries/insights/funnels/funnel_time_to_convert.py index 38600dcc1fec6..24b392ce6c17b 100644 --- a/posthog/hogql_queries/insights/funnels/funnel_time_to_convert.py +++ b/posthog/hogql_queries/insights/funnels/funnel_time_to_convert.py @@ -1,7 +1,6 @@ -from rest_framework.exceptions import ValidationError - from posthog.constants import FUNNEL_TO_STEP from posthog.hogql import ast +from posthog.hogql.errors import ExposedHogQLError from posthog.hogql.parser import parse_select from posthog.hogql_queries.insights.funnels.base import FunnelBase from posthog.hogql_queries.insights.funnels.funnel_query_context import FunnelQueryContext @@ -54,7 +53,7 @@ def get_query(self) -> ast.SelectQuery: """ if not (0 < to_step < len(query.series)): - raise ValidationError( + raise ExposedHogQLError( f'Filter parameter {FUNNEL_TO_STEP} can only be one of {", ".join(map(str, range(1, len(query.series))))} for time to convert!' ) diff --git a/posthog/hogql_queries/insights/funnels/funnel_trends_persons.py b/posthog/hogql_queries/insights/funnels/funnel_trends_persons.py index d6e53d0c72820..fa38c9d3734fe 100644 --- a/posthog/hogql_queries/insights/funnels/funnel_trends_persons.py +++ b/posthog/hogql_queries/insights/funnels/funnel_trends_persons.py @@ -1,8 +1,8 @@ from datetime import datetime -from rest_framework.exceptions import ValidationError from posthog.hogql import ast +from posthog.hogql.errors import ExposedHogQLError from posthog.hogql.parser import parse_expr from posthog.hogql_queries.insights.funnels.funnel_query_context import FunnelQueryContext from posthog.hogql_queries.insights.funnels.funnel_trends import FunnelTrends @@ -19,19 +19,21 @@ def __init__(self, context: FunnelQueryContext, just_summarize=False): team, actorsQuery = self.context.team, self.context.actorsQuery if actorsQuery is None: - raise ValidationError("No actors query present.") + raise ExposedHogQLError("No actors query present.") if actorsQuery.funnelTrendsDropOff is None: - raise ValidationError(f"Actors parameter `funnelTrendsDropOff` must be provided for funnel trends persons!") + raise ExposedHogQLError( + f"Actors parameter `funnelTrendsDropOff` must be provided for funnel trends persons!" + ) if actorsQuery.funnelTrendsEntrancePeriodStart is None: - raise ValidationError( + raise ExposedHogQLError( f"Actors parameter `funnelTrendsEntrancePeriodStart` must be provided funnel trends persons!" ) entrancePeriodStart = relative_date_parse(actorsQuery.funnelTrendsEntrancePeriodStart, team.timezone_info) if entrancePeriodStart is None: - raise ValidationError( + raise ExposedHogQLError( f"Actors parameter `funnelTrendsEntrancePeriodStart` must be a valid relative date string!" ) diff --git a/posthog/hogql_queries/insights/funnels/funnel_unordered.py b/posthog/hogql_queries/insights/funnels/funnel_unordered.py index f718c7fe3e4df..efb5b84715a5a 100644 --- a/posthog/hogql_queries/insights/funnels/funnel_unordered.py +++ b/posthog/hogql_queries/insights/funnels/funnel_unordered.py @@ -1,8 +1,8 @@ from typing import Any, Optional import uuid -from rest_framework.exceptions import ValidationError from posthog.hogql import ast +from posthog.hogql.errors import ExposedHogQLError from posthog.hogql.parser import parse_expr from posthog.hogql_queries.insights.funnels.base import FunnelBase from posthog.hogql_queries.insights.funnels.utils import funnel_window_interval_unit_to_sql @@ -41,7 +41,7 @@ def get_query(self): for exclusion in self.context.funnelsFilter.exclusions or []: if exclusion.funnelFromStep != 0 or exclusion.funnelToStep != max_steps - 1: - raise ValidationError("Partial Exclusions not allowed in unordered funnels") + raise ExposedHogQLError("Partial Exclusions not allowed in unordered funnels") if self.context.breakdown and self.context.breakdownType in [ BreakdownType.PERSON, diff --git a/posthog/hogql_queries/insights/funnels/utils.py b/posthog/hogql_queries/insights/funnels/utils.py index d5c968a913494..58d325a3df012 100644 --- a/posthog/hogql_queries/insights/funnels/utils.py +++ b/posthog/hogql_queries/insights/funnels/utils.py @@ -1,8 +1,8 @@ from posthog.constants import FUNNEL_WINDOW_INTERVAL_TYPES from posthog.hogql import ast +from posthog.hogql.errors import ExposedHogQLError from posthog.hogql.parser import parse_expr from posthog.schema import FunnelConversionWindowTimeUnit, FunnelVizType, FunnelsFilter, StepOrderValue -from rest_framework.exceptions import ValidationError def get_funnel_order_class(funnelsFilter: FunnelsFilter): @@ -56,7 +56,7 @@ def funnel_window_interval_unit_to_sql( elif funnelWindowIntervalUnit == "day": return "DAY" else: - raise ValidationError(f"{funnelWindowIntervalUnit} not supported") + raise ExposedHogQLError(f"{funnelWindowIntervalUnit} not supported") def get_breakdown_expr( diff --git a/requirements.txt b/requirements.txt index c8d3e50b4256c..fa946efd3001a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -345,7 +345,7 @@ langsmith==0.1.106 # -r requirements.in # langchain # langchain-core -lxml==4.9.4 +lxml==5.2.2 # via # -r requirements.in # python3-saml @@ -754,7 +754,7 @@ wrapt==1.15.0 # via aiobotocore wsproto==1.2.0 # via trio-websocket -xmlsec==1.3.13 +xmlsec==1.3.14 # via # -r requirements.in # python3-saml