Merge pull request #89 from rbriski/rbriski/jira_error

Adding jira.Error to better wrap jira-specific errors

board.go

@@ -75,7 +75,8 @@ func (s *BoardService) GetAllBoards(opt *BoardListOptions) (*BoardsList, *Respon
 	boards := new(BoardsList)
 	resp, err := s.client.Do(req, boards)
 	if err != nil {
-		return nil, resp, err
+		jerr := NewJiraError(resp, err)
+		return nil, resp, jerr
 	}
 
 	return boards, resp, err
@@ -95,8 +96,10 @@ func (s *BoardService) GetBoard(boardID int) (*Board, *Response, error) {
 	board := new(Board)
 	resp, err := s.client.Do(req, board)
 	if err != nil {
-		return nil, resp, err
+		jerr := NewJiraError(resp, err)
+		return nil, resp, jerr
 	}
+
 	return board, resp, nil
 }
 
@@ -118,7 +121,8 @@ func (s *BoardService) CreateBoard(board *Board) (*Board, *Response, error) {
 	responseBoard := new(Board)
 	resp, err := s.client.Do(req, responseBoard)
 	if err != nil {
-		return nil, resp, err
+		jerr := NewJiraError(resp, err)
+		return nil, resp, jerr
 	}
 
 	return responseBoard, resp, nil
@@ -135,6 +139,9 @@ func (s *BoardService) DeleteBoard(boardID int) (*Board, *Response, error) {
 	}
 
 	resp, err := s.client.Do(req, nil)
+	if err != nil {
+		err = NewJiraError(resp, err)
+	}
 	return nil, resp, err
 }
 
@@ -151,5 +158,9 @@ func (s *BoardService) GetAllSprints(boardID string) ([]Sprint, *Response, error
 
 	result := new(sprintsResult)
 	resp, err := s.client.Do(req, result)
+	if err != nil {
+		err = NewJiraError(resp, err)
+	}
+
 	return result.Sprints, resp, err
 }

error.go

@@ -0,0 +1,77 @@
+package jira
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+
+	"github.com/pkg/errors"
+)
+
+// Error message from JIRA
+// See https://docs.atlassian.com/jira/REST/cloud/#error-responses
+type Error struct {
+	HTTPError     error
+	ErrorMessages []string          `json:"errorMessages"`
+	Errors        map[string]string `json:"errors"`
+}
+
+// NewJiraError creates a new jira Error
+func NewJiraError(resp *Response, httpError error) error {
+	defer resp.Body.Close()
+	body, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return errors.Wrap(err, httpError.Error())
+	}
+
+	jerr := Error{HTTPError: httpError}
+	err = json.Unmarshal(body, &jerr)
+	if err != nil {
+		return errors.Wrap(err, err.Error())
+	}
+
+	return &jerr
+}
+
+// Error is a short string representing the error
+func (e *Error) Error() string {
+	if len(e.ErrorMessages) > 0 {
+		// return fmt.Sprintf("%v", e.HTTPError)
+		return fmt.Sprintf("%s: %v", e.ErrorMessages[0], e.HTTPError)
+	}
+	if len(e.Errors) > 0 {
+		for key, value := range e.Errors {
+			return fmt.Sprintf("%s - %s: %v", key, value, e.HTTPError)
+		}
+	}
+	return e.HTTPError.Error()
+}
+
+// LongError is a full representation of the error as a string
+func (e *Error) LongError() string {
+	var msg bytes.Buffer
+	if e.HTTPError != nil {
+		msg.WriteString("Original:\n")
+		msg.WriteString(e.HTTPError.Error())
+		msg.WriteString("\n")
+	}
+	if len(e.ErrorMessages) > 0 {
+		msg.WriteString("Messages:\n")
+		for _, v := range e.ErrorMessages {
+			msg.WriteString(" - ")
+			msg.WriteString(v)
+			msg.WriteString("\n")
+		}
+	}
+	if len(e.Errors) > 0 {
+		for key, value := range e.Errors {
+			msg.WriteString(" - ")
+			msg.WriteString(key)
+			msg.WriteString(" - ")
+			msg.WriteString(value)
+			msg.WriteString("\n")
+		}
+	}
+	return msg.String()
+}

error_test.go

