Skip to content

Conversation

@iamrajjoshi
Copy link
Collaborator

@iamrajjoshi iamrajjoshi commented Jun 2, 2025

mark errors like

sentry.api.client.ApiError: status=400 body={'statusDetails': {'inNextRelease': [ErrorDetail(xxx]}}" as halts because it means the user hasn't configured releases correctly.

also actually capture the other halt and failure cases correctly b/c we weren't doing that before

closes ECO-576

@iamrajjoshi iamrajjoshi self-assigned this Jun 2, 2025
@iamrajjoshi iamrajjoshi requested review from a team as code owners June 2, 2025 21:09
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jun 2, 2025
@codecov
Copy link

codecov bot commented Jun 3, 2025

Codecov Report

Attention: Patch coverage is 22.22222% with 7 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/sentry/integrations/msteams/webhook.py 22.22% 7 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #92689       +/-   ##
===========================================
+ Coverage   42.17%   87.89%   +45.72%     
===========================================
  Files       10226    10251       +25     
  Lines      586770   587923     +1153     
  Branches    22826    22826               
===========================================
+ Hits       247451   516757   +269306     
+ Misses     338887    70734   -268153     
  Partials      432      432               

user=user_service.get_user(user_id=identity.user_id),
auth=event_write_key,
)
except client.ApiError as e:
Copy link
Contributor

Choose a reason for hiding this comment

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

curious Q is there a difference between client.ApiError vs ApiError?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is a different ApiError, the one we usually use is from shared_exceptions, so we need to annotate client here

Comment on lines +509 to +510
elif e.status_code >= 400:
lifecycle.record_failure(e)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: thoughts on letting the error through and triggering the lifecycle/recording failure on its own ? (Curious on your thoughts since I've seen both ways)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i think its stylistic atp

@iamrajjoshi iamrajjoshi merged commit 383be82 into master Jun 3, 2025
63 checks passed
@iamrajjoshi iamrajjoshi deleted the raj/msteam-resolve-halt branch June 3, 2025 17:06
@github-actions github-actions bot locked and limited conversation to collaborators Jun 19, 2025
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.

3 participants