Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions src/sentry/api/endpoints/project_stacktrace_link.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from typing import Optional, Tuple

from rest_framework.response import Response
from sentry_sdk import configure_scope

Expand All @@ -6,22 +8,32 @@
from sentry.api.serializers import serialize
from sentry.integrations import IntegrationFeatures
from sentry.models import Integration, RepositoryProjectPathConfig
from sentry.shared_integrations.exceptions import ApiError
from sentry.utils.compat import filter


def get_link(config, filepath, default, version=None):
def get_link(
config: RepositoryProjectPathConfig, filepath: str, default: str, version: Optional[str] = None
) -> Tuple[Optional[str], Optional[str], Optional[str]]:
oi = config.organization_integration
integration = oi.integration
install = integration.get_installation(oi.organization_id)

formatted_path = filepath.replace(config.stack_root, config.source_root, 1)

attempted_url = None
link = install.get_stacktrace_link(config.repository, formatted_path, default, version)
link, attempted_url, error = None, None, None
try:
link = install.get_stacktrace_link(config.repository, formatted_path, default, version)
except ApiError as e:
if e.code != 403:
raise
error = "integration_link_forbidden"

# If the link was not found, attach the URL that we attempted.
if not link:
error = error or "file_not_found"
attempted_url = install.format_source_url(config.repository, formatted_path, default)
return (link, attempted_url)
return link, attempted_url, error


class ProjectStacktraceLinkEndpoint(ProjectEndpoint):
Expand Down Expand Up @@ -78,7 +90,9 @@ def get(self, request, project):
result["error"] = "stack_root_mismatch"
continue

link, attempted_url = get_link(config, filepath, config.default_branch, commitId)
link, attempted_url, error = get_link(
config, filepath, config.default_branch, commitId
)

# it's possible for the link to be None, and in that
# case it means we could not find a match for the
Expand All @@ -87,7 +101,7 @@ def get(self, request, project):
if not link:
scope.set_tag("stacktrace_link.found", False)
scope.set_tag("stacktrace_link.error", "file_not_found")
result["error"] = "file_not_found"
result["error"] = error
result["attemptedUrl"] = attempted_url
else:
scope.set_tag("stacktrace_link.found", True)
Expand Down
8 changes: 6 additions & 2 deletions src/sentry/integrations/custom_scm/integration.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

from uuid import uuid4

from django import forms
Expand Down Expand Up @@ -52,15 +54,17 @@ class CustomSCMIntegration(IntegrationInstallation, RepositoryMixin):
def get_client(self):
pass

def get_stacktrace_link(self, repo, filepath, default, version):
def get_stacktrace_link(
self, repo: Repository, filepath: str, default: str, version: str
) -> str | None:
"""
We don't have access to verify that the file does exists
(using `check_file`) so instead we just return the
formatted source url using the default branch provided.
"""
return self.format_source_url(repo, filepath, default)

def format_source_url(self, repo, filepath, branch):
def format_source_url(self, repo: Repository, filepath: str, branch: str) -> str:
# This format works for GitHub/GitLab, not sure if it would
# need to change for a different provider
return f"{repo.url}/blob/{branch}/{filepath}"
Expand Down
16 changes: 10 additions & 6 deletions src/sentry/integrations/example/integration.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from typing import Any, Mapping, Optional
from __future__ import annotations

from typing import Any, Mapping

from django.http import HttpResponse

Expand All @@ -11,7 +13,7 @@
)
from sentry.integrations.issues import IssueSyncMixin, ResolveSyncAction
from sentry.mediators.plugins import Migrator
from sentry.models import ExternalIssue, User
from sentry.models import ExternalIssue, Repository, User
from sentry.pipeline import PipelineView
from sentry.shared_integrations.exceptions import IntegrationError

Expand Down Expand Up @@ -131,8 +133,8 @@ def get_unmigratable_repositories(self):

def sync_assignee_outbound(
self,
external_issue: "ExternalIssue",
user: Optional["User"],
external_issue: ExternalIssue,
user: User | None,
assign: bool = True,
**kwargs: Any,
) -> None:
Expand All @@ -151,10 +153,12 @@ def get_resolve_sync_action(self, data: Mapping[str, Any]) -> ResolveSyncAction:
def get_issue_display_name(self, external_issue):
return f"display name: {external_issue.key}"

def get_stacktrace_link(self, repo, path, default, version):
def get_stacktrace_link(
self, repo: Repository, filepath: str, default: str, version: str
) -> str | None:
pass

def format_source_url(self, repo, filepath, branch):
def format_source_url(self, repo: Repository, filepath: str, branch: str) -> str:
return f"https://example.com/{repo.name}/blob/{branch}/{filepath}"


Expand Down
5 changes: 4 additions & 1 deletion src/sentry/integrations/github/client.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
from __future__ import annotations

