diff --git a/.sampo/changesets/call-disable-geoip-event-value.md b/.sampo/changesets/call-disable-geoip-event-value.md new file mode 100644 index 00000000..ae143d61 --- /dev/null +++ b/.sampo/changesets/call-disable-geoip-event-value.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: major +--- + +A `disable_geoip` argument to a call now counts as a value of that event. It wins over a `$geoip_disable` in context tags or `super_properties`, and a `$geoip_disable` in the call's own `properties` still wins over it. `disable_geoip=False` now sends `$geoip_disable: false`, so it turns GeoIP lookup on for that event even when a context tag or `super_properties` turns it off. The client's `disable_geoip` setting keeps its place below every caller value. `AsyncPosthog` follows the same rules. diff --git a/.sampo/changesets/exception-level-and-reserved-properties.md b/.sampo/changesets/exception-level-and-reserved-properties.md new file mode 100644 index 00000000..f047fe3d --- /dev/null +++ b/.sampo/changesets/exception-level-and-reserved-properties.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: major +--- + +`capture_exception` takes a `level=` argument on `Client`, `AsyncPosthog` and the module, which sets `$exception_level` (default `"error"`). Reserved exception properties passed in `properties`, such as `$exception_list` and `$exception_level`, are now ignored. In 7.x they overrode the SDK's values with a `DeprecationWarning`. `AsyncPosthog.capture_exception` now sends `$exception_level`. diff --git a/.sampo/changesets/invalid-uuid-warning.md b/.sampo/changesets/invalid-uuid-warning.md new file mode 100644 index 00000000..d89614ea --- /dev/null +++ b/.sampo/changesets/invalid-uuid-warning.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: major +--- + +An invalid event `uuid` is still replaced with a generated one, but the SDK now logs one warning instead of an error, and the warning no longer includes the value. An empty string `uuid` counts as unset, so the SDK generates one without logging. `AsyncPosthog` follows the same rules. diff --git a/.sampo/changesets/request-keyword-only.md b/.sampo/changesets/request-keyword-only.md new file mode 100644 index 00000000..4337cd2b --- /dev/null +++ b/.sampo/changesets/request-keyword-only.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: major +--- + +The parameters after `path` in `posthog.request.post` and after `host` in `posthog.request.flags` are keyword-only. With `gzip` removed, a 7.x call that passed them by position bound them to the wrong parameter. It now raises `TypeError`. diff --git a/.sampo/changesets/session-id-drop-warning.md b/.sampo/changesets/session-id-drop-warning.md new file mode 100644 index 00000000..aad2adb4 --- /dev/null +++ b/.sampo/changesets/session-id-drop-warning.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: major +--- + +A `$session_id` or `$window_id` that is not a string is not sent, because capture v1 would reject the whole batch, and the SDK logs a warning for each one. The warning names the key and the value's type, not the value. `None` counts as unset and drops without a warning. An empty string is sent. In 7.x these values were sent as properties. diff --git a/docs/migration-7.x-to-8.0.md b/docs/migration-7.x-to-8.0.md index 005006d5..ab743c7a 100644 --- a/docs/migration-7.x-to-8.0.md +++ b/docs/migration-7.x-to-8.0.md @@ -9,6 +9,8 @@ Read the checklist first, then the sections that apply to you. You need to change code if your app does any of these: - passes `Client` or `AsyncPosthog` constructor arguments by position after `host` +- calls `posthog.request.post` or `posthog.request.flags` with positional arguments after `path` or `host` +- sets `$exception_level` or another reserved exception property in `capture_exception(properties=...)` - sets `capture_mode`, `POSTHOG_CAPTURE_MODE` or `gzip` - imports `CaptureV1Error`, `posthog.capture_v1`, `request.batch_post`, `EVENTS_ENDPOINT` or `AI_EVENTS_ENDPOINT` - sends events to a self-hosted PostHog that does not serve the capture v1 endpoints @@ -58,7 +60,7 @@ To find MCP traffic, filter on the `$mcp_*` events and properties instead of `$l | `CaptureV1Error` (`from posthog.capture_v1 import CaptureV1Error`) | `CaptureError` (`from posthog import CaptureError`). There is no alias. | | `posthog.capture_v1` | `posthog.capture_event` and `posthog.capture_send` | | `request.batch_post`, `async_batch_post`, `EVENTS_ENDPOINT`, `AI_EVENTS_ENDPOINT` | Nothing. Send events through a client. | -| The `gzip` parameter of `request.post` and `request.flags` | Nothing. | +| The `gzip` parameter of `request.post` and `request.flags` | Nothing. The parameters after `path` in `post` and after `host` in `flags` are keyword-only, so a 7.x call that passes them by position raises `TypeError`. | | The `backoff` dependency | Add it to your own requirements if your code imports it. | The `posthoganalytics` package has the same changes. For example, import `CaptureError` from `posthoganalytics`. @@ -100,7 +102,7 @@ When the whole request is over the billing limit, capture returns a `402` with n - Each event needs its own uuid. Capture rejects a whole batch that contains the same uuid twice, so the other events in that batch are lost too. - The SDK accepts a uuid with hyphens, 32 hex digits, `{...}` braces or a `urn:uuid:` prefix, in any case. It sends the lowercase hyphenated form and returns that form from `capture`. -- Any other value is replaced with a generated uuid, and the SDK logs an error. This applies to `AsyncPosthog` too. +- Any other value is replaced with a generated uuid, and the SDK logs one warning that names the rule, not the value. An empty string counts as unset, so the SDK generates a uuid without a warning. This applies to `AsyncPosthog` too. - Generated uuids are UUIDv7. ## Session and window IDs @@ -110,7 +112,13 @@ The SDK moves them out of `properties` for you. - A string is sent as given, including `""`. - `None` counts as unset and is removed. -- Any other value is removed and not sent, because capture would reject the whole batch. 7.x sent it as a property. +- Any other value is removed and not sent, because capture would reject the whole batch. 7.x sent it as a property. The SDK logs a warning for each one, with the key and the value's type but not the value. + +## Exceptions + +- `capture_exception` takes a `level=` argument on `Client`, `AsyncPosthog` and the module. It sets `$exception_level`, for example `"warning"` or `"fatal"`. The default is `"error"`, and an unknown value counts as unset. +- `capture_exception` ignores reserved exception properties in `properties`, such as `$exception_list`, `$exception_level` and `$exception_source`. In 7.x they overrode the SDK's values, with a `DeprecationWarning`. Use `level=` instead of `$exception_level`. +- `AsyncPosthog.capture_exception` now sends `$exception_level`. ## Event options @@ -141,10 +149,10 @@ Capture v1 sends processing options in an `options` object, next to `properties` Values apply in this order. Steps 2 to 4 fill only the options and properties that the steps before them left unset: -1. the `options` and `properties` of the call +1. the `options` and `properties` of the call, then the call's `disable_geoip` argument, which fills `$geoip_disable` 2. context options and tags 3. `super_options` and `super_properties` -4. values the SDK sets: `$is_server` from `is_server`, `$geoip_disable` from `disable_geoip`, system properties such as `$os` and `$python_version`, `options.process_person_profile = false` for events without a distinct ID, and `$release_id` from `POSTHOG_RELEASE_ID` +4. values the SDK sets: `$is_server` from `is_server`, `$geoip_disable` from the client's `disable_geoip` setting, system properties such as `$os` and `$python_version`, `options.process_person_profile = false` for events without a distinct ID, and `$release_id` from `POSTHOG_RELEASE_ID` 5. `before_send`, which sees the result of steps 1 to 4 and can change or remove any of it 6. legacy properties, which fill unset options and are then removed @@ -157,6 +165,7 @@ These changes follow from this order: - Properties passed to a call now override `super_properties`. `super_properties` can no longer change `$lib` or `$lib_version`. - A `$is_server`, `$geoip_disable` or system property such as `$os` that you set in a call or a context tag now wins over the SDK's value. In 7.x the SDK overwrote it. `super_properties` win over the SDK's value too, so `super_properties={"$geoip_disable": False}` turns GeoIP lookup on for events, even with `disable_geoip=True`. +- A `disable_geoip` argument to a call belongs to that event, so it wins over context tags and `super_properties`. A `$geoip_disable` in the call's own `properties` still wins over it. `disable_geoip=False` now sends `$geoip_disable: false`. In 7.x it sent no `$geoip_disable` property. - The `groups` argument merges into a `$groups` property of the call, and wins key by key. In 7.x it replaced the property. MCP events merge their identity's groups into a custom `$groups` property the same way. - A `$set`, `$set_once`, `$groups` or `$group_set` in `super_properties` no longer replaces the whole value of the call. The two merge, and the call wins key by key. - An event without a distinct ID gets `options.process_person_profile = false`. A `$process_person_profile: true` property no longer turns person processing back on. Set the option instead. diff --git a/posthog/__init__.py b/posthog/__init__.py index 0d47168a..1bac20e1 100644 --- a/posthog/__init__.py +++ b/posthog/__init__.py @@ -786,6 +786,8 @@ def alias( def capture_exception( exception: Optional[ExceptionArg] = None, + *, + level: Optional[str] = None, **kwargs: Unpack[OptionalCaptureArgs], ) -> Optional[str]: """ @@ -793,10 +795,12 @@ def capture_exception( Args: exception: The exception to capture. If not provided, the current exception is captured via `sys.exc_info()` + level: The ``$exception_level``, such as ``"warning"`` or ``"fatal"``. + Defaults to ``"error"``. An unknown value counts as unset. **kwargs: Optional capture arguments including distinct_id, properties, timestamp, uuid, groups, flags, send_feature_flags, disable_geoip, and options. - Overriding reserved exception properties through ``properties`` is - deprecated and will stop working in the next major version. + Reserved exception properties in ``properties``, such as + ``$exception_list`` and ``$exception_level``, are ignored. Details: Capture exception is idempotent - if it is called twice with the same exception instance, only a occurrence will be tracked in posthog. This is because, generally, contexts will cause exceptions to be captured automatically. However, to ensure you track an exception, if you catch and do not re-raise it, capturing it manually is recommended, unless you are certain it will have crossed a context boundary (e.g. by existing a `with posthog.new_context():` block already). If the passed exception was raised and caught, the captured stack trace will consist of every frame between where the exception was raised and the point at which it is captured (the "traceback"). If the passed exception was never raised, e.g. if you call `posthog.capture_exception(ValueError("Some Error"))`, the stack trace captured will be the full stack trace at the moment the exception was captured. Note that heavy use of contexts will lead to truncated stack traces, as the exception will be captured by the context entered most recently, which may not be the point you catch the exception for the final time in your code. It's recommended to use contexts sparingly, for this reason. `capture_exception` takes the same set of optional arguments as `capture`. @@ -814,7 +818,7 @@ def capture_exception( Events """ - return _proxy("capture_exception", exception=exception, **kwargs) + return _proxy("capture_exception", exception=exception, level=level, **kwargs) def feature_enabled( diff --git a/posthog/async_client.py b/posthog/async_client.py index 32e2d7bd..09abdd45 100644 --- a/posthog/async_client.py +++ b/posthog/async_client.py @@ -39,11 +39,11 @@ ) from .capture_event import ( _build_event_defaults, - _canonical_event_uuid, _event_options, _EventDefaults, _fill_event_defaults, _merge_groups, + _resolve_event_uuid, ) from .capture_send import _CAPTURE_AI_V1_PATH, _CAPTURE_V1_PATH from .client import ( @@ -76,7 +76,9 @@ DEFAULT_CODE_VARIABLES_IGNORE_PATTERNS, DEFAULT_CODE_VARIABLES_MASK_PATTERNS, DEFAULT_CODE_VARIABLES_MASK_URL_CREDENTIALS, + _exception_level, _get_current_otel_span_properties, + _without_reserved_exception_properties, exc_info_from_error, exception_is_already_captured, exceptions_from_error_tuple, @@ -96,7 +98,6 @@ from .utils import ( SizeLimitedDict, _normalize_timestamp, - _uuid7, clean, system_context, ) @@ -445,18 +446,7 @@ def enqueue_on_bound_loop() -> None: return admitted.result() def _normalize_uuid(self, msg: dict[str, Any]) -> str: - raw_uuid = msg.pop("uuid", None) - if raw_uuid is not None: - normalized = _canonical_event_uuid(raw_uuid) - if normalized is None: - self.log.error( - "Invalid UUID %r. Falling back to a generated UUID.", raw_uuid - ) - else: - msg["uuid"] = normalized - return normalized - - normalized = str(_uuid7()) + normalized = _resolve_event_uuid(msg.pop("uuid", None)) msg["uuid"] = normalized return normalized @@ -531,8 +521,6 @@ def _event_defaults( disable_geoip: Optional[bool] = None, system_properties: Optional[dict[str, Any]] = None, ) -> _EventDefaults: - if disable_geoip is None: - disable_geoip = self.disable_geoip return _build_event_defaults( super_properties=self.super_properties, super_options=self.super_options, @@ -542,7 +530,8 @@ def _event_defaults( derived_options=derived_options, property_allowlist=property_allowlist, is_server=self.is_server, - disable_geoip=disable_geoip, + disable_geoip=self.disable_geoip, + call_disable_geoip=disable_geoip, system_properties=system_properties, ) @@ -873,9 +862,16 @@ def _enqueue_built_event( def capture_exception( self, exception: Optional[ExceptionArg] = None, + *, + level: Optional[str] = None, **kwargs: Unpack[OptionalCaptureArgs], ) -> Optional[str]: - """Capture an exception. This method never raises, including in debug mode.""" + """Capture an exception. This method never raises, including in debug mode. + + ``level`` sets ``$exception_level`` and defaults to ``"error"``. An unknown + value counts as unset. Reserved exception properties in ``properties``, + such as ``$exception_list`` and ``$exception_level``, are ignored. + """ try: if exception is not None and exception_is_already_captured(exception): self.log.debug("Exception already captured, skipping") @@ -897,8 +893,9 @@ def capture_exception( ) exceptions = event["exception"]["values"] properties = { + **_without_reserved_exception_properties(kwargs.get("properties")), "$exception_list": exceptions, - **(kwargs.get("properties") or {}), + "$exception_level": _exception_level(None, level), } context_enabled = get_capture_exception_code_variables_context() context_mask = get_code_variables_mask_patterns_context() diff --git a/posthog/capture_event.py b/posthog/capture_event.py index a7acabd7..a5c37f28 100644 --- a/posthog/capture_event.py +++ b/posthog/capture_event.py @@ -26,7 +26,7 @@ from typing import Any, Optional from uuid import UUID -from posthog.utils import _normalize_timestamp +from posthog.utils import _normalize_timestamp, _uuid7 from posthog.utils import clean as _clean log = logging.getLogger("posthog") @@ -78,6 +78,37 @@ def _canonical_event_uuid(value: Any) -> Optional[str]: return str(UUID(value.lower())) +def _resolve_event_uuid(value: Any) -> str: + """Return the canonical form of a caller's event uuid, or a generated one. + + A missing or empty uuid is generated silently. An invalid one is replaced + and logs one warning. The warning names the rule and not the value, + because callers can put their own data in the uuid field. + """ + if value is not None and value != "": + canonical = _canonical_event_uuid(value) + if canonical is not None: + return canonical + log.warning( + "Event uuid is not a valid UUID string or uuid.UUID. " + "Sending the event with a generated UUID." + ) + return str(_uuid7()) + + +def _json_type_name(value: Any) -> str: + """Name a value's JSON type, the same names posthog-go and posthog-rs log.""" + if isinstance(value, bool): + return "bool" + if isinstance(value, (int, float)): + return "number" + if isinstance(value, (list, tuple)): + return "array" + if isinstance(value, Mapping): + return "object" + return type(value).__name__ + + def _event_options(value: Any) -> dict[str, Any]: """Return a copy of a caller's ``options``, or ``{}`` when it is not a dict.""" if value is None: @@ -93,7 +124,7 @@ def _event_options(value: Any) -> dict[str, Any]: @dataclass(frozen=True) class _EventDefaults: - """Context, global and SDK-derived values for one event, highest layer first. + """Per-call, context, global and SDK-derived values for one event, highest layer first. They fill in before ``before_send``, so the hook sees them and can change or remove them. The event's own values win over every default. @@ -115,9 +146,18 @@ def _build_event_defaults( property_allowlist: Optional[Collection[str]] = None, is_server: bool = False, disable_geoip: bool = False, + call_disable_geoip: Optional[bool] = None, system_properties: Optional[Mapping[str, Any]] = None, ) -> _EventDefaults: - """Order the layers: context, then global, then values the SDK derives.""" + """Order the layers: per-call arguments, context, global, then SDK values. + + ``disable_geoip`` is the client setting, an SDK value. ``call_disable_geoip`` + is the argument of one call: it is a value of that event, so only the + event's own ``$geoip_disable`` property beats it. + """ + call_properties = ( + {} if call_disable_geoip is None else {"$geoip_disable": call_disable_geoip} + ) # A value in the event, the context or super properties wins over every # value the SDK adds, including `$is_server` and `$geoip_disable`. sdk_properties = dict(system_properties or {}) @@ -129,6 +169,7 @@ def _build_event_defaults( sdk_properties["$geoip_disable"] = True return _EventDefaults( property_layers=( + call_properties, context_properties or {}, super_properties or {}, sdk_properties, @@ -242,10 +283,16 @@ def _to_v1_event(msg: dict) -> dict: if prop_key not in properties: continue # Always removed. A non-string value would fail the whole batch, so it - # is dropped. + # is dropped. None counts as unset and drops silently. value = properties.pop(prop_key) if isinstance(value, str): top_level[field_name] = value + elif value is not None: + log.warning( + "dropping %s: a %s value is not a string", + prop_key, + _json_type_name(value), + ) event = { "event": msg["event"], diff --git a/posthog/client.py b/posthog/client.py index cea28863..daed99dc 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -36,10 +36,10 @@ ) from posthog.capture_event import ( _build_event_defaults, - _canonical_event_uuid, _event_options, _fill_event_defaults, _merge_groups, + _resolve_event_uuid, ) from posthog.capture_send import ( _CAPTURE_AI_V1_PATH, @@ -78,10 +78,11 @@ exc_info_from_error, exception_is_already_captured, exceptions_from_error_tuple, + _exception_level, _get_current_otel_span_properties, handle_in_app, mark_exception_as_captured, - _normalize_exception_level, + _without_reserved_exception_properties, try_attach_code_variables_to_frames, ) from posthog.feature_flag_evaluations import ( @@ -136,7 +137,6 @@ SizeLimitedDict, clean, _normalize_timestamp, - _uuid7, guess_timezone as guess_timezone, system_context, ) @@ -273,15 +273,6 @@ def get_identity_state(passed) -> tuple[str, bool]: return (str(uuid4()), True) -def _stringify_event_uuid(value) -> str: - canonical = _canonical_event_uuid(value) - if canonical is None: - raise ValueError( - f"Invalid event uuid {value!r}. Expected a valid UUID string or uuid.UUID instance." - ) - return canonical - - def _personless_options(personless: bool) -> dict[str, Any]: """The lowest option layer: no person profile for a generated distinct ID.""" return {"process_person_profile": False} if personless else {} @@ -1691,9 +1682,10 @@ def capture( send_feature_flags: Deprecated. Prefer flags=... from evaluate_flags(). When truthy, evaluates flags during capture and attaches them to the event. - disable_geoip: Whether to disable GeoIP for this event. A - ``$geoip_disable`` property in the event, context tags or - ``super_properties`` wins. + disable_geoip: Whether to disable GeoIP for this event. It wins + over a ``$geoip_disable`` in context tags or + ``super_properties``. A ``$geoip_disable`` property in this + call's ``properties`` wins over it. options: Capture options for this event, such as ``{"process_person_profile": False}``. Sent as given, for PostHog to validate. They override context options and @@ -2199,6 +2191,8 @@ def alias( def capture_exception( self, exception: Optional[ExceptionArg], + *, + level: Optional[str] = None, **kwargs: Unpack[OptionalCaptureArgs], ) -> Optional[str]: """ @@ -2209,10 +2203,12 @@ def capture_exception( Args: exception: The exception to capture. + level: The ``$exception_level``, such as ``"warning"`` or ``"fatal"``. + Defaults to ``"error"``. An unknown value counts as unset. distinct_id: The distinct ID of the user. - properties: A dictionary of additional properties. Overriding reserved - exception properties is deprecated and will stop working in the next - major version. + properties: A dictionary of additional properties. Reserved exception + properties, such as ``$exception_list`` and ``$exception_level``, + are ignored. flags: A ``FeatureFlagEvaluations`` snapshot from ``evaluate_flags()``. Attaches those exact flag values to the captured `$exception` event. send_feature_flags: Deprecated. Pass ``flags`` from ``evaluate_flags()`` instead. @@ -2278,52 +2274,18 @@ def capture_exception( ) all_exceptions_with_trace_and_in_app = event["exception"]["values"] - reserved_properties = { - "$exception_list", - "$exception_level", - "$exception_source", - "$debug_images", - "$exception_handled", - "$exception_types", - "$exception_values", - "$exception_sources", - "$exception_functions", - "$exception_fingerprint_version", - "$exception_fingerprint_record", - "$exception_issue_id", - "$exception_release", - "$cymbal_errors", - } - reserved_property_overrides = reserved_properties.intersection(properties) - if reserved_property_overrides: - try: - warnings.warn( - "Reserved exception properties passed through " - "`capture_exception(properties=...)` currently override " - "SDK-owned metadata, but this behavior is deprecated and will " - "be removed in the next major version: " - + ", ".join(sorted(reserved_property_overrides)), - DeprecationWarning, - stacklevel=2, - ) - except DeprecationWarning: - # capture_exception must not drop an event when applications - # promote deprecation warnings to errors. - pass - caller_properties = properties properties = { **_get_current_otel_span_properties(), "$exception_list": all_exceptions_with_trace_and_in_app, - "$exception_level": _normalize_exception_level( - capture_metadata.get("level") - ) - or "error", + "$exception_level": _exception_level( + capture_metadata.get("level"), level + ), } source = capture_metadata.get("source") if isinstance(source, str) and source: properties["$exception_source"] = source - properties.update(caller_properties) + properties.update(_without_reserved_exception_properties(caller_properties)) context_enabled = get_capture_exception_code_variables_context() context_mask = get_code_variables_mask_patterns_context() @@ -2504,17 +2466,8 @@ def _reinit_after_fork(self): def _normalize_event_uuid(self, msg): # type: (...) -> None """Ensure `msg["uuid"]` is a valid uuid string, generating one if missing or invalid.""" - if "uuid" in msg: - uuid = msg.pop("uuid") - if uuid is not None: - try: - msg["uuid"] = _stringify_event_uuid(uuid) - except ValueError as e: - self.log.error("%s Falling back to a generated UUID.", e) - - if "uuid" not in msg: - # Always send a uuid, so we can always return one - msg["uuid"] = str(_uuid7()) + # Always send a uuid, so we can always return one + msg["uuid"] = _resolve_event_uuid(msg.pop("uuid", None)) def _report_capture_failure( self, error: Exception, batch: list[dict], endpoint: str @@ -2575,9 +2528,6 @@ def _enqueue( msg["properties"]["$lib"] = self._library_id msg["properties"]["$lib_version"] = self._library_version - if disable_geoip is None: - disable_geoip = self.disable_geoip - msg["options"] = msg.get("options") or {} _fill_event_defaults( @@ -2591,7 +2541,8 @@ def _enqueue( derived_options=derived_options, property_allowlist=property_allowlist, is_server=self.is_server, - disable_geoip=disable_geoip, + disable_geoip=self.disable_geoip, + call_disable_geoip=disable_geoip, system_properties=system_properties, ), ) diff --git a/posthog/exception_utils.py b/posthog/exception_utils.py index 8608b96f..65037ca0 100644 --- a/posthog/exception_utils.py +++ b/posthog/exception_utils.py @@ -632,6 +632,47 @@ def _normalize_exception_level(level): return _EXCEPTION_LEVELS.get(level.lower()) if isinstance(level, str) else None +# Exception properties the SDK or PostHog owns. A caller's per-call +# `properties` cannot set them. +_RESERVED_EXCEPTION_PROPERTIES = frozenset( + { + "$exception_list", + "$exception_level", + "$exception_source", + "$debug_images", + "$exception_handled", + "$exception_types", + "$exception_values", + "$exception_sources", + "$exception_functions", + "$exception_fingerprint_version", + "$exception_fingerprint_record", + "$exception_issue_id", + "$exception_release", + "$cymbal_errors", + } +) + + +def _exception_level(integration_level, level): + # type: (Any, Any) -> str + """An integration's level wins over the caller's ``level=``; an invalid value counts as unset.""" + return ( + _normalize_exception_level(integration_level) + or _normalize_exception_level(level) + or "error" + ) + + +def _without_reserved_exception_properties(properties): + # type: (Optional[Dict[str, Any]]) -> Dict[str, Any] + return { + key: value + for key, value in (properties or {}).items() + if key not in _RESERVED_EXCEPTION_PROPERTIES + } + + def _capture_exception_with_metadata(client, exception, capture_metadata, **kwargs): # type: (Any, ExceptionArg, _ExceptionCaptureMetadata, **Any) -> Optional[str] """Call capture_exception through the SDK-internal typed integration channel.""" diff --git a/posthog/request.py b/posthog/request.py index 26b965af..f23e40e3 100644 --- a/posthog/request.py +++ b/posthog/request.py @@ -213,6 +213,7 @@ def post( api_key: str, host: Optional[str] = None, path: Optional[str] = None, + *, timeout: int = 15, session: Optional[requests.Session] = None, **kwargs, @@ -298,6 +299,7 @@ def _feature_flags_retry_delay(failed_attempt: int) -> float: def flags( api_key: str, host: Optional[str] = None, + *, timeout: int = 15, max_retries: int = 1, **kwargs, @@ -312,7 +314,7 @@ def flags( api_key, host, "/flags/?v=2", - timeout, + timeout=timeout, session=_get_flags_session(), **kwargs, ) diff --git a/posthog/test/test_async_client.py b/posthog/test/test_async_client.py index f111000b..5404471b 100644 --- a/posthog/test/test_async_client.py +++ b/posthog/test/test_async_client.py @@ -72,6 +72,26 @@ async def send_batch(api_key, host, batch, **kwargs): assert event["uuid"] == event_uuid +@pytest.mark.asyncio +@pytest.mark.parametrize( + "supplied, warnings", + [("not-a-uuid-secret-1", 1), (123, 1), ("", 0), (None, 0)], +) +async def test_capture_replaces_unusable_uuid_and_warns_only_when_invalid( + caplog, supplied, warnings +): + client = AsyncPosthog("test-key", send=False) + with caplog.at_level(logging.WARNING, logger="posthog"): + event_uuid = client.capture("async event", distinct_id="user-1", uuid=supplied) + await client.shutdown() + + assert UUID(event_uuid).version == 7 + logged = [r for r in caplog.records if r.levelno >= logging.WARNING] + assert len(logged) == warnings + assert all(r.levelname == "WARNING" for r in logged) + assert all(str(supplied) not in r.getMessage() for r in logged) + + def test_capture_from_worker_thread_wakes_loop_bound_queue(): script = r""" import asyncio @@ -723,6 +743,32 @@ async def test_capture_exception_never_raises_in_debug_mode(): await client.shutdown() +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("level", "expected"), [(None, "error"), ("warning", "warning")] +) +async def test_capture_exception_sets_level_and_ignores_reserved_properties( + level, expected +): + client = AsyncPosthog("test-key", send=False) + with mock.patch.object(client, "capture", return_value="uuid") as capture: + client.capture_exception( + ValueError("boom"), + level=level, + properties={ + "$exception_list": [], + "$exception_level": "fatal", + "plan": "pro", + }, + ) + + properties = capture.call_args.kwargs["properties"] + assert properties["$exception_list"][0]["type"] == "ValueError" + assert properties["$exception_level"] == expected + assert properties["plan"] == "pro" + await client.shutdown() + + @pytest.mark.asyncio async def test_capture_exception_uses_context_code_variable_settings(): client = AsyncPosthog( diff --git a/posthog/test/test_capture_event.py b/posthog/test/test_capture_event.py index 21380fb7..e4a7c763 100644 --- a/posthog/test/test_capture_event.py +++ b/posthog/test/test_capture_event.py @@ -153,6 +153,7 @@ def test_non_dict_options_are_logged_and_ignored(self, _name, options) -> None: [ ("session_id", "$session_id", "session_id", "s-123"), ("window_id", "$window_id", "window_id", "w-456"), + ("empty_string", "$session_id", "session_id", ""), ] ) def test_top_level_string_sentinels(self, _name, prop_key, field_name, raw) -> None: @@ -160,8 +161,29 @@ def test_top_level_string_sentinels(self, _name, prop_key, field_name, raw) -> N self.assertEqual(event[field_name], raw) self.assertNotIn(prop_key, event["properties"]) - def test_top_level_sentinel_omitted_but_removed_when_not_string(self) -> None: - event = _to_v1_event(_legacy_msg(properties={"$session_id": 42})) + @parameterized.expand( + [ + ("number", "$session_id", 42, "number"), + ("bool", "$window_id", True, "bool"), + ("array", "$session_id", ["s-1"], "array"), + ("object", "$window_id", {"id": "w-1"}, "object"), + ] + ) + def test_non_string_sentinel_is_dropped_with_a_warning( + self, _name, prop_key, raw, type_name + ) -> None: + with self.assertLogs("posthog", level="WARNING") as logs: + event = _to_v1_event(_legacy_msg(properties={prop_key: raw})) + self.assertNotIn(prop_key.lstrip("$"), event) + self.assertNotIn(prop_key, event["properties"]) + self.assertIn( + f"dropping {prop_key}: a {type_name} value is not a string", + logs.records[0].getMessage(), + ) + + def test_null_sentinel_is_dropped_silently(self) -> None: + with self.assertNoLogs("posthog", level="WARNING"): + event = _to_v1_event(_legacy_msg(properties={"$session_id": None})) self.assertNotIn("session_id", event) self.assertNotIn("$session_id", event["properties"]) diff --git a/posthog/test/test_client.py b/posthog/test/test_client.py index 05bbfc65..6d510cd9 100644 --- a/posthog/test/test_client.py +++ b/posthog/test/test_client.py @@ -476,18 +476,17 @@ def test_capture_sends_and_returns_canonical_uuid(self, _name, supplied): @parameterized.expand( [ - ("empty string", ""), - ("invalid string", "not-a-uuid"), + ("invalid string", "not-a-uuid-secret-1"), ("short string", "1234"), ("integer", 123), ] ) - def test_capture_with_invalid_uuid_logs_and_falls_back_to_generated_uuid( + def test_capture_with_invalid_uuid_warns_and_falls_back_to_generated_uuid( self, _name, invalid_uuid ): with patch_capture_send("client") as mock_post: client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail, sync_mode=True) - with self.assertLogs("posthog", level="ERROR") as logs: + with self.assertLogs("posthog", level="WARNING") as logs: msg_uuid = client.capture( "python test event", distinct_id="distinct_id", uuid=invalid_uuid ) @@ -498,18 +497,24 @@ def test_capture_with_invalid_uuid_logs_and_falls_back_to_generated_uuid( msg = sent_batch(mock_post)[0] self.assertEqual(msg["uuid"], msg_uuid) self.assertNotEqual(msg["uuid"], str(invalid_uuid)) - self.assertTrue( - any( - f"Invalid event uuid {invalid_uuid!r}" in message - and "Expected a valid UUID string or uuid.UUID instance" in message - and "Falling back to a generated UUID" in message - for message in logs.output + uuid_warnings = [r for r in logs.records if "Event uuid" in r.getMessage()] + self.assertEqual(len(uuid_warnings), 1) + self.assertEqual(uuid_warnings[0].levelname, "WARNING") + self.assertNotIn(str(invalid_uuid), uuid_warnings[0].getMessage()) + + def test_capture_with_empty_uuid_generates_one_without_logging(self): + with patch_capture_send("client") as mock_post: + client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail, sync_mode=True) + with self.assertNoLogs("posthog", level="WARNING"): + msg_uuid = client.capture( + "python test event", distinct_id="distinct_id", uuid="" ) - ) + + self.assertEqual(UUID(msg_uuid).version, 7) + self.assertEqual(sent_batch(mock_post)[0]["uuid"], msg_uuid) @parameterized.expand( [ - ("empty string", ""), ("invalid string", "not-a-uuid"), ("short string", "1234"), ("integer", 123), @@ -518,7 +523,7 @@ def test_capture_with_invalid_uuid_logs_and_falls_back_to_generated_uuid( def test_capture_with_invalid_uuid_falls_back_in_debug(self, _name, invalid_uuid): with patch_capture_send("client") as mock_post: client = Client(FAKE_TEST_API_KEY, debug=True, sync_mode=True) - with self.assertLogs("posthog", level="ERROR"): + with self.assertLogs("posthog", level="WARNING"): msg_uuid = client.capture( "python test event", distinct_id="distinct_id", uuid=invalid_uuid ) @@ -580,44 +585,47 @@ def test_basic_capture_exception(self): self.assertEqual(capture_call[0][0], "$exception") self.assertEqual(capture_call[1]["distinct_id"], "distinct_id") - def test_reserved_exception_property_overrides_are_deprecated(self): - custom_exception_list = [{"type": "CustomError", "value": "custom"}] + def test_capture_exception_ignores_reserved_properties(self): properties = { - "$exception_list": custom_exception_list, + "$exception_list": [{"type": "CustomError", "value": "custom"}], "$exception_level": "warning", "$exception_source": "custom.source", - "$exception_issue_id": "legacy-issue-id", + "$exception_issue_id": "custom-issue-id", + "plan": "pro", } - with ( - mock.patch.object(Client, "capture", return_value=None) as patch_capture, - self.assertWarnsRegex( - DeprecationWarning, - "Reserved exception properties.*next major version", - ), - ): + with mock.patch.object(Client, "capture", return_value=None) as patch_capture: self.client.capture_exception( Exception("test exception"), properties=properties ) captured_properties = patch_capture.call_args.kwargs["properties"] - self.assertIs(captured_properties["$exception_list"], custom_exception_list) - self.assertEqual(captured_properties["$exception_level"], "warning") - self.assertEqual(captured_properties["$exception_source"], "custom.source") - self.assertEqual(captured_properties["$exception_issue_id"], "legacy-issue-id") + self.assertEqual(captured_properties["$exception_list"][0]["type"], "Exception") + self.assertEqual(captured_properties["$exception_level"], "error") + self.assertNotIn("$exception_source", captured_properties) + self.assertNotIn("$exception_issue_id", captured_properties) + self.assertEqual(captured_properties["plan"], "pro") - def test_reserved_exception_property_warning_cannot_drop_the_event(self): - with ( - mock.patch.object(Client, "capture", return_value=None) as patch_capture, - warnings.catch_warnings(), - ): - warnings.simplefilter("error", DeprecationWarning) + @parameterized.expand( + [ + ("default", None, None, "error"), + ("caller_level", None, "warning", "warning"), + ("alias", None, "WARN", "warning"), + ("unknown_level", None, "loud", "error"), + ("integration_level_wins", "fatal", "warning", "fatal"), + ] + ) + def test_capture_exception_level(self, _name, integration_level, level, expected): + capture_metadata = {"level": integration_level} if integration_level else {} + with mock.patch.object(Client, "capture", return_value=None) as patch_capture: self.client.capture_exception( Exception("test exception"), - properties={"$exception_level": "warning"}, + level=level, + _capture_metadata=capture_metadata, ) - patch_capture.assert_called_once() + captured_properties = patch_capture.call_args.kwargs["properties"] + self.assertEqual(captured_properties["$exception_level"], expected) @parameterized.expand( [ @@ -1356,7 +1364,7 @@ def test_basic_capture_with_feature_flags_and_disable_geoip_returns_correctly( self.assertEqual(msg["event"], "python test event") self.assertTrue(isinstance(msg["timestamp"], str)) self.assertIsNotNone(msg.get("uuid")) - self.assertTrue("$geoip_disable" not in msg["properties"]) + self.assertIs(msg["properties"]["$geoip_disable"], False) self.assertEqual(msg["distinct_id"], "distinct_id") self.assertEqual(msg["properties"]["$lib"], "posthog-python") self.assertEqual(msg["properties"]["$lib_version"], VERSION) @@ -3792,7 +3800,7 @@ def test_disable_geoip_override_on_events(self): # Check page event page_batch = sent_batch(mock_post, 1) identify_msg = page_batch[0] - self.assertEqual("$geoip_disable" not in identify_msg["properties"], True) + self.assertIs(identify_msg["properties"]["$geoip_disable"], False) def test_disable_geoip_method_overrides_init_on_events(self): with patch_capture_send("client") as mock_post: @@ -3811,7 +3819,7 @@ def test_disable_geoip_method_overrides_init_on_events(self): mock_post.assert_called_once() batch_data = sent_batch(mock_post) msg = batch_data[0] - self.assertTrue("$geoip_disable" not in msg["properties"]) + self.assertIs(msg["properties"]["$geoip_disable"], False) @mock.patch("posthog.client.flags") def test_disable_geoip_default_on_decide(self, patch_flags): diff --git a/posthog/test/test_event_options.py b/posthog/test/test_event_options.py index f05aca85..50e974d6 100644 --- a/posthog/test/test_event_options.py +++ b/posthog/test/test_event_options.py @@ -410,6 +410,59 @@ async def test_async_caller_values_beat_sdk_values(source): _assert_caller_values(events, CALLER_SDK_VALUES) +GEOIP_CALL_CASES = [ + pytest.param(True, "context", False, True, id="call_beats_context"), + pytest.param(True, "super", False, True, id="call_beats_super"), + pytest.param(False, "super", True, False, id="call_false_beats_super"), + pytest.param(False, None, None, False, id="call_false_beats_client_setting"), + pytest.param(True, "event", False, False, id="event_property_beats_call"), +] + + +def _capture_with_call_geoip(call_value, source, layer_value): + def call(client): + with new_context(fresh=True): + if source == "context": + tag("$geoip_disable", layer_value) + properties = {"$geoip_disable": layer_value} if source == "event" else None + return client.capture( + "e", distinct_id="u", properties=properties, disable_geoip=call_value + ) + + return call + + +def _geoip_layer_config(source, layer_value): + if source == "super": + return {"super_properties": {"$geoip_disable": layer_value}} + return {} + + +@pytest.mark.parametrize("call_value, source, layer_value, expected", GEOIP_CALL_CASES) +def test_sync_call_disable_geoip_is_an_event_value( + call_value, source, layer_value, expected +): + events = _sync_wire_events( + _capture_with_call_geoip(call_value, source, layer_value), + disable_geoip=True, + **_geoip_layer_config(source, layer_value), + ) + assert events[0]["properties"]["$geoip_disable"] is expected + + +@pytest.mark.asyncio +@pytest.mark.parametrize("call_value, source, layer_value, expected", GEOIP_CALL_CASES) +async def test_async_call_disable_geoip_is_an_event_value( + call_value, source, layer_value, expected +): + events = await _async_wire_events( + _capture_with_call_geoip(call_value, source, layer_value), + disable_geoip=True, + **_geoip_layer_config(source, layer_value), + ) + assert events[0]["properties"]["$geoip_disable"] is expected + + @pytest.mark.parametrize("method", list(CAPTURE_CALLS)) def test_sync_super_properties_beat_sdk_values_on_every_path(method): events = _sync_wire_events( diff --git a/posthog/test/test_module.py b/posthog/test/test_module.py index 09d6ea82..bf433170 100644 --- a/posthog/test/test_module.py +++ b/posthog/test/test_module.py @@ -58,6 +58,15 @@ def test_module_flush_forwards_timeout(self): proxy.assert_called_once_with("flush", timeout_seconds=1.5) + def test_module_capture_exception_forwards_level(self): + error = ValueError("boom") + with mock.patch.object(posthog, "_proxy") as proxy: + posthog.capture_exception(error, level="warning") + + proxy.assert_called_once_with( + "capture_exception", exception=error, level="warning" + ) + class TestModuleLevelSetup(unittest.TestCase): def setUp(self): diff --git a/references/public_api_snapshot.txt b/references/public_api_snapshot.txt index 2edbd944..f8bbd37a 100644 --- a/references/public_api_snapshot.txt +++ b/references/public_api_snapshot.txt @@ -1332,7 +1332,7 @@ function posthog.ai.utils.with_privacy_mode(ph_client: PostHogClient, privacy_mo function posthog.alias(previous_id: ID_TYPES, distinct_id: str, timestamp: Optional[Union[datetime.datetime, str]] = None, uuid: Optional[str] = None, disable_geoip: Optional[bool] = None, options: Optional[Dict[str, Any]] = None) -> Optional[str] function posthog.capture(event: str, **kwargs: Unpack[OptionalCaptureArgs]) -> Optional[str] function posthog.capture_ai(event: str, **kwargs: Unpack[OptionalCaptureArgs]) -> Optional[str] -function posthog.capture_exception(exception: Optional[ExceptionArg] = None, **kwargs: Unpack[OptionalCaptureArgs]) -> Optional[str] +function posthog.capture_exception(exception: Optional[ExceptionArg] = None, *, level: Optional[str] = None, **kwargs: Unpack[OptionalCaptureArgs]) -> Optional[str] function posthog.client.add_context_tags(properties) function posthog.client.get_identity_state(passed) -> tuple[str, bool] function posthog.client.no_throw(default_return=None) @@ -1443,10 +1443,10 @@ function posthog.new_context(fresh: bool = False, capture_exceptions: Optional[b function posthog.request.determine_server_host(host: Optional[str]) -> str function posthog.request.disable_connection_reuse() -> None function posthog.request.enable_keep_alive() -> None -function posthog.request.flags(api_key: str, host: Optional[str] = None, timeout: int = 15, max_retries: int = 1, **kwargs) -> Any +function posthog.request.flags(api_key: str, host: Optional[str] = None, *, timeout: int = 15, max_retries: int = 1, **kwargs) -> Any function posthog.request.get(api_key: str, url: str, host: Optional[str] = None, timeout: Optional[int] = None, etag: Optional[str] = None) -> GetResponse function posthog.request.normalize_host(host: Optional[str]) -> str -function posthog.request.post(api_key: str, host: Optional[str] = None, path: Optional[str] = None, timeout: int = 15, session: Optional[requests.Session] = None, **kwargs) -> requests.Response +function posthog.request.post(api_key: str, host: Optional[str] = None, path: Optional[str] = None, *, timeout: int = 15, session: Optional[requests.Session] = None, **kwargs) -> requests.Response function posthog.request.remote_config(personal_api_key: str, project_api_key: str, host: Optional[str] = None, key: str = '', timeout: int = 15) -> Any function posthog.request.reset_sessions() -> None function posthog.request.set_socket_options(socket_options: Optional[SocketOptions]) -> None @@ -1591,7 +1591,7 @@ method posthog.async_client.AsyncClient.alias(previous_id: ID_TYPES, distinct_id method posthog.async_client.AsyncClient.capture(event: str, **kwargs: Unpack[OptionalCaptureArgs]) -> Optional[str] method posthog.async_client.AsyncClient.capture_ai(event: str, **kwargs: Unpack[OptionalCaptureArgs]) -> Optional[str] method posthog.async_client.AsyncClient.capture_ai_immediate(event: str, **kwargs: Unpack[OptionalCaptureArgs]) -> Optional[str] -method posthog.async_client.AsyncClient.capture_exception(exception: Optional[ExceptionArg] = None, **kwargs: Unpack[OptionalCaptureArgs]) -> Optional[str] +method posthog.async_client.AsyncClient.capture_exception(exception: Optional[ExceptionArg] = None, *, level: Optional[str] = None, **kwargs: Unpack[OptionalCaptureArgs]) -> Optional[str] method posthog.async_client.AsyncClient.capture_immediate(event: str, **kwargs: Unpack[OptionalCaptureArgs]) -> Optional[str] method posthog.async_client.AsyncClient.evaluate_flags(distinct_id: Optional[ID_TYPES] = None, *, groups: Optional[Mapping[str, Union[str, int]]] = None, person_properties: Optional[Dict[str, Any]] = None, group_properties: Optional[Dict[str, Dict[str, Any]]] = None, disable_geoip: Optional[bool] = None, flag_keys: Optional[list[str]] = None, device_id: Optional[str] = None) -> FeatureFlagEvaluations method posthog.async_client.AsyncClient.flush(timeout_seconds: Optional[float] = 10) -> None @@ -1607,7 +1607,7 @@ method posthog.capture_send.CaptureError.verdict_summary() -> str method posthog.client.Client.alias(previous_id: ID_TYPES, distinct_id: Optional[str], timestamp: Optional[Union[datetime, str]] = None, uuid: Optional[str] = None, disable_geoip: Optional[bool] = None, options: Optional[Dict[str, Any]] = None) -> Optional[str] method posthog.client.Client.capture(event: str, **kwargs: Unpack[OptionalCaptureArgs]) -> Optional[str] method posthog.client.Client.capture_ai(event: str, **kwargs: Unpack[OptionalCaptureArgs]) -> Optional[str] -method posthog.client.Client.capture_exception(exception: Optional[ExceptionArg], **kwargs: Unpack[OptionalCaptureArgs]) -> Optional[str] +method posthog.client.Client.capture_exception(exception: Optional[ExceptionArg], *, level: Optional[str] = None, **kwargs: Unpack[OptionalCaptureArgs]) -> Optional[str] method posthog.client.Client.evaluate_flags(distinct_id: Optional[ID_TYPES] = None, *, groups: Optional[Mapping[str, Union[str, int]]] = None, person_properties: Optional[Dict[str, Any]] = None, group_properties: Optional[Dict[str, Dict[str, Any]]] = None, only_evaluate_locally: bool = False, disable_geoip: Optional[bool] = None, flag_keys: Optional[List[str]] = None, device_id: Optional[str] = None) -> FeatureFlagEvaluations method posthog.client.Client.feature_enabled(key: str, distinct_id: ID_TYPES, *, groups: Optional[Mapping[str, Union[str, int]]] = None, person_properties: Optional[Dict[str, Any]] = None, group_properties: Optional[Dict[str, Dict[str, Any]]] = None, only_evaluate_locally: bool = False, send_feature_flag_events: bool = True, disable_geoip: Optional[bool] = None, device_id: Optional[str] = None) -> Optional[bool] method posthog.client.Client.feature_flag_definitions()