Skip to content

Commit

Permalink
exposed funnel errors
Browse files Browse the repository at this point in the history
  • Loading branch information
aspicer committed Sep 5, 2024
1 parent 6f6a80a commit 2c97f8b
Show file tree
Hide file tree
Showing 9 changed files with 35 additions and 34 deletions.
18 changes: 9 additions & 9 deletions posthog/hogql_queries/insights/funnels/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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}")

Check failure on line 198 in posthog/hogql_queries/insights/funnels/base.py

View workflow job for this annotation

GitHub Actions / Python code quality checks

Unexpected keyword argument "detail" for "ExposedHogQLError"

def _format_results(
self, results
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions posthog/hogql_queries/insights/funnels/funnel.py
Original file line number Diff line number Diff line change
@@ -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


Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
6 changes: 3 additions & 3 deletions posthog/hogql_queries/insights/funnels/funnel_event_query.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -14,7 +15,6 @@
FunnelExclusionActionsNode,
FunnelExclusionEventsNode,
)
from rest_framework.exceptions import ValidationError


class FunnelEventQuery:
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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!'
)

Expand Down
12 changes: 7 additions & 5 deletions posthog/hogql_queries/insights/funnels/funnel_trends_persons.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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!"
)

Expand Down
4 changes: 2 additions & 2 deletions posthog/hogql_queries/insights/funnels/funnel_unordered.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions posthog/hogql_queries/insights/funnels/utils.py
Original file line number Diff line number Diff line change
@@ -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):
Expand Down Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 2c97f8b

Please sign in to comment.