pkg/module: return KindNotFound on incorrect mod download (#1300)

pkg/download/latest.go

@@ -26,7 +26,9 @@ func LatestHandler(dp Protocol, lggr log.Entry, df *mode.DownloadFile) http.Hand
 
 		info, err := dp.Latest(r.Context(), mod)
 		if err != nil {
-			lggr.SystemErr(errors.E(op, err))
+			severityLevel := errors.Expect(err, errors.KindNotFound)
+			err = errors.E(op, err, severityLevel)
+			lggr.SystemErr(err)
 			w.WriteHeader(errors.Kind(err))
 			return
 		}

pkg/download/list.go

@@ -27,7 +27,9 @@ func ListHandler(dp Protocol, lggr log.Entry, df *mode.DownloadFile) http.Handle
 
 		versions, err := dp.List(r.Context(), mod)
 		if err != nil {
-			lggr.SystemErr(errors.E(op, err))
+			severityLevel := errors.Expect(err, errors.KindNotFound)
+			err = errors.E(op, err, severityLevel)
+			lggr.SystemErr(err)
 			w.WriteHeader(errors.Kind(err))
 			return
 		}

pkg/download/version_info.go

@@ -23,7 +23,8 @@ func InfoHandler(dp Protocol, lggr log.Entry, df *mode.DownloadFile) http.Handle
 		}
 		info, err := dp.Info(r.Context(), mod, ver)
 		if err != nil {
-			lggr.SystemErr(errors.E(op, err, errors.M(mod), errors.V(ver)))
+			severityLevel := errors.Expect(err, errors.KindNotFound, errors.KindRedirect)
+			lggr.SystemErr(errors.E(op, err, errors.M(mod), errors.V(ver), severityLevel))
 			if errors.Kind(err) == errors.KindRedirect {
 				http.Redirect(w, r, getRedirectURL(df.URL(mod), r.URL.Path), errors.KindRedirect)
 				return

pkg/download/version_module.go

@@ -24,6 +24,8 @@ func ModuleHandler(dp Protocol, lggr log.Entry, df *mode.DownloadFile) http.Hand
 		}
 		modBts, err := dp.GoMod(r.Context(), mod, ver)
 		if err != nil {
+			severityLevel := errors.Expect(err, errors.KindNotFound, errors.KindRedirect)
+			err = errors.E(op, err, severityLevel)
 			lggr.SystemErr(err)
 			if errors.Kind(err) == errors.KindRedirect {
 				http.Redirect(w, r, getRedirectURL(df.URL(mod), r.URL.Path), errors.KindRedirect)

pkg/download/version_zip.go

@@ -24,6 +24,8 @@ func ZipHandler(dp Protocol, lggr log.Entry, df *mode.DownloadFile) http.Handler
 		}
 		zip, err := dp.Zip(r.Context(), mod, ver)
 		if err != nil {
+			severityLevel := errors.Expect(err, errors.KindNotFound, errors.KindRedirect)
+			err = errors.E(op, err, severityLevel)
 			lggr.SystemErr(err)
 			if errors.Kind(err) == errors.KindRedirect {
 				http.Redirect(w, r, getRedirectURL(df.URL(mod), r.URL.Path), errors.KindRedirect)

pkg/errors/errors.go

@@ -130,6 +130,18 @@ func Severity(err error) logrus.Level {
 	return e.Severity
 }
 
+// Expect is a helper that returns an Info level
+// if the error has the expected kind, otherwise
+// it returns an Error level.
+func Expect(err error, kinds ...int) logrus.Level {
+	for _, kind := range kinds {
+		if Kind(err) == kind {
+			return logrus.InfoLevel
+		}
+	}
+	return logrus.ErrorLevel
+}
+
 // Kind recursively searches for the
 // first error kind it finds.
 func Kind(err error) int {

pkg/errors/errors_test.go

@@ -80,3 +80,19 @@ func (op *OpTests) TestString() {
 	const op1 Op = "testOps.op1"
 	require.Equal(op.T(), op1.String(), "testOps.op1")
 }
+
+func TestExpect(t *testing.T) {
+	err := E("TestExpect", "error message", KindBadRequest)
+
+	severity := Expect(err, KindBadRequest)
+	require.Equalf(t, severity, logrus.InfoLevel, "expected an info level log but got %v", severity)
+
+	severity = Expect(err, KindAlreadyExists)
+	require.Equalf(t, severity, logrus.ErrorLevel, "expected an error level but got %v", severity)
+
+	severity = Expect(err, KindAlreadyExists, KindBadRequest)
+	require.Equalf(t, severity, logrus.InfoLevel, "expected an info level log but got %v", severity)
+
+	severity = Expect(err, KindAlreadyExists, KindNotImplemented)
+	require.Equalf(t, severity, logrus.ErrorLevel, "expected an error level but got %v", severity)
+}

pkg/module/go_get_fetcher.go

@@ -115,11 +115,15 @@ func downloadModule(goBinaryName string, fs afero.Fs, gopath, repoRoot, module,
 	err := cmd.Run()
 	if err != nil {
 		err = fmt.Errorf("%v: %s", err, stderr)
-		// github quota exceeded
-		if isLimitHit(err.Error()) {
-			return goModule{}, errors.E(op, err, errors.KindRateLimit)
+		var m goModule
+		if jsonErr := json.NewDecoder(stdout).Decode(&m); jsonErr != nil {
+			return goModule{}, errors.E(op, err)
 		}
-		return goModule{}, errors.E(op, err)
+		// github quota exceeded
+		if isLimitHit(m.Error) {
+			return goModule{}, errors.E(op, m.Error, errors.KindRateLimit)
+		}
+		return goModule{}, errors.E(op, m.Error, errors.KindNotFound)
 	}
 
 	var m goModule

pkg/module/go_get_fetcher_test.go

@@ -5,6 +5,7 @@ import (
 	"io/ioutil"
 	"runtime"
 
+	"github.com/gomods/athens/pkg/errors"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 )
@@ -51,3 +52,18 @@ func (s *ModuleSuite) TestGoGetFetcherFetch() {
 	// close the version's zip file (which also cleans up the underlying GOPATH) and expect it to fail again
 	r.NoError(ver.Zip.Close())
 }
+
+func (s *ModuleSuite) TestNotFoundFetches() {
+	r := s.Require()
+	fetcher, err := NewGoGetFetcher(s.goBinaryName, afero.NewOsFs())
+	r.NoError(err)
+	// when someone buys laks47dfjoijskdvjxuyyd.com, and implements
+	// a git server on top of it, this test will fail :)
+	_, err = fetcher.Fetch(ctx, "laks47dfjoijskdvjxuyyd.com/pkg/errors", "v0.8.1")
+	if err == nil {
+		s.Fail("expected an error but got nil")
+	}
+	if errors.Kind(err) != errors.KindNotFound {
+		s.Failf("incorrect error kind", "expected a not found error but got %v", errors.Kind(err))
+	}
+}