From 74996fe6a13c86974c160f2fd25b7c69cf07ad8c Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Thu, 31 Oct 2024 10:30:32 -0400 Subject: [PATCH] bug-1901998: fix buginfo view to stop sending sentry errors When Bugzilla is in a mood, the buginfo view queries the BugInfo API and that API throws a requests.exceptions.RetryError which the buginfo view didn't handle and thus we end up with a Sentry error. When this happens, the report view Bugzilla tab gets back an HTTP 500 from querying the buginfo view and kind of shrugs and doesn't show bug details. The user isn't really impacted--they can continue their merry day. Bugzilla seems to be down a lot, so we get these Sentry errors we're not going to do anything with. Thus I did a couple of things: 1. increase the backoff_factor for Bugzilla requests giving us a little more time for Bugzilla to find its happy place 2. change the buginfo view to handle the RetryError by returning an HTTP 500 with an error message and _not_ sending a Sentry event --- webapp/crashstats/crashstats/models.py | 9 ++++++--- .../crashstats/crashstats/tests/test_views.py | 17 +++++++++++++++++ webapp/crashstats/crashstats/views.py | 7 +++++-- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/webapp/crashstats/crashstats/models.py b/webapp/crashstats/crashstats/models.py index 9813c83b9f..a2f0267970 100644 --- a/webapp/crashstats/crashstats/models.py +++ b/webapp/crashstats/crashstats/models.py @@ -57,7 +57,7 @@ def get_bugs_and_related_bugs(self, signatures): class BugAssociation(models.Model): - """Specifies assocations between bug ids in Bugzilla and signatures""" + """Specifies associations between bug ids in Bugzilla and signatures""" bug_id = models.IntegerField(null=False, help_text="Bugzilla bug id") signature = models.TextField( @@ -244,7 +244,7 @@ class ParameterTypeError(Exception): def _clean_path(path): - """Santize a URL path + """Sanitize a URL path :arg path: the path to sanitize @@ -608,7 +608,7 @@ class ProcessedCrash(SocorroMiddleware): HELP_TEXT = """ API for retrieving crash data generated by processing. - Requires protected data acess to view protected parts of the processed crash. + Requires protected data access to view protected parts of the processed crash. Params: @@ -1027,6 +1027,9 @@ def get(self, bugs, **kwargs): # 503 = Service Unavailable # 504 = Gateway Time-out status_forcelist=(500, 502, 503, 504), + # This changes backoff to (0.2, 0.4, 0.8, 1.6, 3.2) which + # gives Bugzilla a better chance of responding helpfully + backoff_factor=0.2, ) response = session.get(url, headers=headers) if response.status_code != 200: diff --git a/webapp/crashstats/crashstats/tests/test_views.py b/webapp/crashstats/crashstats/tests/test_views.py index fee2a1927f..7cb802bc6d 100644 --- a/webapp/crashstats/crashstats/tests/test_views.py +++ b/webapp/crashstats/crashstats/tests/test_views.py @@ -11,6 +11,7 @@ import requests_mock import pyquery from markus.testing import MetricsMock +from requests.exceptions import RetryError from django.conf import settings from django.contrib.auth import REDIRECT_FIELD_NAME @@ -358,6 +359,22 @@ def mocked_get(url, **options): cache_key = "buginfo:987" assert cache.get(cache_key) == struct["bugs"][0] + def test_buginfo_retry_error(self, client): + with requests_mock.Mocker(real_http=False) as mock_requests: + mock_requests.get( + ( + "http://bugzilla.example.com/rest/bug?" + + "id=1901998&include_fields=summary,status,id,resolution" + ), + exc=RetryError, + ) + + url = reverse("crashstats:buginfo") + response = client.get(url, {"bug_ids": "1901998"}) + assert response.status_code == 500 + json_response = json.loads(response.content) + assert json_response == {"error": "Max retries exceeded with Bugzilla."} + class Test_quick_search: def test_quick_search(self, client): diff --git a/webapp/crashstats/crashstats/views.py b/webapp/crashstats/crashstats/views.py index 3ce9c719be..4696020669 100644 --- a/webapp/crashstats/crashstats/views.py +++ b/webapp/crashstats/crashstats/views.py @@ -7,6 +7,7 @@ from urllib.parse import quote import glom +from requests.exceptions import RetryError from django import http from django.conf import settings @@ -332,8 +333,10 @@ def buginfo(request, signatures=None): bug_ids = form.cleaned_data["bug_ids"] bzapi = models.BugzillaBugInfo() - result = bzapi.get(bug_ids) - return result + try: + return bzapi.get(bug_ids) + except RetryError: + return {"error": "Max retries exceeded with Bugzilla."}, 500 @pass_default_context