Adding a better error message for download mode

cmd/proxy/actions/app.go

@@ -130,8 +130,7 @@ func App(conf *config.Config) (http.Handler, error) {
 		lggr,
 		conf,
 	); err != nil {
-		err = fmt.Errorf("error adding proxy routes (%s)", err)
-		return nil, err
+		return nil, fmt.Errorf("Error initializing Athens:\n%s", err)
 	}
 
 	h := &ochttp.Handler{

pkg/config/config.go

@@ -50,14 +50,25 @@ type Config struct {
 	TLSKeyFile       string    `envconfig:"ATHENS_TLSKEY_FILE"`
 	SumDBs           []string  `envconfig:"ATHENS_SUM_DBS"`
 	NoSumPatterns    []string  `envconfig:"ATHENS_GONOSUM_PATTERNS"`
-	DownloadMode     mode.Mode `envconfig:"ATHENS_DOWNLOAD_MODE"`
+	DownloadMode     mode.Mode `validate:"required" envconfig:"ATHENS_DOWNLOAD_MODE" default:"sync"`
 	DownloadURL      string    `envconfig:"ATHENS_DOWNLOAD_URL"`
 	SingleFlightType string    `envconfig:"ATHENS_SINGLE_FLIGHT_TYPE"`
 	RobotsFile       string    `envconfig:"ATHENS_ROBOTS_FILE"`
 	SingleFlight     *SingleFlight
 	Storage          *StorageConfig
 }
 
+// Validate implements Validator
+func (c *Config) Validate() error {
+	const op errors.Op = "Config.Validate"
+	if !strings.HasPrefix(c.Port, ":") {
+		return errors.E(
+			op,
+			"Invalid configuration for Port. This value must start with ':'",
+		)
+	}
+}
+
 // EnvList is a list of key-value environment
 // variables that are passed to the Go command
 type EnvList []string
@@ -266,6 +277,9 @@ func validateConfig(config Config) error {
 	if err != nil {
 		return err
 	}
+	if err := config.Validate(); err != nil {
+		return err
+	}
 	switch config.StorageType {
 	case "memory":
 		return nil

pkg/config/validator.go

@@ -0,0 +1,8 @@
+package config
+
+// Validator can validate a config struct. If you implement this,
+// validate all of the configuration in your struct. It will
+// automatically be called when Athens starts
+type Validator interface {
+	Validate() error
+}

pkg/download/mode/mode.go

@@ -18,15 +18,33 @@ type Mode string
 
 // DownloadMode constants. For more information see config.dev.toml
 const (
-	Sync            Mode = "sync"
-	Async           Mode = "async"
-	Redirect        Mode = "redirect"
-	AsyncRedirect   Mode = "async_redirect"
-	None            Mode = "none"
-	downloadModeErr      = "download mode is not set"
-	invalidModeErr       = "unrecognized download mode: %s"
+	Sync          Mode = "sync"
+	Async         Mode = "async"
+	Redirect      Mode = "redirect"
+	AsyncRedirect Mode = "async_redirect"
+	None          Mode = "none"
+	// This is the URL that logs will show when the DownloadMode
+	// config value is invalid
+	downloadModeURL = "https://docs.gomods.io/configuration/download/"
 )
 
+// Validate ensures that m is a valid mode. If this function returns nil, you are
+// guaranteed that m is valid
+func (m Mode) Validate() error {
+	const op errors.Op = "Mode.Validate"
+	switch m {
+	case Sync, Async, Redirect, AsyncRedirect, None:
+		return nil
+	default:
+		return errors.Config(
+			op,
+			"mode",
+			fmt.Sprintf("%s isn't a valid value.", m),
+			"https://docs.gomods.io/configuration/download/",
+		)
+	}
+}
+
 // DownloadFile represents a custom HCL format of
 // how to handle module@version requests that are
 // not found in storage.
@@ -44,6 +62,20 @@ type DownloadPath struct {
 	DownloadURL string `hcl:"downloadURL,optional"`
 }
 
+// Validate implements config.Validator
+func (d DownloadPath) Validate() error {
+	const op errors.Op = "DownloadPath.Validate"
+	if d.DownloadURL == "" && (d.Mode == Redirect || d.Mode == AsyncRedirect) {
+		return errors.Config(
+			op,
+			fmt.Sprintf("DownloadURL (inside %s in the download file)", d.Pattern),
+			"You must set a value when the download mode is 'redirect' or 'async_redirect'",
+			"https://docs.gomods.io/configuration/download/",
+		)
+	}
+	return nil
+}
+
 // NewFile takes a mode and returns a DownloadFile.
 // Mode can be one of the constants declared above
 // or a custom HCL file. To pass a custom HCL file,
@@ -53,8 +85,8 @@ type DownloadPath struct {
 func NewFile(m Mode, downloadURL string) (*DownloadFile, error) {
 	const op errors.Op = "downloadMode.NewFile"
 
-	if m == "" {
-		return nil, errors.E(op, downloadModeErr)
+	if err := m.Validate(); err != nil {
+		return nil, err
 	}
 
 	if strings.HasPrefix(string(m), "file:") {
@@ -72,12 +104,11 @@ func NewFile(m Mode, downloadURL string) (*DownloadFile, error) {
 		return parseFile(bts)
 	}
 
-	switch m {
-	case Sync, Async, Redirect, AsyncRedirect, None:
-		return &DownloadFile{Mode: m, DownloadURL: downloadURL}, nil
-	default:
-		return nil, errors.E(op, errors.KindBadRequest, fmt.Sprintf(invalidModeErr, m))
+	retFile := &DownloadFile{Mode: m, DownloadURL: downloadURL}
+	if err := retFile.Validate(); err != nil {
+		return nil, err
 	}
+	return retFile, nil
 }
 
 // parseFile parses an HCL file according to the
@@ -93,19 +124,29 @@ func parseFile(file []byte) (*DownloadFile, error) {
 	if dig.HasErrors() {
 		return nil, errors.E(op, dig.Error())
 	}
-	if err := df.validate(); err != nil {
+	if err := df.Validate(); err != nil {
 		return nil, errors.E(op, err)
 	}
 	return &df, nil
 }
 
-func (d *DownloadFile) validate() error {
-	const op errors.Op = "downloadMode.validate"
+// Validate validates the download file. It implements the
+// config.Validator interface
+func (d *DownloadFile) Validate() error {
+	const op errors.Op = "DownloadFile.Validate"
 	for _, p := range d.Paths {
+		if err := p.Mode.Validate(); err != nil {
+			return err
+		}
 		switch p.Mode {
 		case Sync, Async, Redirect, AsyncRedirect, None:
 		default:
-			return errors.E(op, fmt.Errorf("unrecognized mode for %v: %v", p.Pattern, p.Mode))
+			return errors.Config(
+				op,
+				fmt.Sprintf("mode (in pattern %v", p.Pattern),
+				fmt.Sprintf("%s is unrecognized", p.Mode),
+				"https://docs.gomods.io/configuration/download/",
+			)
 		}
 	}
 	return nil

pkg/download/mode/mode_test.go

@@ -1,8 +1,9 @@
 package mode
 
 import (
-	"fmt"
 	"testing"
+
+	"github.com/gomods/athens/pkg/errors"
 )
 
 var testCases = []struct {
@@ -119,28 +120,57 @@ func TestMode(t *testing.T) {
 }
 
 func TestNewFile_err(t *testing.T) {
+	const op = errors.Op("downloadMode.NewFile")
 	tc := []struct {
-		name     string
-		mode     Mode
-		expected string
+		name   string
+		mode   Mode
+		hasErr bool
 	}{
 		{
-			name:     "empty mode",
-			mode:     "",
-			expected: downloadModeErr,
+			name:   "empty mode",
+			mode:   "",
+			hasErr: true,
 		},
 		{
-			name:     "invalid mode",
-			mode:     "invalidMode",
-			expected: fmt.Sprintf(invalidModeErr, "invalidMode"),
+			name:   "invalid mode",
+			mode:   "invalidMode",
+			hasErr: true,
 		},
 	}
 	for _, c := range tc {
 		t.Run(c.name, func(subT *testing.T) {
 			_, err := NewFile(c.mode, "github.com/gomods/athens")
-			if err.Error() != c.expected {
-				t.Fatalf("expected error %s from NewFile, got %s", c.expected, err.Error())
+			if c.hasErr && err == nil {
+				t.Errorf(
+					"Expected error for mode %s, but got none",
+					c.mode,
+				)
+			}
+			if !c.hasErr && err != nil {
+				t.Errorf(
+					"Expected no error for mode %s, but got %s",
+					c.mode,
+					err,
+				)
 			}
 		})
 	}
+	// loop through all of the valid modes
+	modeStrings := []string{
+		"sync",
+		"async",
+		"redirect",
+		"async_redirect",
+		"none",
+	}
+	for _, modeString := range modeStrings {
+		_, err := NewFile(Mode(modeString), "github.com/gomods/athens")
+		if err != nil {
+			t.Errorf(
+				"Expected no error for mode %s, got %s",
+				modeString,
+				err,
+			)
+		}
+	}
 }

pkg/errors/config.go

@@ -0,0 +1,22 @@
+package errors
+
+import (
+	"fmt"
+	"strings"
+)
+
+// Config returns an error specifically tailored for reporting errors with configuration
+// values. You can check for these errors by calling errors.Is(err, KindConfigError)
+// (from the github.com/gomods/athens/pkg/errors package).
+//
+// Generally these kinds of errors should make Athens crash because it was configured
+// improperly
+func Config(op Op, field, helpText, url string) error {
+	slc := []string{
+		fmt.Sprintf("There was a configuration error with %s. %s", field, helpText),
+	}
+	if url != "" {
+		slc = append(slc, fmt.Sprintf("Please see %s for more information.", url))
+	}
+	return E(op, KindConfigError, strings.Join(slc, " "))
+}

pkg/errors/errors.go

@@ -18,6 +18,7 @@ const (
 	KindRateLimit      = http.StatusTooManyRequests
 	KindNotImplemented = http.StatusNotImplemented
 	KindRedirect       = http.StatusMovedPermanently
+	KindConfigError    = http.StatusUnprocessableEntity
 )
 
 // Error is an Athens system error.