Make Requester.__createException robust against missing message and body #2159
Conversation
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## master #2159 +/- ##
=======================================
Coverage 98.68% 98.68%
=======================================
Files 117 117
Lines 11825 11826 +1
=======================================
+ Hits 11670 11671 +1
Misses 155 155
☔ View full report in Codecov by Sentry. |
EnricoMi
force-pushed
the
branch-fix-error-in-createException
branch
from
e508688
to
563ad66
Compare
EnricoMi
force-pushed
the
branch-fix-error-in-createException
branch
from
563ad66
to
07fd134
Compare
|
@s-t-e-v-e-n-k what do you think, can we get this merged? |
EnricoMi
force-pushed
the
branch-fix-error-in-createException
branch
from
07fd134
to
1c2da8b
Compare
| @@ -452,26 +452,26 @@ def __customConnection(self, url): | |||
| return cnx | |||
|
|
|||
| def __createException(self, status, headers, output): | |||
| if status == 401 and output.get("message") == "Bad credentials": | |||
| message = output.get("message", "").lower() if output is not None else "" | |||
There was a problem hiding this comment.
Good idea making this case insensitive! 👍
This adds tests for
Requester.__createExceptionwhile making this method robust againstoutputmissing"message"andoutput = Nonefor all status.Note: This simplifies logic while making
BadCredentialsException,BadUserAgentExceptionandUnknownObjectExceptioncase-insensitive to the message.Fixes #1399 and #2158.
Note: Add that extra message from #2127 to
tests.Requester.testShouldCreateRateLimitExceededException, either in that PR or here, whatever gets merged last.