Do not export types behind TYPE_CHECKING

[evanpurkhiser]

Problem Statement

Currently Event and friends are exported behind the TYPE_CHECKING conditional. Meaning imports of these types must also be behind these conditionals.

cc @asottile-sentry

Solution Brainstorm

Export without the TYPE_CHECKING conditional

[asottile-sentry]

alternatively #2829 (comment) -- though it would be ideal to have the actual type definitions be real and available

[szokeasaurusrex]

Export without the TYPE_CHECKING conditional

Unfortunately, this would not be possible in the SDK, since we have to maintain compatibility with legacy Python versions at runtime. Many of our types, including the Event type, use Python features that are only available in newer Python versions, so we can only evaluate the type checking code in TYPE_CHECKING environments, where we only need to be compatible with newer Python versions.

alternatively #2829 (comment) -- though it would be ideal to have the actual type definitions be real and available

Likewise, we cannot implement the suggestion in #2829 (comment), because dict subscripting was only introduced in Python 3.9, so this line would cause a type error in lower Python versions.

I think the best solution is to set the types to None at runtime. This is definitely compatible with old Pythons, and the type hints are anyways only really useful in type checking environments; I don't see why anyone would use them at runtime.

[HansAarneLiblik]

@szokeasaurusrex If you can't use dict, then you what would be against

from typing import TYPE_CHECKING, Dict, Any

if TYPE_CHECKING:
    from sentry_sdk._types import Event, Hint  # noqa: F401
else:
    Event = Dict[str, Any]
    Hint = Dict[str, Any]

?

[szokeasaurusrex]

@HansAarneLiblik There is no use for Event or Hint at runtime, as far as I am aware, since their only purpose is to enable type-checking by mypy and other static analyzers. Setting both to None helps indicate that these variables are meaningless at runtime.

Setting these to a different type at runtime is somewhat confusing, because Event is not actually a Dict[str, Any]; it is a TypedDict which is different. I could imagine someone looking at the source code being confused about why we sometimes set Event to a different type.

Is there a specific use case that setting these types to Dict[str, Any] at runtime would enable for you?

[HansAarneLiblik]

Nope, just was a question on the thought process 🙂

[szokeasaurusrex]

@HansAarneLiblik alright, thanks for the clarification! I understand that this solution is perhaps less than ideal, but I think it is the best we can do in order to enable detailed type hints while also maintaining runtime compatibility with legacy Python versions which lack support for many typing features.

[HansAarneLiblik]

@szokeasaurusrex Actually I may have an issue with this.

We have typed before_send as Callable[[Event, dict[str, Any]], Event | None]. Note the | None as the return type, because we have an exception, that we KNOW we raise, but don't want to send to Sentry and Docs state, we can return null (I'm guessing it should be None though) to skip reporting the event

def before_send_sentry(event: Event, hint: dict[str, Any]) -> Event | None:
    exc_info = hint.get("exc_info")
    if exc_info and "FailTask" in str(exc_info[0]):
        print("FailTask exception, NOT SENDING TO SENTRY")
        return None
        
    return event

Now, runtime, Event is None, and launching pytest raises error

TypeError: unsupported operand type(s) for |: 'NoneType' and 'NoneType'

Perhaps there is another way to handle skip_exceptions via Sentry configuration? Did not find a way in the docs though

[szokeasaurusrex]

Hi @HansAarneLiblik, thank you for bringing this problem to our attention! This seems like a pretty serious bug, so we will need to assign Event to something different than None at runtime. I opened #2926 to document this; should be a quick fix though 🙂