@@ -0,0 +1,133 @@
+package jira
+
+import (
+	"errors"
+	"fmt"
+	"net/http"
+	"strings"
+	"testing"
+)
+
+func TestError_NewJiraError(t *testing.T) {
+	setup()
+	defer teardown()
+
+	testMux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprint(w, `{"errorMessages":["Issue does not exist or you do not have permission to see it."],"errors":{}}`)
+	})
+
+	req, _ := testClient.NewRequest("GET", "/", nil)
+	resp, _ := testClient.Do(req, nil)
+
+	err := NewJiraError(resp, errors.New("Original http error"))
+	if err, ok := err.(*Error); !ok {
+		t.Errorf("Expected jira Error. Got %s", err.Error())
+	}
+
+	if !strings.Contains(err.Error(), "Issue does not exist") {
+		t.Errorf("Expected issue message. Got: %s", err.Error())
+	}
+}
+
+func TestError_NilOriginalMessage(t *testing.T) {
+	defer func() {
+		if r := recover(); r != nil {
+			t.Errorf("Expected an error message. Got a panic (%v)", r)
+		}
+	}()
+
+	msgErr := &Error{
+		HTTPError:     nil,
+		ErrorMessages: []string{"Issue does not exist"},
+		Errors: map[string]string{
+			"issuetype": "issue type is required",
+			"title":     "title is required",
+		},
+	}
+
+	_ = msgErr.Error()
+}
+
+func TestError_NilOriginalMessageLongError(t *testing.T) {
+	defer func() {
+		if r := recover(); r != nil {
+			t.Errorf("Expected an error message. Got a panic (%v)", r)
+		}
+	}()
+
+	msgErr := &Error{
+		HTTPError:     nil,
+		ErrorMessages: []string{"Issue does not exist"},
+		Errors: map[string]string{
+			"issuetype": "issue type is required",
+			"title":     "title is required",
+		},
+	}
+
+	_ = msgErr.LongError()
+}
+
+func TestError_ShortMessage(t *testing.T) {
+	msgErr := &Error{
+		HTTPError:     errors.New("Original http error"),
+		ErrorMessages: []string{"Issue does not exist"},
+		Errors: map[string]string{
+			"issuetype": "issue type is required",
+			"title":     "title is required",
+		},
+	}
+
+	mapErr := &Error{
+		HTTPError:     errors.New("Original http error"),
+		ErrorMessages: nil,
+		Errors: map[string]string{
+			"issuetype": "issue type is required",
+			"title":     "title is required",
+		},
+	}
+
+	noErr := &Error{
+		HTTPError:     errors.New("Original http error"),
+		ErrorMessages: nil,
+		Errors:        nil,
+	}
+
+	err := msgErr.Error()
+	if err != "Issue does not exist: Original http error" {
+		t.Errorf("Expected short message. Got %s", err)
+	}
+
+	err = mapErr.Error()
+	if !(strings.Contains(err, "issue type is required") || strings.Contains(err, "title is required")) {
+		t.Errorf("Expected short message. Got %s", err)
+	}
+
+	err = noErr.Error()
+	if err != "Original http error" {
+		t.Errorf("Expected original error message. Got %s", err)
+	}
+}
+
+func TestError_LongMessage(t *testing.T) {
+	longError := &Error{
+		HTTPError:     errors.New("Original http error"),
+		ErrorMessages: []string{"Issue does not exist."},
+		Errors: map[string]string{
+			"issuetype": "issue type is required",
+			"title":     "title is required",
+		},
+	}
+
+	msg := longError.LongError()
+	if !strings.Contains(msg, "Original http error") {
+		t.Errorf("Expected the error message: Got\n%s\n", msg)
+	}
+
+	if !strings.Contains(msg, "Issue does not exist") {
+		t.Errorf("Expected the error message: Got\n%s\n", msg)
+	}
+
+	if !strings.Contains(msg, "title - title is required") {
+		t.Errorf("Expected the error map: Got\n%s\n", msg)
+	}
+}

issue.go

