plugin/discovery: sentinel error values for Get errors

Some errors from Get are essentially user error, so we want to be able to
recognize them and give the user good feedback on how to proceed.

Although sentinel values are not an ideal solution to this, it's something
reasonably simple we can do to get this done without lots of refactoring.

plugin/discovery/error.go

@@ -0,0 +1,30 @@
+package discovery
+
+// Error is a type used to describe situations that the caller must handle
+// since they indicate some form of user error.
+//
+// The functions and methods that return these specialized errors indicate so
+// in their documentation. The Error type should not itself be used directly,
+// but rather errors should be compared using the == operator with the
+// error constants in this package.
+//
+// Values of this type are _not_ used when the error being reported is an
+// operational error (server unavailable, etc) or indicative of a bug in
+// this package or its caller.
+type Error string
+
+// ErrorNoSuitableVersion indicates that a suitable version (meeting given
+// constraints) is not available.
+const ErrorNoSuitableVersion = Error("no suitable version is available")
+
+// ErrorNoVersionCompatible indicates that all of the available versions
+// that otherwise met constraints are not compatible with the current
+// version of Terraform.
+const ErrorNoVersionCompatible = Error("no available version is compatible with this version of Terraform")
+
+// ErrorNoSuchProvider indicates that no provider exists with a name given
+const ErrorNoSuchProvider = Error("no provider exists with the given name")
+
+func (err Error) Error() string {
+	return string(err)
+}

plugin/discovery/get.go

@@ -71,6 +71,25 @@ type ProviderInstaller struct {
 	PluginProtocolVersion uint
 }
 
+// Get is part of an implementation of type Installer, and attempts to download
+// and install a Terraform provider matching the given constraints.
+//
+// This method may return one of a number of sentinel errors from this
+// package to indicate issues that are likely to be resolvable via user action:
+//
+//     ErrorNoSuchProvider: no provider with the given name exists in the repository.
+//     ErrorNoSuitableVersion: the provider exists but no available version matches constraints.
+//     ErrorNoVersionCompatible: a plugin was found within the constraints but it is
+//                               incompatible with the current Terraform version.
+//
+// These errors should be recognized and handled as special cases by the caller
+// to present a suitable user-oriented error message.
+//
+// All other errors indicate an internal problem that is likely _not_ solvable
+// through user action, or at least not within Terraform's scope. Error messages
+// are produced under the assumption that if presented to the user they will
+// be presented alongside context about what is being installed, and thus the
+// error messages do not redundantly include such information.
 func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, error) {
 	versions, err := listProviderVersions(provider)
 	// TODO: return multiple errors
@@ -79,12 +98,12 @@ func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, e
 	}
 
 	if len(versions) == 0 {
-		return PluginMeta{}, fmt.Errorf("no plugins found for provider %q", provider)
+		return PluginMeta{}, ErrorNoSuitableVersion
 	}
 
 	versions = allowedVersions(versions, req)
 	if len(versions) == 0 {
-		return PluginMeta{}, fmt.Errorf("no version of %q available that fulfills constraints %s", provider, req)
+		return PluginMeta{}, ErrorNoSuitableVersion
 	}
 
 	// sort them newest to oldest
@@ -116,8 +135,8 @@ func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, e
 				// contains an executable file whose name doesn't match the
 				// expected convention.
 				return PluginMeta{}, fmt.Errorf(
-					"failed to find installed provider %s %s; this is a bug in Terraform and should be reported",
-					provider, v,
+					"failed to find installed plugin version %s; this is a bug in Terraform and should be reported",
+					v,
 				)
 			}
 
@@ -127,8 +146,8 @@ func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, e
 				// executable filename. We consider releases as immutable, so
 				// this is an error.
 				return PluginMeta{}, fmt.Errorf(
-					"multiple plugins installed for %s %s; this is a bug in Terraform and should be reported",
-					provider, v,
+					"multiple plugins installed for version %s; this is a bug in Terraform and should be reported",
+					v,
 				)
 			}
 
@@ -140,7 +159,7 @@ func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, e
 		log.Printf("[INFO] incompatible ProtocolVersion for %s version %s", provider, v)
 	}
 
-	return PluginMeta{}, fmt.Errorf("no versions of %q compatible with the plugin ProtocolVersion", provider)
+	return PluginMeta{}, ErrorNoVersionCompatible
 }
 
 func (i *ProviderInstaller) PurgeUnused(used map[string]PluginMeta) (PluginMetaSet, error) {
@@ -223,7 +242,9 @@ func allowedVersions(available []Version, required Constraints) []Version {
 func listProviderVersions(name string) ([]Version, error) {
 	versions, err := listPluginVersions(providerVersionsURL(name))
 	if err != nil {
-		return nil, fmt.Errorf("failed to fetch versions for provider %q: %s", name, err)
+		// listPluginVersions returns a verbose error message indicating
+		// what was being accessed and what failed
+		return nil, err
 	}
 	return versions, nil
 }
@@ -232,6 +253,8 @@ func listProviderVersions(name string) ([]Version, error) {
 func listPluginVersions(url string) ([]Version, error) {
 	resp, err := httpClient.Get(url)
 	if err != nil {
+		// http library produces a verbose error message that includes the
+		// URL being accessed, etc.
 		return nil, err
 	}
 	defer resp.Body.Close()
@@ -239,7 +262,18 @@ func listPluginVersions(url string) ([]Version, error) {
 	if resp.StatusCode != http.StatusOK {
 		body, _ := ioutil.ReadAll(resp.Body)
 		log.Printf("[ERROR] failed to fetch plugin versions from %s\n%s\n%s", url, resp.Status, body)
-		return nil, errors.New(resp.Status)
+
+		switch resp.StatusCode {
+		case http.StatusNotFound, http.StatusForbidden:
+			// These are treated as indicative of the given name not being
+			// a valid provider name at all.
+			return nil, ErrorNoSuchProvider
+
+		default:
+			// All other errors are assumed to be operational problems.
+			return nil, fmt.Errorf("error accessing %s: %s", url, resp.Status)
+		}
+
 	}
 
 	body, err := html.Parse(resp.Body)

plugin/discovery/get_test.go

@@ -123,7 +123,7 @@ func TestProviderInstallerGet(t *testing.T) {
 		PluginProtocolVersion: 5,
 	}
 	_, err = i.Get("test", AllVersions)
-	if err == nil {
+	if err != ErrorNoVersionCompatible {
 		t.Fatal("want error for incompatible version")
 	}
 
@@ -132,6 +132,21 @@ func TestProviderInstallerGet(t *testing.T) {
 
 		PluginProtocolVersion: 3,
 	}
+
+	{
+		_, err := i.Get("test", ConstraintStr(">9.0.0").MustParse())
+		if err != ErrorNoSuitableVersion {
+			t.Fatal("want error for mismatching constraints")
+		}
+	}
+
+	{
+		_, err := i.Get("nonexist", AllVersions)
+		if err != ErrorNoSuchProvider {
+			t.Fatal("want error for no such provider")
+		}
+	}
+
 	gotMeta, err := i.Get("test", AllVersions)
 	if err != nil {
 		t.Fatal(err)