Skip to content

Commit

Permalink
feat(code_mappings): Support top level packages
Browse files Browse the repository at this point in the history
A file at the top of a repository is a valid location, thus, add support for it.

This also now returns a 400 instead of a 500 when a file path is not supported.
  • Loading branch information
armenzg committed Sep 15, 2023
1 parent 0718ad0 commit 9c6606d
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 18 deletions.
18 changes: 12 additions & 6 deletions src/sentry/api/endpoints/organization_derive_code_mappings.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
CodeMappingTreesHelper,
FrameFilename,
Repo,
UnsupportedFrameFilename,
create_code_mapping,
)
from sentry.models import Project
Expand Down Expand Up @@ -54,12 +55,17 @@ def get(self, request: Request, organization: Organization) -> Response:

trees = installation.get_trees_for_org()
trees_helper = CodeMappingTreesHelper(trees)
frame_filename = FrameFilename(stacktrace_filename)
possible_code_mappings: List[Dict[str, str]] = trees_helper.list_file_matches(
frame_filename
)
resp_status = status.HTTP_200_OK if possible_code_mappings else status.HTTP_204_NO_CONTENT
return Response(serialize(possible_code_mappings), status=resp_status)
try:
frame_filename = FrameFilename(stacktrace_filename)
possible_code_mappings: List[Dict[str, str]] = trees_helper.list_file_matches(
frame_filename
)
resp_status = (
status.HTTP_200_OK if possible_code_mappings else status.HTTP_204_NO_CONTENT
)
return Response(serialize(possible_code_mappings), status=resp_status)
except UnsupportedFrameFilename:
return Response("We do not support this case.", status=status.HTTP_400_BAD_REQUEST)

def post(self, request: Request, organization: Organization) -> Response:
"""
Expand Down
19 changes: 12 additions & 7 deletions src/sentry/integrations/utils/code_mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ def __init__(self, frame_file_path: str) -> None:
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.")

Expand Down Expand Up @@ -150,12 +149,18 @@ def _straight_path_logic(self, frame_file_path: str) -> None:

start_at_index = get_straight_path_prefix_end_index(frame_file_path)
backslash_index = frame_file_path.find("/", start_at_index)
dir_path, self.file_name = frame_file_path.rsplit("/", 1) # foo.tsx (both)
self.root = frame_file_path[0:backslash_index] # some or .some
self.dir_path = dir_path.replace(self.root, "") # some/path/ (both)
self.file_and_dir_path = remove_straight_path_prefix(
frame_file_path
) # some/path/foo.tsx (both)
if backslash_index > -1:
dir_path, self.file_name = frame_file_path.rsplit("/", 1) # foo.tsx (both)
self.root = frame_file_path[0:backslash_index] # some or .some
self.dir_path = dir_path.replace(self.root, "") # some/path/ (both)
self.file_and_dir_path = remove_straight_path_prefix(
frame_file_path
) # some/path/foo.tsx (both)
else:
self.file_name = frame_file_path
self.file_and_dir_path = frame_file_path
self.root = ""
self.dir_path = ""

def __repr__(self) -> str:
return f"FrameFilename: {self.full_path}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,29 @@ def test_get_start_with_backslash(self, mock_get_trees_for_org):
assert response.status_code == 200, response.content
assert response.data == expected_matches

@patch("sentry.integrations.github.GitHubIntegration.get_trees_for_org")
def test_get_top_level_file(self, mock_get_trees_for_org):
file = "index.php"
frame_info = FrameFilename(file)
config_data = {"stacktraceFilename": file}
expected_matches = [
{
"filename": file,
"repo_name": "getsentry/codemap",
"repo_branch": "master",
"stacktrace_root": f"{frame_info.root}",
"source_path": _get_code_mapping_source_path(file, frame_info),
}
]
with patch(
"sentry.integrations.utils.code_mapping.CodeMappingTreesHelper.list_file_matches",
return_value=expected_matches,
):
response = self.client.get(self.url, data=config_data, format="json")
assert mock_get_trees_for_org.call_count == 1
assert response.status_code == 200, response.content
assert response.data == expected_matches

@patch("sentry.integrations.github.GitHubIntegration.get_trees_for_org")
def test_get_multiple_matches(self, mock_get_trees_for_org):
config_data = {
Expand Down
22 changes: 17 additions & 5 deletions tests/sentry/integrations/utils/test_code_mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,15 @@

UNSUPPORTED_FRAME_FILENAMES = [
"async https://s1.sentry-cdn.com/_static/dist/sentry/entrypoints/app.js",
"/gtm.js", # Top source; starts with backslash
"<anonymous>",
"<frozen importlib._bootstrap>",
"[native code]",
"O$t",
"async https://s1.sentry-cdn.com/_static/dist/sentry/entrypoints/app.js",
"/foo/bar/baz", # no extension
"foo/bar/baz", # no extension
"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",
]


Expand Down Expand Up @@ -79,13 +75,21 @@ def test_get_extension():

def test_buckets_logic():
stacktraces = [
"/gtm.js",
"initialization.dart",
"backburner.js",
"app://foo.js",
"./app/utils/handleXhrErrorResponse.tsx",
"getsentry/billing/tax/manager.py",
"/cronscripts/monitoringsync.php",
] + UNSUPPORTED_FRAME_FILENAMES
buckets = stacktrace_buckets(stacktraces)
assert buckets == {
"": [
FrameFilename("gtm.js"),
FrameFilename("initialization.dart"),
FrameFilename("backburner.js"),
],
"./app": [FrameFilename("./app/utils/handleXhrErrorResponse.tsx")],
"app:": [FrameFilename("app://foo.js")],
"cronscripts": [FrameFilename("/cronscripts/monitoringsync.php")],
Expand All @@ -109,6 +113,14 @@ def test_frame_filename_repr(self):
path = "getsentry/billing/tax/manager.py"
assert FrameFilename(path).__repr__() == f"FrameFilename: {path}"

def test_frame_filename_root_level(self):
file_name = "index.php"
ff = FrameFilename(file_name)
assert ff.file_name == file_name
assert ff.root == ""
assert ff.dir_path == ""
assert ff.extension == "php"

def test_raises_unsupported(self):
for filepath in UNSUPPORTED_FRAME_FILENAMES:
with pytest.raises(UnsupportedFrameFilename):
Expand Down

0 comments on commit 9c6606d

Please sign in to comment.