from datetime import datetime

import sentry_sdk

from sentry.integrations.client import ApiClient
from sentry.integrations.github.utils import get_jwt
from sentry.models import Repository
from sentry.utils import jwt


Expand Down Expand Up @@ -160,7 +163,7 @@ def create_token(self):
headers=headers,
)

def check_file(self, repo, path, version):
def check_file(self, repo: Repository, path: str, version: str) -> str | None:
repo_name = repo.name
return self.head_cached(path=f"/repos/{repo_name}/contents/{path}", params={"ref": version})

Expand Down
4 changes: 3 additions & 1 deletion src/sentry/integrations/github/integration.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

import re

from django.utils.text import slugify
Expand Down Expand Up @@ -118,7 +120,7 @@ def get_repositories(self, query=None):
def search_issues(self, query):
return self.get_client().search_issues(query)

def format_source_url(self, repo, filepath, branch):
def format_source_url(self, repo: Repository, filepath: str, branch: str) -> str:
# Must format the url ourselves since `check_file` is a head request
# "https://github.com/octokit/octokit.rb/blob/master/README.md"
return f"https://github.com/{repo.name}/blob/{branch}/{filepath}"
Expand Down
5 changes: 4 additions & 1 deletion src/sentry/integrations/gitlab/client.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
from __future__ import annotations

from urllib.parse import quote

from django.urls import reverse

from sentry.integrations.client import ApiClient
from sentry.models import Repository
from sentry.shared_integrations.exceptions import ApiError, ApiUnauthorized
from sentry.utils.http import absolute_uri

Expand Down Expand Up @@ -252,7 +255,7 @@ def get_diff(self, project_id, sha):
path = GitLabApiClientPath.diff.format(project=project_id, sha=sha)
return self.get(path)

def check_file(self, repo, path, ref):
def check_file(self, repo: Repository, path: str, ref: str) -> str | None:
"""Fetch a file for stacktrace linking

See https://docs.gitlab.com/ee/api/repository_files.html#get-file-from-repository
Expand Down
5 changes: 4 additions & 1 deletion src/sentry/integrations/gitlab/integration.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

from urllib.parse import urlparse

from django import forms
Expand All @@ -14,6 +16,7 @@
IntegrationProvider,
)
from sentry.integrations.repositories import RepositoryMixin
from sentry.models import Repository
from sentry.pipeline import NestedPipelineView, PipelineView
from sentry.shared_integrations.exceptions import ApiError, IntegrationError
from sentry.utils.hashlib import sha1_text
Expand Down Expand Up @@ -110,7 +113,7 @@ def get_repositories(self, query=None):
resp = self.get_client().search_group_projects(group, query)
return [{"identifier": repo["id"], "name": repo["name_with_namespace"]} for repo in resp]

def format_source_url(self, repo, filepath, branch):
def format_source_url(self, repo: Repository, filepath: str, branch: str) -> str:
base_url = self.model.metadata["base_url"]
repo_name = repo.config["path"]

Expand Down
39 changes: 21 additions & 18 deletions src/sentry/integrations/repositories.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
from __future__ import annotations

from typing import Mapping, Sequence

from sentry_sdk import configure_scope

from sentry.auth.exceptions import IdentityNotValid
Expand All @@ -11,19 +15,18 @@ class RepositoryMixin:
# dynamically given a search query
repo_search = False

def format_source_url(self, repo, filepath, branch):
"""
Formats the source code url used for stack trace linking.
"""
def format_source_url(self, repo: Repository, filepath: str, branch: str) -> str:
"""Formats the source code url used for stack trace linking."""
raise NotImplementedError

def check_file(self, repo, filepath, branch):
def check_file(self, repo: Repository, filepath: str, branch: str) -> str | None:
"""
Calls the client's `check_file` method to see if the file exists.
Returns the link to the file if it's exists, otherwise return `None`.

So far only GitHub and GitLab have this implemented, both of which give use back 404s. If for some reason an integration gives back
a different status code, this method could be overwritten.
So far only GitHub and GitLab have this implemented, both of which give
use back 404s. If for some reason an integration gives back a different
status code, this method could be overwritten.

repo: Repository (object)
filepath: file from the stacktrace (string)
Expand All @@ -43,16 +46,18 @@ def check_file(self, repo, filepath, branch):

return self.format_source_url(repo, filepath, branch)

def get_stacktrace_link(self, repo, filepath, default, version):
def get_stacktrace_link(
self, repo: Repository, filepath: str, default: str, version: str
) -> str | None:
"""
Handle formatting and returning back the stack trace link if the client
request was successful.

Uses the version first, and re-tries with the default branch if we 404
trying to use the version (commit sha).

If no file was found return `None`, and re-raise for non "Not Found" errors

If no file was found return `None`, and re-raise for non-"Not Found"
errors, like 403 "Account Suspended".
"""
with configure_scope() as scope:
scope.set_tag("stacktrace_link.tried_version", False)
Expand All @@ -67,7 +72,7 @@ def get_stacktrace_link(self, repo, filepath, default, version):

