feat(github): Add issue types support for Sentry issues#111501
feat(github): Add issue types support for Sentry issues#111501Asynchronite wants to merge 22 commits intogetsentry:masterfrom
Conversation
github-actions
bot
added
the
Scope: Backend
cvxluo
added
the
Trigger: getsentry tests
|
Does the team who usually reviews this get automatically notified or..? |
billyvg
left a comment
billyvg
left a comment
There was a problem hiding this comment.
some nits w/ naming but otherwise looks fine -- this needs some frontend work though to actually add it to the modal I think?
| """ | ||
| return self._get_with_pagination(f"/repos/{owner}/{repo}/labels") | ||
|
|
||
| def get_types(self, owner: str) -> list[Any]: |
There was a problem hiding this comment.
let's be a bit more specific w/ the naming since types is very generic
| def get_types(self, owner: str) -> list[Any]: | |
| def get_issue_types(self, owner: str) -> list[Any]: |
| owner, repo = default_repo.split("/") | ||
| labels = self.get_repo_labels(owner, repo) | ||
| try: | ||
| types = self.get_org_types(owner) |
There was a problem hiding this comment.
| types = self.get_org_types(owner) | |
| types = self.get_issue_types(owner) |
There was a problem hiding this comment.
if these are org-wide types (as opposed to repo-wide types) it could also be get_org_issue_types or something too. But more prescriptive as Billy said makes sense
|
|
||
| assignees = self.get_allowed_assignees(default_repo) if default_repo else [] | ||
| labels: Sequence[tuple[str, str]] = [] | ||
| types: Sequence[tuple[str, str]] = [] |
There was a problem hiding this comment.
let's call this issue_types everywhere too
Note: We split FE and BE work in separate PRs |
|
@billyvg @bruno-garcia Implemented both of your changes, thanks for the suggestions (If one of you would be so kind as to re-review and trigger the |
github-actions
bot
removed
the
Trigger: getsentry tests
Frontend work is not really my domain, though if you can point me to the repo/code which houses this stuff I can def. look into it! |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| issue_types = self.get_org_issue_types(owner) | ||
| except IntegrationResourceNotFoundError: | ||
| issue_types = [] |
There was a problem hiding this comment.
Bug: The exception handling for get_org_issue_types is too narrow, only catching IntegrationResourceNotFoundError. Other API errors, like 403s, will cause uncaught exceptions and crash callers.
Severity: HIGH
Suggested Fix
Broaden the exception handler to catch IntegrationError in addition to or instead of IntegrationResourceNotFoundError. This will gracefully handle other expected API errors like 403 Forbidden (IntegrationConfigurationError) and fall back to an empty list as intended.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/sentry/integrations/github/issues.py#L188-L190
Potential issue: The call to `get_org_issue_types` is wrapped in a `try...except` block
that only catches `IntegrationResourceNotFoundError`. However, the underlying API can
return other errors, such as a 403 Forbidden if the feature is not enabled for an
organization, which raises an `IntegrationConfigurationError`. This uncaught exception
will propagate. When this function is called from `IntegrationConfigSerializer`, which
has no exception handling, the serializer will crash. This breaks the ticket rules
configuration modal for GitHub integrations where the issue types feature is unavailable
for reasons other than a 404.
Did we get this right? 👍 / 👎 to inform future reviews.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| try: | ||
| issue_types = self.get_org_issue_types(owner) | ||
| except IntegrationResourceNotFoundError: | ||
| issue_types = [] |
There was a problem hiding this comment.
Non-404 errors from issue types API crash form
Medium Severity
The try/except around get_org_issue_types only catches IntegrationResourceNotFoundError (HTTP 404). If the GitHub API returns a 403 (e.g., the GitHub App lacks org-level permissions for the newer issue types API) or any other non-404 error, raise_error converts it to IntegrationConfigurationError or IntegrationError, which propagates uncaught and prevents the entire issue creation form from loading. Since issue types is an optional, non-required field and a relatively new GitHub feature (March 2025), this new API call can break a previously-working form for existing installations. Broadening the exception handler to catch a wider set of exceptions (or catching Exception and falling back to []) would make this more resilient.
Additional Locations (1)
bruno-garcia
added
the
Trigger: getsentry tests
|
Thank you! Retriggered CI. Team will take a look next week |
4 participants
This PR introduces the addition of types to the modal that opens when you try to link a GitHub issue to an external code tracking software.
An example of issue types:
Closes #111404.
@cvxluo
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.