diff --git a/src/sentry/api/endpoints/project_stacktrace_link.py b/src/sentry/api/endpoints/project_stacktrace_link.py index 2d020b65ceb234..8f4a32c49d44bc 100644 --- a/src/sentry/api/endpoints/project_stacktrace_link.py +++ b/src/sentry/api/endpoints/project_stacktrace_link.py @@ -1,3 +1,5 @@ +from typing import Optional, Tuple + from rest_framework.response import Response from sentry_sdk import configure_scope @@ -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): @@ -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 @@ -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) diff --git a/src/sentry/integrations/custom_scm/integration.py b/src/sentry/integrations/custom_scm/integration.py index 0d3573c61d831a..2d1a8efce1fe89 100644 --- a/src/sentry/integrations/custom_scm/integration.py +++ b/src/sentry/integrations/custom_scm/integration.py @@ -1,3 +1,5 @@ +from __future__ import annotations + from uuid import uuid4 from django import forms @@ -52,7 +54,9 @@ 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 @@ -60,7 +64,7 @@ def get_stacktrace_link(self, repo, filepath, default, version): """ 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}" diff --git a/src/sentry/integrations/example/integration.py b/src/sentry/integrations/example/integration.py index 3ca76e027e436b..83111c2e5fa054 100644 --- a/src/sentry/integrations/example/integration.py +++ b/src/sentry/integrations/example/integration.py @@ -1,4 +1,6 @@ -from typing import Any, Mapping, Optional +from __future__ import annotations + +from typing import Any, Mapping from django.http import HttpResponse @@ -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 @@ -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: @@ -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}" diff --git a/src/sentry/integrations/github/client.py b/src/sentry/integrations/github/client.py index 833b08455e204b..01c9b94d972951 100644 --- a/src/sentry/integrations/github/client.py +++ b/src/sentry/integrations/github/client.py @@ -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 @@ -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}) diff --git a/src/sentry/integrations/github/integration.py b/src/sentry/integrations/github/integration.py index ace5e1d6009f18..ab4d441ec9c0cc 100644 --- a/src/sentry/integrations/github/integration.py +++ b/src/sentry/integrations/github/integration.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import re from django.utils.text import slugify @@ -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}" diff --git a/src/sentry/integrations/gitlab/client.py b/src/sentry/integrations/gitlab/client.py index 39b56e9a9aba00..c9551a0a59dbf3 100644 --- a/src/sentry/integrations/gitlab/client.py +++ b/src/sentry/integrations/gitlab/client.py @@ -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 @@ -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 diff --git a/src/sentry/integrations/gitlab/integration.py b/src/sentry/integrations/gitlab/integration.py index 3aa6464b4b4778..a52f5fe1a29d72 100644 --- a/src/sentry/integrations/gitlab/integration.py +++ b/src/sentry/integrations/gitlab/integration.py @@ -1,3 +1,5 @@ +from __future__ import annotations + from urllib.parse import urlparse from django import forms @@ -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 @@ -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"] diff --git a/src/sentry/integrations/repositories.py b/src/sentry/integrations/repositories.py index f3200ff69a0291..2fe43ea510362b 100644 --- a/src/sentry/integrations/repositories.py +++ b/src/sentry/integrations/repositories.py @@ -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 @@ -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) @@ -43,7 +46,9 @@ 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. @@ -51,8 +56,8 @@ def get_stacktrace_link(self, repo, filepath, default, version): 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) @@ -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 @@ -85,13 +90,11 @@ 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), @@ -99,10 +102,10 @@ def reinstall_repositories(self): 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. diff --git a/src/sentry/shared_integrations/client.py b/src/sentry/shared_integrations/client.py index df938c3b1c54a4..e541ee8d3a3346 100644 --- a/src/sentry/shared_integrations/client.py +++ b/src/sentry/shared_integrations/client.py @@ -284,7 +284,7 @@ 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) @@ -292,10 +292,13 @@ def get_cached(self, path, *args, **kwargs): 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) @@ -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 diff --git a/static/app/components/events/interfaces/frame/stacktraceLink.tsx b/static/app/components/events/interfaces/frame/stacktraceLink.tsx index dacfdc0a61f379..f32ab3a5bd720c 100644 --- a/static/app/components/events/interfaces/frame/stacktraceLink.tsx +++ b/static/app/components/events/interfaces/frame/stacktraceLink.tsx @@ -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; }; @@ -77,6 +82,8 @@ class StacktraceLink extends AsyncComponent { 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'); } diff --git a/static/app/utils/analytics/integrationAnalyticsEvents.tsx b/static/app/utils/analytics/integrationAnalyticsEvents.tsx index 36c78150dd4f5d..f0288d68467644 100644 --- a/static/app/utils/analytics/integrationAnalyticsEvents.tsx +++ b/static/app/utils/analytics/integrationAnalyticsEvents.tsx @@ -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 @@ -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 = {