return source_url

def get_repositories(self, query=None):
def get_repositories(self, query: str | None = None) -> Sequence[Repository]:
"""
Get a list of available repositories for an installation

Expand All @@ -85,24 +90,22 @@ def get_repositories(self, query=None):
"""
raise NotImplementedError

def get_unmigratable_repositories(self):
def get_unmigratable_repositories(self) -> Sequence[Repository]:
return []

def reinstall_repositories(self):
"""
reinstalls repositories associated with the integration
"""
def reinstall_repositories(self) -> None:
"""Reinstalls repositories associated with the integration."""
organizations = self.model.organizations.all()
Repository.objects.filter(
organization_id__in=organizations.values_list("id", flat=True),
provider=f"integrations:{self.model.provider}",
integration_id=self.model.id,
).update(status=ObjectStatus.VISIBLE)

def has_repo_access(self, repo):
def has_repo_access(self, repo: Repository) -> bool:
raise NotImplementedError

def get_codeowner_file(self, repo, ref=None):
def get_codeowner_file(self, repo: Repository, ref: str | None = None) -> Mapping[str, str]:
"""
Find and get the contents of a CODEOWNERS file.
Should use client().get_file to get and decode the contents.
Expand Down
18 changes: 6 additions & 12 deletions src/sentry/shared_integrations/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,18 +284,21 @@ def request(self, *args, **kwargs):
def delete(self, *args, **kwargs):
return self.request("DELETE", *args, **kwargs)

def get_cached(self, path, *args, **kwargs):
def _get_cached(self, path: str, method: str, *args, **kwargs):
query = ""
if kwargs.get("params", None):
query = json.dumps(kwargs.get("params"), sort_keys=True)
key = self.get_cache_prefix() + md5_text(self.build_url(path), query).hexdigest()

result = cache.get(key)
if result is None:
result = self.request("GET", path, *args, **kwargs)
result = self.request(method, path, *args, **kwargs)
cache.set(key, result, self.cache_time)
return result

def get_cached(self, path, *args, **kwargs):
return self._get_cached(path, "GET", *args, **kwargs)

def get(self, *args, **kwargs):
return self.request("GET", *args, **kwargs)

Expand All @@ -312,16 +315,7 @@ def head(self, *args, **kwargs):
return self.request("HEAD", *args, **kwargs)

def head_cached(self, path, *args, **kwargs):
query = ""
if kwargs.get("params", None):
query = json.dumps(kwargs.get("params"), sort_keys=True)
key = self.get_cache_prefix() + md5_text(self.build_url(path), query).hexdigest()

result = cache.get(key)
if result is None:
result = self.head(path, *args, **kwargs)
cache.set(key, result, self.cache_time)
return result
return self._get_cached(path, "HEAD", *args, **kwargs)

def get_with_pagination(self, path, gen_params, get_results, *args, **kwargs):
page_size = self.page_size
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,17 @@ type Props = AsyncComponent['props'] & {
projects: Project[];
};

export type StacktraceErrorMessage =
| 'file_not_found'
| 'stack_root_mismatch'
| 'integration_link_forbidden';

// format of the ProjectStacktraceLinkEndpoint response
type StacktraceResultItem = {
integrations: Integration[];
config?: RepositoryProjectPathConfigWithIntegration;
sourceUrl?: string;
error?: 'file_not_found' | 'stack_root_mismatch';
error?: StacktraceErrorMessage;
attemptedUrl?: string;
};

Expand Down Expand Up @@ -77,6 +82,8 @@ class StacktraceLink extends AsyncComponent<Props, State> {
return t('Error matching your configuration.');
case 'file_not_found':
return t('Source file not found.');
case 'integration_link_forbidden':
return t('The repository integration was disconnected.');
default:
return t('There was an error encountered with the code mapping for this project');
}
Expand Down
3 changes: 2 additions & 1 deletion static/app/utils/analytics/integrationAnalyticsEvents.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {StacktraceErrorMessage} from 'app/components/events/interfaces/frame/stacktraceLink';
import {IntegrationType, PlatformType, SentryAppStatus} from 'app/types';

// define the various event payloads
Expand Down Expand Up @@ -41,7 +42,7 @@ type IntegrationStacktraceLinkEventParams = {
provider?: string;
platform?: PlatformType;
setup_type?: 'automatic' | 'manual';
error_reason?: 'file_not_found' | 'stack_root_mismatch';
error_reason?: StacktraceErrorMessage;
} & View;

type IntegrationServerlessFunctionsViewedParams = {
Expand Down