Skip to content

Commit

Permalink
FIX error handling when adding comments to Jira (#59)
Browse files Browse the repository at this point in the history
In some cases Jira returned error messages in "errorMessages" and field
"errors" is empty which caused IndexError...
  • Loading branch information
fpob authored and dmranck committed Sep 14, 2018
1 parent 08617a5 commit e7fa7fa
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 4 deletions.
54 changes: 53 additions & 1 deletion tests/test_jira.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import logging
from collections import namedtuple
from unittest import main, TestCase
from unittest.mock import patch
from unittest.mock import patch, Mock

import requests
from ticketutil.ticket import TicketException
Expand Down Expand Up @@ -506,6 +506,58 @@ def test_prepare_ticket_fields_components(self):
prepared_fields = jira._prepare_ticket_fields(fields)
self.assertEqual(prepared_fields, expected_fields)

@patch.object(jira.JiraTicket, '_create_requests_session')
def test_comment_errors(self, mock_session_factoy):
"""
Test if error responses are handled correctly when adding comments.
"""
mock_response = Mock()
mock_response.raise_for_status.side_effect = requests.RequestException()
mock_response.status_code = 400
mock_response.json.return_value = {
"errorMessages": [],
"errors": {
"comment": "Comment body can not be empty!"
}
}

mock_session = Mock()
mock_session.post = Mock(return_value=mock_response)

mock_session_factoy.return_value = mock_session

ticket = jira.JiraTicket(URL, PROJECT, ticket_id=TICKET_ID)

# This should not raise `IndexError: list index out of range` when
# handling error
ticket.add_comment('foobar')

@patch.object(jira.JiraTicket, '_create_requests_session')
def test_comment_error_messages(self, mock_session_factoy):
"""
Test if error responses are handled correctly when adding comments.
"""
mock_response = Mock()
mock_response.raise_for_status.side_effect = requests.RequestException()
mock_response.status_code = 400
mock_response.json.return_value = {
"errorMessages": [
"You do not have the permission to comment on this issue."
],
"errors": {}
}

mock_session = Mock()
mock_session.post.return_value = mock_response

mock_session_factoy.return_value = mock_session

ticket = jira.JiraTicket(URL, PROJECT, ticket_id=TICKET_ID)

# This should not raise `IndexError: list index out of range` when
# handling error
ticket.add_comment('foobar')


if __name__ == '__main__':
main()
14 changes: 11 additions & 3 deletions ticketutil/jira.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ def _create_ticket_request(self, params):
logger.debug("Create ticket: status code: {0}".format(r.status_code))
r.raise_for_status()
except requests.RequestException as e:
error_message = "Error creating ticket - {0}".format(list(r.json()['errors'].values())[0])
error_message = "Error creating ticket - {0}".format(_extract_error_messages(r.json()))
logger.error(error_message)
logger.error(e)
return self.request_result._replace(status='Failure', error_message=error_message)
Expand Down Expand Up @@ -236,7 +236,7 @@ def edit(self, **kwargs):
self.request_result = self.get_ticket_content()
return self.request_result
except requests.RequestException as e:
error_message = "Error editing ticket - {0}".format(list(r.json()['errors'].values())[0])
error_message = "Error editing ticket - {0}".format(_extract_error_messages(r.json()))
logger.error(error_message)
logger.error(e)
return self.request_result._replace(status='Failure', error_message=error_message)
Expand Down Expand Up @@ -264,7 +264,7 @@ def add_comment(self, comment):
self.request_result = self.get_ticket_content()
return self.request_result
except requests.RequestException as e:
error_message = "Error adding comment to ticket - {0}".format(list(r.json()['errors'].values())[0])
error_message = "Error adding comment to ticket - {0}".format(_extract_error_messages(r.json()))
logger.error(error_message)
logger.error(e)
return self.request_result._replace(status='Failure', error_message=error_message)
Expand Down Expand Up @@ -512,6 +512,14 @@ def _prepare_ticket_fields(fields):
return fields


def _extract_error_messages(data):
if data.get('errorMessages'):
return ' '.join(data['errorMessages'])
elif data.get('errors'):
return ' '.join(data['errors'].values())
return ''


def main():
"""
main() function, not directly callable.
Expand Down

0 comments on commit e7fa7fa

Please sign in to comment.