Logs enhancement

provider/github/client.go

@@ -414,14 +414,25 @@ func (t *limitRoundTripper) RoundTrip(req *http.Request) (*http.Response, error)
 
 var _ http.RoundTripper = &limitRoundTripper{}
 
-func handleAPIError(resp *github.Response, err error) error {
+func handleAPIError(resp *github.Response, err error, msg string) error {
 	if err != nil {
-		return ErrGitHubAPI.Wrap(err)
+		if e, ok := err.(*github.ErrorResponse); ok {
+			if e.Response == nil {
+				e.Response = resp.Response
+			} else if e.Response.Request == nil {
+				e.Response.Request = resp.Response.Request
+			}
+		}
+
+		return ErrGitHubAPI.Wrap(err, msg)
 	}
 
 	if resp.StatusCode == 200 {
 		return nil
 	}
 
-	return ErrGitHubAPI.Wrap(fmt.Errorf("bad HTTP status: %d", resp.StatusCode))
+	return ErrGitHubAPI.Wrap(
+		fmt.Errorf("bad HTTP status: %d", resp.StatusCode),
+		msg,
+	)
 }

provider/github/client_test.go

@@ -1,9 +1,14 @@
 package github
 
 import (
+	"fmt"
+	"net/http"
+	"net/url"
 	"testing"
 
+	"github.com/google/go-github/github"
 	"github.com/src-d/lookout"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gopkg.in/src-d/lookout-sdk.v0/pb"
 )
@@ -125,3 +130,41 @@ func TestClientPoolMultipleDeleteRepos(t *testing.T) {
 
 	require.Equal(newRepos, p.ReposByClient(client))
 }
+func TestErrorResponseDoesNotPanic(t *testing.T) {
+	require := require.New(t)
+
+	url, _ := url.Parse("http://example.com")
+
+	mockResponse := &github.Response{Response: &http.Response{
+		StatusCode: http.StatusOK,
+		Request:    &http.Request{Method: "GET", URL: url},
+	}}
+
+	apiResponseErrWithoutEmbededResponse := &github.ErrorResponse{Response: nil}
+
+	apiResponseErrWithEmbededResponseWithoutRequest := &github.ErrorResponse{
+		Response: &http.Response{
+			StatusCode: http.StatusOK,
+			Request:    nil,
+		},
+	}
+
+	apiResponseErrWithProperEmbededResponse := &github.ErrorResponse{
+		Response: &http.Response{
+			StatusCode: http.StatusOK,
+			Request:    &http.Request{Method: "GET", URL: url},
+		},
+	}
+
+	processAPIError := func(apiErr error) assert.PanicTestFunc {
+		return func() {
+			err := handleAPIError(mockResponse, apiErr, "")
+			msg := fmt.Sprintf("%s", err.Error())
+			msg += ""
+		}
+	}
+
+	require.NotPanics(processAPIError(apiResponseErrWithProperEmbededResponse), "API error should not panic when stringed")
+	require.NotPanics(processAPIError(apiResponseErrWithEmbededResponseWithoutRequest), "API error with embedded empty response should not panic when stringed")
+	require.NotPanics(processAPIError(apiResponseErrWithoutEmbededResponse), "empty API error should not panic when stringed")
+}

provider/github/poster.go

@@ -16,7 +16,7 @@ import (
 
 var (
 	// ErrGitHubAPI signals an error while making a request to the GitHub API.
-	ErrGitHubAPI = errors.NewKind("github api error")
+	ErrGitHubAPI = errors.NewKind("github api error: %s")
 	// ErrEventNotSupported signals that this provider does not support the
 	// given event for a given operation.
 	ErrEventNotSupported = errors.NewKind("event not supported")
@@ -85,7 +85,7 @@ func (p *Poster) postPR(ctx context.Context, e *lookout.ReviewEvent,
 	cc, resp, err := client.Repositories.CompareCommits(ctx, owner, repo,
 		e.Base.Hash,
 		e.Head.Hash)
-	if err = handleAPIError(resp, err); err != nil {
+	if err = handleAPIError(resp, err, "commits could not be compared"); err != nil {
 		return err
 	}
 
@@ -268,7 +268,7 @@ func (p *Poster) statusPR(ctx context.Context, e *lookout.ReviewEvent, status lo
 
 	_, _, err = client.Repositories.CreateStatus(ctx, owner, repo, e.CommitRevision.Head.Hash, repoStatus)
 	if err != nil {
-		return ErrGitHubAPI.Wrap(err)
+		return ErrGitHubAPI.Wrap(err, "commit status could not be pushed")
 	}
 
 	return nil

provider/github/review.go

@@ -40,7 +40,8 @@ func createReview(
 	requests := splitReviewRequest(req, batchReviewComments)
 	for i, req := range requests {
 		_, resp, err := client.PullRequests.CreateReview(ctx, owner, repo, number, req)
-		if err = handleAPIError(resp, err); err != nil {
+
+		if err = handleAPIError(resp, err, "review could not be pushed"); err != nil {
 			return err
 		}
 
@@ -99,7 +100,7 @@ func getPostedComment(ctx context.Context, client *Client, owner, repo string, n
 	var reviews []*github.PullRequestReview
 	for {
 		rs, resp, err := client.PullRequests.ListReviews(ctx, owner, repo, number, listReviewsOpts)
-		if handleAPIError(resp, err) != nil {
+		if handleAPIError(resp, err, "pull request reviews could not be listed") != nil {
 			return nil, err
 		}
 
@@ -118,7 +119,7 @@ func getPostedComment(ctx context.Context, client *Client, owner, repo string, n
 
 		for {
 			comments, resp, err := client.PullRequests.ListReviewComments(ctx, owner, repo, int64(number), review.GetID(), listCommentsOpts)
-			if handleAPIError(resp, err) != nil {
+			if handleAPIError(resp, err, "review comments could not be listed") != nil {
 				return nil, err
 			}
 

provider/github/watcher.go

@@ -312,7 +312,7 @@ func (w *Watcher) doPRListRequest(ctx context.Context, client *Client, username,
 
 	prs, resp, err := client.PullRequests.List(ctx, username, repository, &github.PullRequestListOptions{})
 	if err != nil {
-		return resp, nil, ErrGitHubAPI.Wrap(err)
+		return resp, nil, ErrGitHubAPI.Wrap(err, "pull requests could not be listed")
 	}
 
 	if isStatusNotModified(resp.Response) {
@@ -333,7 +333,7 @@ func (w *Watcher) doEventRequest(ctx context.Context, client *Client, username,
 	)
 
 	if err != nil {
-		return resp, nil, ErrGitHubAPI.Wrap(err)
+		return resp, nil, ErrGitHubAPI.Wrap(err, "repository events could not be listed")
 	}
 
 	if isStatusNotModified(resp.Response) {