plumbing: transport/http, Wrap http errors to return reason. Fixes #1097

5 files changed, 54 insertions, 9 deletions

diff --git a/internal/test/checkers.go b/internal/test/checkers.go
new file mode 100644
index 0000000..257d93d
--- /dev/null
+++ b/internal/test/checkers.go
@@ -0,0 +1,43 @@
+package test
+
+import (
+	"errors"
+	"fmt"
+
+	check "gopkg.in/check.v1"
+)
+
+// This check.Checker implementation exists because there's no implementation
+// in the library that compares errors using `errors.Is`. If / when the check
+// library fixes https://github.com/go-check/check/issues/139, this code can
+// likely be removed and replaced with the library implementation.
+//
+// Added in Go 1.13 [https://go.dev/blog/go1.13-errors] `errors.Is` is the
+// best mechanism to use to compare errors that might be wrapped in other
+// errors.
+type errorIsChecker struct {
+	*check.CheckerInfo
+}
+
+var ErrorIs check.Checker = errorIsChecker{
+	&check.CheckerInfo{
+		Name:   "ErrorIs",
+		Params: []string{"obtained", "expected"},
+	},
+}
+
+func (e errorIsChecker) Check(params []interface{}, names []string) (bool, string) {
+	obtained, ok := params[0].(error)
+	if !ok {
+		return false, "obtained is not an error"
+	}
+	expected, ok := params[1].(error)
+	if !ok {
+		return false, "expected is not an error"
+	}
+
+	if !errors.Is(obtained, expected) {
+		return false, fmt.Sprintf("obtained: %+v expected: %+v", obtained, expected)
+	}
+	return true, ""
+}
diff --git a/plumbing/transport/http/common.go b/plumbing/transport/http/common.go
index 1c4ceee..120008d 100644
--- a/plumbing/transport/http/common.go
+++ b/plumbing/transport/http/common.go
@@ -430,11 +430,11 @@ func NewErr(r *http.Response) error {
 
 	switch r.StatusCode {
 	case http.StatusUnauthorized:
-		return transport.ErrAuthenticationRequired
+		return fmt.Errorf("%w: %s", transport.ErrAuthenticationRequired, reason)
 	case http.StatusForbidden:
-		return transport.ErrAuthorizationFailed
+		return fmt.Errorf("%w: %s", transport.ErrAuthorizationFailed, reason)
 	case http.StatusNotFound:
-		return transport.ErrRepositoryNotFound
+		return fmt.Errorf("%w: %s", transport.ErrRepositoryNotFound, reason)
 	}
 
 	return plumbing.NewUnexpectedError(&Err{r, reason})
diff --git a/plumbing/transport/http/common_test.go b/plumbing/transport/http/common_test.go
index c831ac3..f0eb68d 100644
--- a/plumbing/transport/http/common_test.go
+++ b/plumbing/transport/http/common_test.go
@@ -76,15 +76,15 @@ func (s *ClientSuite) TestNewErrOK(c *C) {
 }
 
 func (s *ClientSuite) TestNewErrUnauthorized(c *C) {
-	s.testNewHTTPError(c, http.StatusUnauthorized, "authentication required")
+	s.testNewHTTPError(c, http.StatusUnauthorized, ".*authentication required.*")
 }
 
 func (s *ClientSuite) TestNewErrForbidden(c *C) {
-	s.testNewHTTPError(c, http.StatusForbidden, "authorization failed")
+	s.testNewHTTPError(c, http.StatusForbidden, ".*authorization failed.*")
 }
 
 func (s *ClientSuite) TestNewErrNotFound(c *C) {
-	s.testNewHTTPError(c, http.StatusNotFound, "repository not found")
+	s.testNewHTTPError(c, http.StatusNotFound, ".*repository not found.*")
 }
 
 func (s *ClientSuite) TestNewHTTPError40x(c *C) {
diff --git a/plumbing/transport/http/upload_pack_test.go b/plumbing/transport/http/upload_pack_test.go
index abb7adf..3a1610a 100644
--- a/plumbing/transport/http/upload_pack_test.go
+++ b/plumbing/transport/http/upload_pack_test.go
@@ -8,6 +8,7 @@ import (
 	"os"
 	"path/filepath"
 
+	. "github.com/go-git/go-git/v5/internal/test"
 	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/go-git/go-git/v5/plumbing/protocol/packp"
 	"github.com/go-git/go-git/v5/plumbing/transport"
@@ -37,7 +38,7 @@ func (s *UploadPackSuite) TestAdvertisedReferencesNotExists(c *C) {
 	r, err := s.Client.NewUploadPackSession(s.NonExistentEndpoint, s.EmptyAuth)
 	c.Assert(err, IsNil)
 	info, err := r.AdvertisedReferences()
-	c.Assert(err, Equals, transport.ErrRepositoryNotFound)
+	c.Assert(err, ErrorIs, transport.ErrRepositoryNotFound)
 	c.Assert(info, IsNil)
 }
 
diff --git a/plumbing/transport/test/receive_pack.go b/plumbing/transport/test/receive_pack.go
index 9414fba..d4d2b10 100644
--- a/plumbing/transport/test/receive_pack.go
+++ b/plumbing/transport/test/receive_pack.go
@@ -9,6 +9,7 @@ import (
 	"os"
 	"path/filepath"
 
+	. "github.com/go-git/go-git/v5/internal/test"
 	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/go-git/go-git/v5/plumbing/format/packfile"
 	"github.com/go-git/go-git/v5/plumbing/protocol/packp"
@@ -42,7 +43,7 @@ func (s *ReceivePackSuite) TestAdvertisedReferencesNotExists(c *C) {
 	r, err := s.Client.NewReceivePackSession(s.NonExistentEndpoint, s.EmptyAuth)
 	c.Assert(err, IsNil)
 	ar, err := r.AdvertisedReferences()
-	c.Assert(err, Equals, transport.ErrRepositoryNotFound)
+	c.Assert(err, ErrorIs, transport.ErrRepositoryNotFound)
 	c.Assert(ar, IsNil)
 	c.Assert(r.Close(), IsNil)
 
@@ -54,7 +55,7 @@ func (s *ReceivePackSuite) TestAdvertisedReferencesNotExists(c *C) {
 	}
 
 	writer, err := r.ReceivePack(context.Background(), req)
-	c.Assert(err, Equals, transport.ErrRepositoryNotFound)
+	c.Assert(err, ErrorIs, transport.ErrRepositoryNotFound)
 	c.Assert(writer, IsNil)
 	c.Assert(r.Close(), IsNil)
 }