Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(derive_code_mappings): Do not report error if installation is not found #42487

Merged
merged 1 commit into from Dec 20, 2022

Conversation

armenzg
Copy link
Member

@armenzg armenzg commented Dec 20, 2022

This only handles the case when the Github App installation is not found:

{"message":"Not Found","documentation_url":"https://docs.github.com/rest/reference/apps#create-an-installation-access-token-for-an-app"}

Partially fixes SENTRY-WSE

@@ -71,6 +73,12 @@ def derive_code_mappings(
try:
with lock.acquire():
trees = installation.get_trees_for_org()
except ApiError as error:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we call get_trees_for_org, we need to request access tokens from the GH installation for that org.
If the admins, uninstalled the Sentry GH App we would then not be able to request tokens anymore.
If you load the Sentry issue you will see that HTTPError 404 Client Error: Not Found for url: https://api.github.com/app/installations/<some_id>/access_tokens is raised at the bottom of the stack trace.

json_data: JSONData = error.json
msg: str = json_data.get("message")
if msg == "Not Found":
logger.warning("The org has uninstalled the Sentry App.")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should stop raising errors when there's nothing we can do about it.

@armenzg armenzg marked this pull request as ready for review December 20, 2022 15:19
@armenzg armenzg requested review from a team December 20, 2022 15:19
@armenzg armenzg enabled auto-merge (squash) December 20, 2022 15:19
@armenzg armenzg merged commit 92c0eb2 into master Dec 20, 2022
@armenzg armenzg deleted the armenzg/fix/no-installation branch December 20, 2022 18:28
armenzg added a commit that referenced this pull request Dec 21, 2022
In #42487 we introduced a regression of not reporting other kinds of API Errors to Sentry
armenzg added a commit that referenced this pull request Dec 21, 2022
In #42487 we introduced a regression of not reporting other kinds of API Errors to Sentry.

There's also some other minor fixes.

Fixes [SENTRY-XPC](https://sentry.io/organizations/sentry/issues/3820120776).
Fixes [SENTRY-XPW](https://sentry.io/organizations/sentry/issues/3825762675).
@github-actions github-actions bot locked and limited conversation to collaborators Jan 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants