clients/http: better error handling

4 files changed, 66 insertions, 7 deletions

diff --git a/clients/http/common.go b/clients/http/common.go
index a4b86bd..703208b 100644
--- a/clients/http/common.go
+++ b/clients/http/common.go
@@ -48,11 +48,14 @@ func NewHTTPError(r *http.Response) error {
 		return nil
 	}
 
-	err := &HTTPError{r}
-	if r.StatusCode == 404 || r.StatusCode == 401 {
-		return core.NewPermanentError(common.ErrNotFound)
+	switch r.StatusCode {
+	case 401:
+		return common.ErrAuthorizationRequired
+	case 404:
+		return common.ErrRepositoryNotFound
 	}
 
+	err := &HTTPError{r}
 	return core.NewUnexpectedError(err)
 }
 
diff --git a/clients/http/common_test.go b/clients/http/common_test.go
index d3e2c9d..287897d 100644
--- a/clients/http/common_test.go
+++ b/clients/http/common_test.go
@@ -28,11 +28,11 @@ func (s *SuiteCommon) TestNewHTTPError200(c *C) {
 }
 
 func (s *SuiteCommon) TestNewHTTPError401(c *C) {
-	s.testNewHTTPError(c, 401, "permanent client error.*not found.*")
+	s.testNewHTTPError(c, 401, "authorization required")
 }
 
 func (s *SuiteCommon) TestNewHTTPError404(c *C) {
-	s.testNewHTTPError(c, 404, "permanent client error.*not found.*")
+	s.testNewHTTPError(c, 404, "repository not found")
 }
 
 func (s *SuiteCommon) TestNewHTTPError40x(c *C) {
diff --git a/clients/http/git_upload_pack.go b/clients/http/git_upload_pack.go
index 79aec3f..bca4534 100644
--- a/clients/http/git_upload_pack.go
+++ b/clients/http/git_upload_pack.go
@@ -1,6 +1,7 @@
 package http
 
 import (
+	"bufio"
 	"fmt"
 	"io"
 	"net/http"
@@ -66,11 +67,20 @@ func (s *GitUploadPackService) Fetch(r *common.GitUploadPackRequest) (io.ReadClo
 		return nil, err
 	}
 
-	if err := s.discardResponseInfo(res.Body); err != nil {
+	reader := newBufferedReadCloser(res.Body)
+	if _, err := reader.Peek(1); err != nil {
+		if err == io.ErrUnexpectedEOF {
+			return nil, common.ErrEmptyGitUploadPack
+		}
+
+		return nil, err
+	}
+
+	if err := s.discardResponseInfo(reader); err != nil {
 		return nil, err
 	}
 
-	return res.Body, nil
+	return reader, nil
 }
 
 func (s *GitUploadPackService) discardResponseInfo(r io.Reader) error {
@@ -138,3 +148,16 @@ func (s *GitUploadPackService) applyAuthToRequest(req *http.Request) {
 func (s *GitUploadPackService) Disconnect() (err error) {
 	return nil
 }
+
+type bufferedReadCloser struct {
+	*bufio.Reader
+	closer io.Closer
+}
+
+func newBufferedReadCloser(r io.ReadCloser) *bufferedReadCloser {
+	return &bufferedReadCloser{bufio.NewReader(r), r}
+}
+
+func (r *bufferedReadCloser) Close() error {
+	return r.closer.Close()
+}
diff --git a/clients/http/git_upload_pack_test.go b/clients/http/git_upload_pack_test.go
index e252ced..e344a49 100644
--- a/clients/http/git_upload_pack_test.go
+++ b/clients/http/git_upload_pack_test.go
@@ -41,6 +41,26 @@ func (s *RemoteSuite) TestConnectWithAuthWrongType(c *C) {
 	c.Assert(r.ConnectWithAuth(&mockAuth{}), Equals, common.ErrInvalidAuthMethod)
 }
 
+func (s *RemoteSuite) TestInfoEmpty(c *C) {
+	endpoint, _ := common.NewEndpoint("https://github.com/git-fixture/empty")
+	r := NewGitUploadPackService(endpoint)
+	c.Assert(r.Connect(), IsNil)
+
+	info, err := r.Info()
+	c.Assert(err, Equals, common.ErrAuthorizationRequired)
+	c.Assert(info, IsNil)
+}
+
+func (s *RemoteSuite) TestInfoNotExists(c *C) {
+	endpoint, _ := common.NewEndpoint("https://github.com/git-fixture/not-exists")
+	r := NewGitUploadPackService(endpoint)
+	c.Assert(r.Connect(), IsNil)
+
+	info, err := r.Info()
+	c.Assert(err, Equals, common.ErrAuthorizationRequired)
+	c.Assert(info, IsNil)
+}
+
 func (s *RemoteSuite) TestDefaultBranch(c *C) {
 	r := NewGitUploadPackService(s.Endpoint)
 	c.Assert(r.Connect(), IsNil)
@@ -74,6 +94,19 @@ func (s *RemoteSuite) TestFetch(c *C) {
 	c.Assert(b, HasLen, 85374)
 }
 
+func (s *RemoteSuite) TestFetchNoChanges(c *C) {
+	r := NewGitUploadPackService(s.Endpoint)
+	c.Assert(r.Connect(), IsNil)
+
+	req := &common.GitUploadPackRequest{}
+	req.Want(core.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5"))
+	req.Have(core.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5"))
+
+	reader, err := r.Fetch(req)
+	c.Assert(err, Equals, common.ErrEmptyGitUploadPack)
+	c.Assert(reader, IsNil)
+}
+
 func (s *RemoteSuite) TestFetchMulti(c *C) {
 	r := NewGitUploadPackService(s.Endpoint)
 	c.Assert(r.Connect(), IsNil)