Skip to content

Commit

Permalink
bug-1892787: handle case where Bugzilla returns bad payload (#6785)
Browse files Browse the repository at this point in the history
Sometimes Bugzilla returns an HTTP 200 with JSON content, but it doesn't
have "bugs" in it.

Rather than throw an HTTP 500 because of an unhandled KeyError, this
changes the code to raise a BugzillaRestHTTPUnexpectedError with the
payload it did get. This will give us a better idea of what's happening
so we can figure out what to do about it if anything.
  • Loading branch information
willkg authored Nov 2, 2024
1 parent 18c768a commit 0753f8d
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 2 deletions.
17 changes: 15 additions & 2 deletions webapp/crashstats/crashstats/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1033,12 +1033,25 @@ def get(self, bugs, **kwargs):
)
response = session.get(url, headers=headers)
if response.status_code != 200:
raise BugzillaRestHTTPUnexpectedError(response.status_code)
raise BugzillaRestHTTPUnexpectedError(
f"status code: {response.status_code}"
)

data = response.json()
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]
raise BugzillaRestHTTPUnexpectedError(
f"status code: {response.status_code}, payload: {payload_data!r}"
)

for each in response.json()["bugs"]:
for each in data["bugs"]:
cache_key = self.make_cache_key(each["id"])
cache.set(cache_key, each, self.BUG_CACHE_SECONDS)
results.append(each)

return {"bugs": results}

post = None
Expand Down
18 changes: 18 additions & 0 deletions webapp/crashstats/crashstats/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,24 @@ def mocked_get(url, **options):
with pytest.raises(models.BugzillaRestHTTPUnexpectedError):
api.get("123456789")

@mock.patch("requests.Session")
def test_bugzilla_api_no_bugs(self, rsession):
model = models.BugzillaBugInfo

api = model()

def mocked_get(url, **options):
# NOTE(willkg): Bugzilla sometimes returns an HTTP 200 with a JSON payload
# that has no "bugs" key. We don't know what is in it, but we do know it has
# no "bugs".
return Response({"otherkey": "othervalue"})

rsession().get.side_effect = mocked_get
with pytest.raises(models.BugzillaRestHTTPUnexpectedError) as excinfo:
api.get("123456789")

assert 'payload: \'{"otherkey": "othervalue"}\'' in str(excinfo.value)

@mock.patch("requests.Session")
def test_massive_querystring_caching(self, rsession):
# doesn't actually matter so much what API model we use
Expand Down

0 comments on commit 0753f8d

Please sign in to comment.