@@ -496,7 +496,8 @@ func (s *IssueService) Get(issueID string, options *GetQueryOptions) (*Issue, *R
 	issue := new(Issue)
 	resp, err := s.client.Do(req, issue)
 	if err != nil {
-		return nil, resp, err
+		jerr := NewJiraError(resp, err)
+		return nil, resp, jerr
 	}
 
 	return issue, resp, nil
@@ -515,7 +516,8 @@ func (s *IssueService) DownloadAttachment(attachmentID string) (*Response, error
 
 	resp, err := s.client.Do(req, nil)
 	if err != nil {
-		return resp, err
+		jerr := NewJiraError(resp, err)
+		return resp, jerr
 	}
 
 	return resp, nil
@@ -552,7 +554,8 @@ func (s *IssueService) PostAttachment(issueID string, r io.Reader, attachmentNam
 	attachment := new([]Attachment)
 	resp, err := s.client.Do(req, attachment)
 	if err != nil {
-		return nil, resp, err
+		jerr := NewJiraError(resp, err)
+		return nil, resp, jerr
 	}
 
 	return attachment, resp, nil
@@ -616,7 +619,8 @@ func (s *IssueService) Update(issue *Issue) (*Issue, *Response, error) {
 	}
 	resp, err := s.client.Do(req, nil)
 	if err != nil {
-		return nil, resp, err
+		jerr := NewJiraError(resp, err)
+		return nil, resp, jerr
 	}
 
 	// This is just to follow the rest of the API's convention of returning an issue.
@@ -657,7 +661,8 @@ func (s *IssueService) AddComment(issueID string, comment *Comment) (*Comment, *
 	responseComment := new(Comment)
 	resp, err := s.client.Do(req, responseComment)
 	if err != nil {
-		return nil, resp, err
+		jerr := NewJiraError(resp, err)
+		return nil, resp, jerr
 	}
 
 	return responseComment, resp, nil
@@ -698,6 +703,10 @@ func (s *IssueService) AddLink(issueLink *IssueLink) (*Response, error) {
 	}
 
 	resp, err := s.client.Do(req, nil)
+	if err != nil {
+		err = NewJiraError(resp, err)
+	}
+
 	return resp, err
 }
 
@@ -720,6 +729,9 @@ func (s *IssueService) Search(jql string, options *SearchOptions) ([]Issue, *Res
 
 	v := new(searchResult)
 	resp, err := s.client.Do(req, v)
+	if err != nil {
+		err = NewJiraError(resp, err)
+	}
 	return v.Issues, resp, err
 }
 
@@ -774,7 +786,8 @@ func (s *IssueService) GetCustomFields(issueID string) (CustomFields, *Response,
 	issue := new(map[string]interface{})
 	resp, err := s.client.Do(req, issue)
 	if err != nil {
-		return nil, resp, err
+		jerr := NewJiraError(resp, err)
+		return nil, resp, jerr
 	}
 
 	m := *issue
@@ -812,6 +825,9 @@ func (s *IssueService) GetTransitions(id string) ([]Transition, *Response, error
 
 	result := new(transitionResult)
 	resp, err := s.client.Do(req, result)
+	if err != nil {
+		err = NewJiraError(resp, err)
+	}
 	return result.Transitions, resp, err
 }
 
@@ -842,10 +858,10 @@ func (s *IssueService) DoTransitionWithPayload(ticketID, payload interface{}) (*
 
 	resp, err := s.client.Do(req, nil)
 	if err != nil {
-		return nil, err
+		err = NewJiraError(resp, err)
 	}
 
-	return resp, nil
+	return resp, err
 }
 
 // InitIssueWithMetaAndFields returns Issue with with values from fieldsConfig properly set.

project.go

@@ -94,8 +94,10 @@ func (s *ProjectService) GetList() (*ProjectList, *Response, error) {
 	projectList := new(ProjectList)
 	resp, err := s.client.Do(req, projectList)
 	if err != nil {
-		return nil, resp, err
+		jerr := NewJiraError(resp, err)
+		return nil, resp, jerr
 	}
+
 	return projectList, resp, nil
 }
 
@@ -114,7 +116,9 @@ func (s *ProjectService) Get(projectID string) (*Project, *Response, error) {
 	project := new(Project)
 	resp, err := s.client.Do(req, project)
 	if err != nil {
-		return nil, resp, err
+		jerr := NewJiraError(resp, err)
+		return nil, resp, jerr
 	}
+
 	return project, resp, nil
 }

sprint.go

@@ -37,6 +37,9 @@ func (s *SprintService) MoveIssuesToSprint(sprintID int, issueIDs []string) (*Re
 	}
 
 	resp, err := s.client.Do(req, nil)
+	if err != nil {
+		err = NewJiraError(resp, err)
+	}
 	return resp, err
 }
 
@@ -56,5 +59,9 @@ func (s *SprintService) GetIssuesForSprint(sprintID int) ([]Issue, *Response, er
 
 	result := new(IssuesInSprintResult)
 	resp, err := s.client.Do(req, result)
+	if err != nil {
+		err = NewJiraError(resp, err)
+	}
+
 	return result.Issues, resp, err
 }