diff --git a/src/sentry/integrations/utils/code_mapping.py b/src/sentry/integrations/utils/code_mapping.py index 4eabab1ab05d89..12922c1e407e3c 100644 --- a/src/sentry/integrations/utils/code_mapping.py +++ b/src/sentry/integrations/utils/code_mapping.py @@ -61,7 +61,12 @@ 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 @@ -69,7 +74,6 @@ def __init__(self, frame_file_path: str) -> None: 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}" diff --git a/tests/sentry/integrations/utils/test_code_mapping.py b/tests/sentry/integrations/utils/test_code_mapping.py index 2623053400191e..c9863feb76e317 100644 --- a/tests/sentry/integrations/utils/test_code_mapping.py +++ b/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", ] diff --git a/tests/sentry/tasks/test_derive_code_mappings.py b/tests/sentry/tasks/test_derive_code_mappings.py index 11f0067f6f2a55..887735544d41cc 100644 --- a/tests/sentry/tasks/test_derive_code_mappings.py +++ b/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()