ref(errors): Use enum for stacktrace order (#96719)

Users can choose to display their stacktraces on the issue details page in reverse order, using the `stacktrace_order` user option. With a few caveats (see below), under the hood we store the option values as the strings `"-1"` (default), `"1"` (chronological order, with newest frame last), and `"2"` (same as default, reverse chronological order, with newest frame first).* In the front end we use numbers instead of strings, but with corresponding values.

Because the mapping is not obvious, this adds to the backend a `StacktraceOrder` enum (and a `_SerializedStacktraceOrder` enum to handle the conversion to numbers), and uses them rather than the corresponding strings/numbers. A similar change for the front end will happen in a follow-up PR.

Note: The description above is not strictly correct, for two reasons (each of which now has a corresponding TODO):

  1) There are a small number of users which don't have any of these and instead have the empty string as their option value. This is treated the same as not having the option set at all, so it's not breaking anything, but we should nonetheless probably just clean those up.

  2) `"2"` is not always the same as the default, or rather, the default behavior is not always to have the newest frame first. There is one place, (indirectly) in the `Stacktrace` interface's `to_string` method[1], where for Python events (and events with no platform, though practically speaking Relay normalization means there's always a platform) the default is the opposite, newest last[2]. This doesn't match stacktrace ordering behavior anywhere else, so it might be worth investigating if we want to keep that special-casing in place. (It's also not immediately obvious where we (ultimately) use that method. Back when the special-casing was introduced[3] (in late 2012), it was controlling the result of `Stacktrace.to_html`, but now we obviously let react create the HTML, and in our current front end there's no such exception for Python.)


[1] https://github.com/getsentry/sentry/blob/5778c7a72e21e9d8c83248e0ad6b7d906c3aa02b/src/sentry/interfaces/stacktrace.py#L519-L526
[2] https://github.com/getsentry/sentry/blob/5778c7a72e21e9d8c83248e0ad6b7d906c3aa02b/src/sentry/interfaces/stacktrace.py#L89
[3] 329085b

Author: lobsterkatie

src/sentry/interfaces/stacktrace.py

@@ -1,4 +1,5 @@
 import math
+from enum import Enum
 
 from django.utils.translation import gettext as _
 
@@ -11,6 +12,12 @@
 __all__ = ("Stacktrace",)
 
 
+class StacktraceOrder(str, Enum):
+    DEFAULT = "-1"  # Equivalent to `MOST_RECENT_FIRST`
+    MOST_RECENT_LAST = "1"
+    MOST_RECENT_FIRST = "2"
+
+
 def max_addr(cur, addr):
     if addr is None:
         return cur
@@ -86,16 +93,21 @@ def get_context(lineno, context_line, pre_context=None, post_context=None):
 
 
 def is_newest_frame_first(event):
+    # TODO: Investigate if we should keep this special-casing for python, since we don't special
+    # case it anywhere else we check stacktrace order. If we do remove it, we might consider
+    # ditching this helper altogether, and just checking and using the option value directly in the
+    # one spot this helper is used.
+    # (See https://github.com/getsentry/sentry/pull/96719 for more context.)
     newest_first = event.platform not in ("python", None)
 
     if env.request and env.request.user.is_authenticated:
         options = user_option_service.get_many(
             filter=dict(user_ids=[env.request.user.id], keys=["stacktrace_order"])
         )
         display = get_option_from_list(options, default=None)
-        if display == "1":
+        if display == StacktraceOrder.MOST_RECENT_LAST:
             newest_first = False
-        elif display == "2":
+        elif display == StacktraceOrder.MOST_RECENT_FIRST:
             newest_first = True
 
     return newest_first

src/sentry/users/api/serializers/user.py

@@ -5,6 +5,7 @@
 from collections import defaultdict
 from collections.abc import Mapping, MutableMapping, Sequence
 from datetime import datetime
+from enum import Enum
 from typing import Any, DefaultDict, TypedDict, cast
 
 from django.conf import settings
@@ -15,6 +16,7 @@
 from sentry.app import env
 from sentry.auth.elevated_mode import has_elevated_mode
 from sentry.hybridcloud.services.organization_mapping import organization_mapping_service
+from sentry.interfaces.stacktrace import StacktraceOrder
 from sentry.models.authidentity import AuthIdentity
 from sentry.models.organization import OrganizationStatus
 from sentry.models.organizationmapping import OrganizationMapping
