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

bug-1935274: increase message length for BugzillaRestHTTPUnexpectedError #6833

Merged
merged 1 commit into from
Dec 10, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions webapp/crashstats/crashstats/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1041,8 +1041,8 @@ def get(self, bugs, **kwargs):
if "bugs" not in data:
# We know the payload is JSON, but we don't know what shape it is--could
# be an array or an object. We know it doesn't have sensitive data in
# it, so let's dump to a string and truncate that to 100 characters.
payload_data = response.content[:100]
# it, so let's dump to a string and truncate that to 200 characters.
payload_data = response.content[:200]
Copy link
Member Author

@relud relud Dec 9, 2024

Choose a reason for hiding this comment

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

in sentry we can see that response.content is a bytes and not a string. I tried adding a .decode("utf8") here, but found out that in tests we mock it as a string. I decided the test is close enough and to leave it as is, rather than try to modify the test.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's my bad. I should remember that .content returns a bytes. It gets !r later, so it doesn't need to be a string. We could fix the mocked test to make it a bytes.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's also response.text, which is the same as .content, but decoded to a Unicode string.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't switch to .text here. I think we want to know what the payload is and avoid additional transforms that can kick up other issues.

raise BugzillaRestHTTPUnexpectedError(
f"status code: {response.status_code}, payload: {payload_data!r}"
)
Expand Down