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.

Sign up for GitHub

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

Jump to bottom

Output error message for provider failures #578

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Output error message for provider failures #578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e3b1db2
Output error message for provider failures
pyoor Oct 24, 2019
a62a6e2
Fix flak8 failures
pyoor Oct 24, 2019
499c4bb
Handle potential failures in BZ error responses
pyoor Nov 13, 2019
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
34 changes: 24 additions & 10 deletions server/crashmanager/Bugtracker/BugzillaProvider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,15 @@
from django.utils import dateparse

from .BugzillaREST import BugzillaREST
from .Provider import Provider
from .Provider import Provider, ProviderException
from ..models import BugzillaTemplate, User


class BugzillaProviderException(ProviderException):
''' Bugzilla Provider related exceptions '''
pass


class BugzillaProvider(Provider):
def __init__(self, pk, hostname):
super(BugzillaProvider, self).__init__(pk, hostname)
Expand Down Expand Up @@ -302,14 +307,23 @@ def handlePOSTCreate(self, request, crashEntry):
# Create the bug
bz = BugzillaREST(self.hostname, username, password, api_key)
ret = bz.createBug(**args)
if "id" not in ret:
raise RuntimeError("Failed to create bug: %s", ret)
if ret.status_code != 200:
try:
content = json.loads(ret.content)
if 'message' not in content:
raise ValueError
except ValueError:
raise BugzillaProviderException('Invalid JSON response')

raise BugzillaProviderException(content['message'])
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably try to handle more error cases here:

  1. The resulting JSON might miss the message.
  2. ret.content might not even parse as JSON (if the server has a hickup and returns nothing or e.g. the hardhat page)

In both cases, this would cause a 500 again. Instead we should probably wrap these cases in a BugzillaProviderException as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


body = ret.json()

# If we were told to attach the crash data, do so now
if crashdata_attach:
cRet = bz.addAttachment(ret["id"], crashdata_attach, "crash_data.txt", "Detailed Crash Information",
cRet = bz.addAttachment(body["id"], crashdata_attach, "crash_data.txt", "Detailed Crash Information",
is_binary=False)
ret["crashdataAttachmentResponse"] = cRet
body["crashdataAttachmentResponse"] = cRet

# If we have a binary testcase or the testcase is too large,
# attach it here in a second step
Expand All @@ -323,13 +337,13 @@ def handlePOSTCreate(self, request, crashEntry):
filename = submitted_testcase_filename

if crashEntry.testcase.isBinary:
aRet = bz.addAttachment(ret["id"], data, filename, "Testcase", is_binary=True)
ret["attachmentResponse"] = aRet
aRet = bz.addAttachment(body["id"], data, filename, "Testcase", is_binary=True)
body["attachmentResponse"] = aRet
elif len(data) > 2048 or testcase_attach:
aRet = bz.addAttachment(ret["id"], data, filename, "Testcase", is_binary=False)
ret["attachmentResponse"] = aRet
aRet = bz.addAttachment(body["id"], data, filename, "Testcase", is_binary=False)
body["attachmentResponse"] = aRet

return ret["id"]
return body["id"]

def handlePOSTComment(self, request, crashEntry):
args = {}
Expand Down
3 changes: 1 addition & 2 deletions server/crashmanager/Bugtracker/BugzillaREST.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,7 @@ def createBug(self, product, component, summary, version, type='defect', descrip
if self.login():
createUrl = "%s?token=%s" % (createUrl, self.authToken)

response = requests.post(createUrl, bug, headers=self.request_headers)
return response.json()
return requests.post(createUrl, bug, headers=self.request_headers)

def createComment(self, id, comment, is_private=False):
if is_private:
Expand Down
5 changes: 5 additions & 0 deletions server/crashmanager/Bugtracker/Provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
import six


class ProviderException(Exception):
''' Generic Exception for Provider related errors '''
pass


@six.add_metaclass(ABCMeta)
class Provider():
'''
Expand Down
6 changes: 5 additions & 1 deletion server/crashmanager/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from .models import CrashEntry, Bucket, BucketWatch, BugProvider, Bug, Tool, User
from .serializers import InvalidArgumentException, BucketSerializer, CrashEntrySerializer
from server.auth import CheckAppPermission
from Bugtracker.Provider import ProviderException

from django.conf import settings as django_settings

Expand Down Expand Up @@ -959,7 +960,10 @@ def createExternalBug(request, crashid):

# Let the provider handle the POST request, which will file the bug
# and return us the external bug ID
extBugId = provider.getInstance().handlePOSTCreate(request, entry)
try:
extBugId = provider.getInstance().handlePOSTCreate(request, entry)
except ProviderException as e:
return renderError(request, e.message)

# Now create a bug in our database with that ID and assign it to the bucket
extBug = Bug(externalId=extBugId, externalType=provider)
Expand Down