feat(code-mappings): Add support for stacktrace frames with backslashes for automatic code mappings (#68845)

This PR updates the existing code mapping logic to support stack traces
whose filepaths contained backslashes instead of forward slashes. This
effectively adds automatic code mapping support for issues derived from
Windows machines (for the existing supported languages).

I've made the choice to save stack roots in the code mappings using
whichever slash delimiter that the original stack path used. For
example, if we have the following stack path and source:

* Stack Path = `"D:\Users\code\sentry\models\release.py"`
* Source path = `"src/sentry/models/release.py"`

Then the roots generated will be:

* Stack Root = `"D:\Users\code\"` (note that this source root still uses
backslashes instead of forward slashes)
* Source Root = `"src/"`

This should be less confusing to the end user since we explicitly say we
are going to replace the `stack_root` in the `stack_path` with the
`source_root` to get the `source_path`. It might be confusing if the
`stack_root`'s slash-delimiter was not the same as that in the
`stack_path`.

This also means that the previous logic to apply code mappings to a
given frame to generate a stacktrace link needed to be updated so that
the source path generated would only include slashes (since this path is
used in a github URL, which only uses forward slashes).

Author: MichaelSun48

src/sentry/integrations/utils/code_mapping.py

@@ -61,15 +61,19 @@ class UnsupportedFrameFilename(Exception):
 class FrameFilename:
     def __init__(self, frame_file_path: str) -> None:
         self.raw_path = frame_file_path
-        if frame_file_path[0] == "/":
+        is_windows_path = False
+        if "\\" in frame_file_path:
+            is_windows_path = True
+            frame_file_path = frame_file_path.replace("\\", "/")
+
+        if frame_file_path[0] == "/" or frame_file_path[0] == "\\":
             frame_file_path = frame_file_path[1:]
 
         # Using regexes would be better but this is easier to understand
         if (
             not frame_file_path
             or frame_file_path[0] in ["[", "<"]
             or frame_file_path.find(" ") > -1
-            or frame_file_path.find("\\") > -1  # Windows support
             or frame_file_path.find("/") == -1
         ):
             raise UnsupportedFrameFilename("This path is not supported.")
@@ -79,8 +83,20 @@ def __init__(self, frame_file_path: str) -> None:
         if not self.extension:
             raise UnsupportedFrameFilename("It needs an extension.")
 
+        # Remove drive letter if it exists
+        if is_windows_path and frame_file_path[1] == ":":
+            frame_file_path = frame_file_path[2:]
+            # windows drive letters can be like C:\ or C:
+            # so we need to remove the slash if it exists
+            if frame_file_path[0] == "/":
+                frame_file_path = frame_file_path[1:]
+
         start_at_index = get_straight_path_prefix_end_index(frame_file_path)
         self.straight_path_prefix = frame_file_path[:start_at_index]
+
+        # We normalize the path to be as close to what the path would
+        # look like in the source code repository, hence why we remove
+        # the straight path prefix and drive letter
         self.normalized_path = frame_file_path[start_at_index:]
         if start_at_index == 0:
             self.root = frame_file_path.split("/")[0]
@@ -145,7 +161,7 @@ def list_file_matches(self, frame_filename: FrameFilename) -> list[dict[str, str
                     )
                     continue
 
-                if stack_path.replace(stack_root, source_root, 1) != source_path:
+                if stack_path.replace(stack_root, source_root, 1).replace("\\", "/") != source_path:
                     logger.info(
                         "Unexpected stack_path/source_path found. A code mapping was not generated.",
                         extra={
@@ -259,7 +275,7 @@ def _generate_code_mapping_from_tree(
             )
             return []
 
-        if stack_path.replace(stack_root, source_root, 1) != source_path:
+        if stack_path.replace(stack_root, source_root, 1).replace("\\", "/") != source_path:
             logger.info(
                 "Unexpected stack_path/source_path found. A code mapping was not generated.",
                 extra={
@@ -370,6 +386,10 @@ def convert_stacktrace_frame_path_to_source_path(
     If the code mapping does not apply to the frame, returns None.
     """
 
+    stack_root = code_mapping.stack_root
+    if "\\" in code_mapping.stack_root:
+        stack_root = code_mapping.stack_root.replace("\\", "/")
+
     # In most cases, code mappings get applied to frame.filename, but some platforms such as Java
     # contain folder info in other parts of the frame (e.g. frame.module="com.example.app.MainActivity"
     # gets transformed to "com/example/app/MainActivity.java"), so in those cases we use the
@@ -379,13 +399,13 @@ def convert_stacktrace_frame_path_to_source_path(
     )
 
     if stacktrace_path and stacktrace_path.startswith(code_mapping.stack_root):
-        return stacktrace_path.replace(code_mapping.stack_root, code_mapping.source_root, 1)
+        return stacktrace_path.replace(stack_root, code_mapping.source_root, 1)
 
     # Some platforms only provide the file's name without folder paths, so we
     # need to use the absolute path instead. If the code mapping has a non-empty
     # stack_root value and it matches the absolute path, we do the mapping on it.
     if frame.abs_path and frame.abs_path.startswith(code_mapping.stack_root):
-        return frame.abs_path.replace(code_mapping.stack_root, code_mapping.source_root, 1)
+        return frame.abs_path.replace(stack_root, code_mapping.source_root, 1)
 
     return None
 
@@ -505,8 +525,8 @@ def find_roots(stack_path: str, source_path: str) -> tuple[str, str]:
     If there is no overlap, raise an exception since this should not happen
     """
     stack_root = ""
-    if stack_path[0] == "/":
-        stack_root += "/"
+    if stack_path[0] == "/" or stack_path[0] == "\\":
+        stack_root += stack_path[0]
         stack_path = stack_path[1:]
 
     if stack_path == source_path:
@@ -534,7 +554,7 @@ def find_roots(stack_path: str, source_path: str) -> tuple[str, str]:
             source_root = source_path.rpartition(overlap)[0]
             stack_root += stack_path_delim.join(stack_root_items)
 
-            if stack_root and stack_root[-1] != SLASH:  # append trailing slash
+            if stack_root and stack_root[-1] != stack_path_delim:  # append trailing slash
                 stack_root = f"{stack_root}{stack_path_delim}"
             if source_root and source_root[-1] != SLASH:
                 source_root = f"{source_root}{SLASH}"

tests/sentry/integrations/utils/test_code_mapping.py

@@ -48,7 +48,6 @@
     "README",  # no extension
     "ssl.py",
     # XXX: The following will need to be supported
-    "C:\\Users\\Donia\\AppData\\Roaming\\Adobe\\UXP\\Plugins\\External\\452f92d2_0.13.0\\main.js",
     "initialization.dart",
     "backburner.js",
 ]

tests/sentry/tasks/test_derive_code_mappings.py

@@ -97,6 +97,83 @@ def test_raises_generic_errors(self, mock_logger):
                 )
 
 
+class TestBackSlashDeriveCodeMappings(BaseDeriveCodeMappings):
+    def setUp(self):
+        super().setUp()
+        self.platform = "python"
+        # The lack of a \ after the drive letter in the third frame signals that
+        # this is a relative path. This may be unlikely to occur in practice,
+        # but worth testing nonetheless.
+        self.event_data = self.generate_data(
+            [
+                {"in_app": True, "filename": "\\sentry\\mouse.py"},
+                {"in_app": True, "filename": "\\sentry\\dog\\cat\\parrot.py"},
+                {"in_app": True, "filename": "C:sentry\\tasks.py"},
+                {"in_app": True, "filename": "D:\\Users\\code\\sentry\\models\\release.py"},
+            ]
+        )
+
+    @responses.activate
+    def test_backslash_filename_simple(self):
+        repo_name = "foo/bar"
+        with patch(
+            "sentry.integrations.github.client.GitHubClientMixin.get_trees_for_org"
+        ) as mock_get_trees_for_org:
+            mock_get_trees_for_org.return_value = {
+                repo_name: RepoTree(Repo(repo_name, "master"), ["sentry/mouse.py"])
+            }
+            derive_code_mappings(self.project.id, self.event_data)
+            code_mapping = RepositoryProjectPathConfig.objects.all()[0]
+            assert code_mapping.stack_root == "\\"
+            assert code_mapping.source_root == ""
+            assert code_mapping.repository.name == repo_name
+
+    @responses.activate
+    def test_backslash_drive_letter_filename_simple(self):
+        repo_name = "foo/bar"
+        with patch(
+            "sentry.integrations.github.client.GitHubClientMixin.get_trees_for_org"
+        ) as mock_get_trees_for_org:
+            mock_get_trees_for_org.return_value = {
+                repo_name: RepoTree(Repo(repo_name, "master"), ["sentry/tasks.py"])
+            }
+            derive_code_mappings(self.project.id, self.event_data)
+            code_mapping = RepositoryProjectPathConfig.objects.all()[0]
+            assert code_mapping.stack_root == "C:sentry\\"
+            assert code_mapping.source_root == "sentry/"
+            assert code_mapping.repository.name == repo_name
+
+    @responses.activate
+    def test_backslash_drive_letter_filename_monorepo(self):
+        repo_name = "foo/bar"
+        with patch(
+            "sentry.integrations.github.client.GitHubClientMixin.get_trees_for_org"
+        ) as mock_get_trees_for_org:
+            mock_get_trees_for_org.return_value = {
+                repo_name: RepoTree(Repo(repo_name, "master"), ["src/sentry/tasks.py"])
+            }
+            derive_code_mappings(self.project.id, self.event_data)
+            code_mapping = RepositoryProjectPathConfig.objects.all()[0]
+            assert code_mapping.stack_root == "C:sentry\\"
+            assert code_mapping.source_root == "src/sentry/"
+            assert code_mapping.repository.name == repo_name
+
+    @responses.activate
+    def test_backslash_drive_letter_filename_abs_path(self):
+        repo_name = "foo/bar"
+        with patch(
+            "sentry.integrations.github.client.GitHubClientMixin.get_trees_for_org"
+        ) as mock_get_trees_for_org:
+            mock_get_trees_for_org.return_value = {
+                repo_name: RepoTree(Repo(repo_name, "master"), ["sentry/models/release.py"])
+            }
+            derive_code_mappings(self.project.id, self.event_data)
+            code_mapping = RepositoryProjectPathConfig.objects.all()[0]
+            assert code_mapping.stack_root == "D:\\Users\\code\\"
+            assert code_mapping.source_root == ""
+            assert code_mapping.repository.name == repo_name
+
+
 class TestJavascriptDeriveCodeMappings(BaseDeriveCodeMappings):
     def setUp(self):
         super().setUp()