@@ -57,10 +59,16 @@ class _Identity(TypedDict):
     dateSynced: datetime
 
 
+class _SerializedStacktraceOrder(int, Enum):
+    DEFAULT = int(StacktraceOrder.DEFAULT)  # Equivalent to `MOST_RECENT_FIRST`
+    MOST_RECENT_LAST = int(StacktraceOrder.MOST_RECENT_LAST)
+    MOST_RECENT_FIRST = int(StacktraceOrder.MOST_RECENT_FIRST)
+
+
 class _UserOptions(TypedDict):
     theme: str  # TODO: enum/literal for theme options
     language: str
-    stacktraceOrder: int  # TODO: enum/literal
+    stacktraceOrder: _SerializedStacktraceOrder
     defaultIssueEvent: str
     timezone: str
     clock24Hours: bool
@@ -191,7 +199,14 @@ def serialize(
                 for o in UserOption.objects.filter(user_id=user.id, project_id__isnull=True)
                 if o.value is not None
             }
-            stacktrace_order = int(options.get("stacktrace_order", -1) or -1)
+
+            stacktrace_order = _SerializedStacktraceOrder(
+                int(
+                    options.get("stacktrace_order", StacktraceOrder.DEFAULT)
+                    # TODO: This second `or` won't be necessary once we remove empty strings from the DB
+                    or StacktraceOrder.DEFAULT
+                )
+            )
 
             d["options"] = {
                 "theme": options.get("theme") or "light",

src/sentry/users/models/user_option.py

@@ -185,7 +185,7 @@ class UserOption(Model):
      - self_notifications
         - "Notify Me About My Own Activity"
      - stacktrace_order
-        - default, most recent first, most recent last
+        - default, most recent first, most recent last (see `StacktraceOrder` enum)
      - subscribe_by_default
         - "Only On Issues I Subscribe To", "Only On Deploys With My Commits"
      - subscribe_notes

tests/sentry/users/api/endpoints/test_user_details.py

@@ -2,6 +2,7 @@
 from pytest import fixture
 
 from sentry.deletions.tasks.hybrid_cloud import schedule_hybrid_cloud_foreign_key_jobs
+from sentry.interfaces.stacktrace import StacktraceOrder
 from sentry.models.deletedorganization import DeletedOrganization
 from sentry.models.organization import Organization, OrganizationStatus
 from sentry.models.organizationmember import OrganizationMember
@@ -43,7 +44,7 @@ def test_lookup_self(self) -> None:
         assert resp.data["options"]["defaultIssueEvent"] == "recommended"
         assert resp.data["options"]["timezone"] == "UTC"
         assert resp.data["options"]["language"] == "en"
-        assert resp.data["options"]["stacktraceOrder"] == -1
+        assert resp.data["options"]["stacktraceOrder"] == int(StacktraceOrder.DEFAULT)
         assert not resp.data["options"]["clock24Hours"]
         assert not resp.data["options"]["prefersIssueDetailsStreamlinedUI"]
         assert not resp.data["options"]["prefersStackedNavigation"]
@@ -113,7 +114,7 @@ def test_simple(self) -> None:
                 "theme": "system",
                 "defaultIssueEvent": "latest",
                 "timezone": "UTC",
-                "stacktraceOrder": "2",
+                "stacktraceOrder": StacktraceOrder.MOST_RECENT_FIRST,
                 "language": "fr",
                 "clock24Hours": True,
                 "extra": True,
@@ -135,7 +136,10 @@ def test_simple(self) -> None:
         assert UserOption.objects.get_value(user=self.user, key="theme") == "system"
         assert UserOption.objects.get_value(user=self.user, key="default_issue_event") == "latest"
         assert UserOption.objects.get_value(user=self.user, key="timezone") == "UTC"
-        assert UserOption.objects.get_value(user=self.user, key="stacktrace_order") == "2"
+        assert (
+            UserOption.objects.get_value(user=self.user, key="stacktrace_order")
+            == StacktraceOrder.MOST_RECENT_FIRST
+        )
         assert UserOption.objects.get_value(user=self.user, key="language") == "fr"
         assert UserOption.objects.get_value(user=self.user, key="clock_24_hours")
         assert UserOption.objects.get_value(