chore(sentry apps): Audit errors for token exchange process #83064
Conversation
github-actions
bot
added
the
Scope: Backend
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #83064 +/- ##
===========================================
+ Coverage 56.21% 87.59% +31.38%
===========================================
Files 9416 9446 +30
Lines 536122 536852 +730
Branches 21120 21120
===========================================
+ Hits 301375 470258 +168883
+ Misses 234387 66234 -168153
Partials 360 360 |
|
|
||
| if not self._grant_is_active(): | ||
| raise APIUnauthorized("Grant has already expired") | ||
| raise SentryAppIntegratorError( | ||
| APIUnauthorized("Grant has already expired"), status_code=403 |
There was a problem hiding this comment.
Are we changing the status code for this, or is this just because we're now wrapping the error? Also is it worth using the same enums you're using in testing for these?
There was a problem hiding this comment.
APIUnauthorized has a built in status code of 403, but SentryAppIntegratorError's default status code is 400 and we use the status code attached to SentryAppIntegratorError.
er I'm a bit confused wdym by using the same enums you're using in testing for these like testing we raise a SentryAppIntegratorError ?
There was a problem hiding this comment.
Ahh okay, so it's a side effect of the composition swallowing the status code then 😅
er I'm a bit confused wdym by using the same enums you're using in testing for these like testing we raise a SentryAppIntegratorError ?
Oh I just meant the drf status import in the test:
from rest_framework import serializers, status
There was a problem hiding this comment.
ohhhhh, makes sense 👍
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
tl;dr audited and wrapped all the errors in the token exchange process
Move all the token exchange process errors to sentryapp errors, update the tests to follow suit