Adding a better error message for download mode

cmd/proxy/actions/app.go

@@ -132,8 +132,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{

cmd/proxy/main.go

@@ -15,6 +15,7 @@ import (
 	"github.com/gomods/athens/cmd/proxy/actions"
 	"github.com/gomods/athens/pkg/build"
 	"github.com/gomods/athens/pkg/config"
+	"github.com/gomods/athens/pkg/errors"
 )
 
 var (
@@ -32,6 +33,9 @@ func main() {
 	if err != nil {
 		log.Fatalf("could not load config file: %v", err)
 	}
+	if err := conf.Validate(errors.Op("main")); err != nil {
+		log.Fatal(err)
+	}
 
 	handler, err := actions.App(conf)
 	if err != nil {

pkg/config/config.go

@@ -58,6 +58,13 @@ type Config struct {
 	Storage          *StorageConfig
 }
 
+// Validate checks various values in c that are commonly improperly set.
+// This function is not exhaustive, so c is not guaranteed to be valid even if
+// it doesn't return nil
+func (c Config) Validate(op errors.Op) error {
+	return c.DownloadMode.Validate(op)
+}
+
 // EnvList is a list of key-value environment
 // variables that are passed to the Go command
 type EnvList []string

pkg/download/mode/mode.go

@@ -18,15 +18,29 @@ 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"
 )
 
+// Validate ensures that m is a valid mode. If this function returns nil, you are
+// guaranteed that m is valid
+func (m Mode) Validate(op errors.Op) error {
+	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,17 +58,30 @@ type DownloadPath struct {
 	DownloadURL string `hcl:"downloadURL,optional"`
 }
 
+func (d DownloadPath) validate(op errors.Op) error {
+	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,
 // you can either point to a file path by passing
 // file:/path/to/file OR custom:<base64-encoded-hcl>
 // directly.
 func NewFile(m Mode, downloadURL string) (*DownloadFile, error) {
+	const downloadModeURL = "https://docs.gomods.io/configuration/download/"
 	const op errors.Op = "downloadMode.NewFile"
 
-	if m == "" {
-		return nil, errors.E(op, downloadModeErr)
+	if err := m.Validate(op); err != nil {
+		return nil, err
 	}
 
 	if strings.HasPrefix(string(m), "file:") {
@@ -72,12 +99,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(op); err != nil {
+		return nil, err
 	}
+	return retFile, nil
 }
 
 // parseFile parses an HCL file according to the
@@ -93,19 +119,26 @@ 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(op); err != nil {
 		return nil, errors.E(op, err)
 	}
 	return &df, nil
 }
 
-func (d *DownloadFile) validate() error {
-	const op errors.Op = "downloadMode.validate"
+func (d *DownloadFile) validate(op errors.Op) error {
 	for _, p := range d.Paths {
+		if err := p.Mode.Validate(op); 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,6 +120,7 @@ func TestMode(t *testing.T) {
 }
 
 func TestNewFile_err(t *testing.T) {
+	const op = errors.Op("downloadMode.NewFile")
 	tc := []struct {
 		name     string
 		mode     Mode
@@ -127,12 +129,12 @@ func TestNewFile_err(t *testing.T) {
 		{
 			name:     "empty mode",
 			mode:     "",
-			expected: downloadModeErr,
+			expected: Mode("").Validate(op).Error(),
 		},
 		{
 			name:     "invalid mode",
 			mode:     "invalidMode",
-			expected: fmt.Sprintf(invalidModeErr, "invalidMode"),
+			expected: Mode("invalidMode").Validate(op).Error(),
 		},
 	}
 	for _, c := range tc {

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    = -1
 )
 
 // Error is an Athens system error.